resource: improve error handling

Make some more varargs error functions
Make pw_resource_error always just send the error to the resource id.
Make sure we send errors to the right destination.
Add proxy error event and emit it when the core finds an error for
the given proxy id.
The client error is supposed to be sent to all resources of a client
for the given global.

src/modules/module-access.c

@@ -174,7 +174,7 @@ core_check_access(void *data, struct pw_client *client)
 	return;
 
       blacklisted:
-	pw_resource_error(pw_client_get_core_resource(client), 0, res, "blacklisted");
+	pw_resource_error(pw_client_get_core_resource(client), res, "blacklisted");
 	pw_client_update_properties(client, &SPA_DICT_INIT(items, 1));
 	return;
 

src/modules/module-audio-dsp.c

@@ -154,18 +154,18 @@ static void *create_object(void *_data,
 
       no_resource:
 	pw_log_error("audio-dsp needs a resource");
-	pw_resource_error(resource, new_id, -EINVAL, "no resource");
+	pw_resource_error(resource, -EINVAL, "no resource");
 	goto done;
       no_props:
 	pw_log_error("audio-dsp needs a property");
-	pw_resource_error(resource, new_id, -EINVAL, "no property");
+	pw_resource_error(resource, -EINVAL, "no property");
 	goto done;
       no_mem:
 	pw_log_error("can't create node");
-	pw_resource_error(resource, new_id, -ENOMEM, "no memory");
+	pw_resource_error(resource, -ENOMEM, "no memory");
 	goto done;
       no_bind:
-	pw_resource_error(resource, new_id, res, "can't bind dsp node");
+	pw_resource_error(resource, res, "can't bind dsp node");
 	goto done;
       done:
 	if (properties)

src/modules/module-client-node.c

@@ -89,7 +89,7 @@ static void *create_object(void *_data,
 
       no_mem:
 	pw_log_error("can't create node");
-	pw_resource_error(resource, new_id, -ENOMEM, "no memory");
+	pw_core_resource_error(pw_client_get_core_resource(client), new_id, -ENOMEM, "can't create node: no memory");
 	goto done;
       done:
 	if (properties)

src/modules/module-link-factory.c

@@ -246,30 +246,30 @@ static void *create_object(void *_data,
 
       no_properties:
 	pw_log_error("link-factory needs properties");
-	pw_resource_error(resource, new_id, -EINVAL, "no properties");
+	pw_resource_error(resource, -EINVAL, "no properties");
 	goto done;
       no_output:
-	pw_log_error("link-factory unknown output node %d", output_node_id);
-	pw_resource_error(resource, new_id, -EINVAL, "unknown output node");
+	pw_log_error("link-factory unknown output node %u", output_node_id);
+	pw_resource_error(resource, -EINVAL, "unknown output node %u", output_node_id);
 	goto done;
       no_input:
-	pw_log_error("link-factory unknown input node %d", input_node_id);
-	pw_resource_error(resource, new_id, -EINVAL, "unknown input node");
+	pw_log_error("link-factory unknown input node %u", input_node_id);
+	pw_resource_error(resource, -EINVAL, "unknown input node %u", input_node_id);
 	goto done;
       no_output_port:
-	pw_log_error("link-factory unknown output port %d", output_port_id);
-	pw_resource_error(resource, new_id, -EINVAL, "unknown output port");
+	pw_log_error("link-factory unknown output port %u", output_port_id);
+	pw_resource_error(resource, -EINVAL, "unknown output port %u", output_port_id);
 	goto done;
       no_input_port:
-	pw_log_error("link-factory unknown input port %d", input_port_id);
-	pw_resource_error(resource, new_id, -EINVAL, "unknown input port");
+	pw_log_error("link-factory unknown input port %u", input_port_id);
+	pw_resource_error(resource, -EINVAL, "unknown input port %u", input_port_id);
 	goto done;
       no_mem:
 	pw_log_error("can't create link: %s", error);
-	pw_resource_error(resource, new_id, -ENOMEM, error);
+	pw_resource_error(resource, -ENOMEM, "can't create link: %s", error);
 	goto done;
       no_bind:
-	pw_resource_error(resource, new_id, res, "can't bind link");
+	pw_resource_error(resource, res, "can't bind link");
 	goto done;
       done:
 	if (properties)

src/modules/module-protocol-native.c

@@ -117,13 +117,13 @@ process_messages(struct client_data *data)
 	uint8_t opcode;
 	uint32_t id, size;
 	void *message;
+	struct pw_resource *resource;
 
 	core->current_client = client;
 
 	/* when the client is busy processing an async action, stop processing messages
 	 * for the client until it finishes the action */
 	while (!data->busy) {
-		struct pw_resource *resource;
 		const struct pw_protocol_native_demarshal *demarshal;
 	        const struct pw_protocol_marshal *marshal;
 		uint32_t permissions, required;
@@ -143,7 +143,7 @@ process_messages(struct client_data *data)
 		if (resource == NULL) {
 			pw_log_error("protocol-native %p: unknown resource %u",
 				     client->protocol, id);
-			pw_core_resource_error(client->core_resource, id,
+			pw_resource_error(client->core_resource,
 				     -EINVAL, "unknown resource %u", id);
 			continue;
 		}
@@ -162,7 +162,7 @@ process_messages(struct client_data *data)
 		if ((required & permissions) != required) {
 			pw_log_error("protocol-native %p: method %u on %u requires %08x, have %08x",
 				     client->protocol, opcode, id, required, permissions);
-			pw_core_resource_error(client->core_resource, id,
+			pw_resource_error(resource,
 				-EACCES, "no permission to call method %u ", opcode, id);
 			continue;
 		}
@@ -177,15 +177,13 @@ process_messages(struct client_data *data)
       invalid_method:
 	pw_log_error("protocol-native %p: invalid method %u on resource %u",
 		     client->protocol, opcode, id);
-	pw_core_resource_error(client->core_resource, id,
-		     -EINVAL, "invalid method %u on resource %u", opcode, id);
+	pw_resource_error(resource, -EINVAL, "invalid method %u", opcode);
 	pw_client_destroy(client);
 	goto done;
       invalid_message:
 	pw_log_error("protocol-native %p: invalid message received %u %u",
 		     client->protocol, id, opcode);
-	pw_core_resource_error(client->core_resource, id,
-		     -EINVAL, "invalid message %u %u", opcode, id);
+	pw_resource_error(resource, -EINVAL, "invalid message %u", opcode);
 	spa_debug_pod(0, NULL, (struct spa_pod *)message);
 	pw_client_destroy(client);
 	goto done;

src/modules/module-protocol-native/protocol-native.c

@@ -252,23 +252,17 @@ static void core_marshal_done(void *object, uint32_t seq)
 	pw_protocol_native_end_resource(resource, b);
 }
 
-static void core_marshal_error(void *object, uint32_t id, int res, const char *error, ...)
+static void core_marshal_error(void *object, uint32_t id, int res, const char *error)
 {
 	struct pw_resource *resource = object;
-	char buffer[128];
 	struct spa_pod_builder *b;
-	va_list ap;
 
 	b = pw_protocol_native_begin_resource(resource, PW_CORE_PROXY_EVENT_ERROR);
 
-	va_start(ap, error);
-	vsnprintf(buffer, sizeof(buffer), error, ap);
-	va_end(ap);
-
 	spa_pod_builder_add_struct(b,
 			       SPA_POD_Int(id),
 			       SPA_POD_Int(res),
-			       SPA_POD_String(buffer));
+			       SPA_POD_String(error));
 
 	pw_protocol_native_end_resource(resource, b);
 }

src/modules/spa/module-node-factory.c

@@ -122,16 +122,16 @@ static void *create_object(void *_data,
       no_properties:
 	pw_log_error("needed properties: spa.library.name=<library-name> spa.factory.name=<factory-name>");
 	if (resource) {
-		pw_resource_error(resource, new_id, -EINVAL,
+		pw_resource_error(resource, -EINVAL,
 					"needed properties: "
 						"spa.library.name=<library-name> "
 						"spa.factory.name=<factory-name>");
 	}
 	return NULL;
       no_mem:
-	pw_log_error("can't create node");
+	pw_log_error("can't create node: no memory");
 	if (resource) {
-		pw_resource_error(resource, new_id, -ENOMEM, "no memory");
+		pw_resource_error(resource, -ENOMEM, "no memory");
 	}
 	return NULL;
 }

src/pipewire/client.c

@@ -107,7 +107,18 @@ static void client_error(void *object, uint32_t id, int res, const char *error)
 	struct pw_resource *resource = object;
 	struct resource_data *data = pw_resource_get_user_data(resource);
 	struct pw_client *client = data->client;
-	pw_resource_error(client->core_resource, id, res, error);
+	struct pw_global *global;
+	struct pw_resource *r, *t;
+
+	global = pw_core_find_global(client->core, id);
+	if (global == NULL)
+		return;
+
+	spa_list_for_each_safe(r, t, &global->resource_list, link) {
+		if (t->client != client)
+			continue;
+		pw_resource_error(r, res, error);
+	}
 }
 
 static void client_update_properties(void *object, const struct spa_dict *props)
@@ -194,7 +205,7 @@ global_bind(void *_data, struct pw_client *client, uint32_t permissions,
 
       no_mem:
 	pw_log_error("can't create client resource");
-	pw_resource_error(client->core_resource, id, -ENOMEM, "no memory");
+	pw_core_resource_error(client->core_resource, id, -ENOMEM, "can't create client resource: no memory");
 	return;
 }
 

src/pipewire/core.c

@@ -87,13 +87,11 @@ static void registry_bind(void *object, uint32_t id,
 
       no_id:
 	pw_log_debug("registry %p: no global with id %u to bind to %u", resource, id, new_id);
-	pw_core_resource_error(client->core_resource, id,
-			     -ENOENT, "no such global %u", id);
+	pw_resource_error(resource, -ENOENT, "no such global %u", id);
 	goto exit;
       wrong_interface:
 	pw_log_debug("registry %p: global with id %u has no interface %u", resource, id, type);
-	pw_core_resource_error(client->core_resource, id,
-			     -ENOENT, "no such interface %u", type);
+	pw_resource_error(resource, -ENOENT, "no such interface %u", type);
 	goto exit;
       exit:
 	/* unmark the new_id the map, the client does not yet know about the failed
@@ -263,21 +261,18 @@ core_create_object(void *object,
 
       no_factory:
 	pw_log_error("can't find node factory %s", factory_name);
-	pw_core_resource_error(client->core_resource,
-			       new_id, -EINVAL, "unknown factory name %s", factory_name);
+	pw_resource_error(resource, -EINVAL, "unknown factory name %s", factory_name);
 	goto error;
       wrong_version:
       wrong_type:
 	pw_log_error("invalid resource type/version");
-	pw_core_resource_error(client->core_resource,
-			       new_id, -EINVAL, "wrong resource type/version");
+	pw_resource_error(resource, -EINVAL, "wrong resource type/version");
 	goto error;
       no_properties:
 	pw_log_error("can't create properties");
 	goto no_mem;
       no_mem:
-	pw_core_resource_error(client->core_resource,
-			       new_id, -ENOMEM, "no memory");
+	pw_resource_error(resource, -ENOMEM, "no memory");
 	goto error;
       error:
 	pw_map_insert_at(&client->objects, new_id, NULL);
@@ -303,7 +298,7 @@ static void core_destroy(void *object, uint32_t id)
 
       no_resource:
 	pw_log_error("can't find resouce %d", id);
-	pw_core_resource_error(client->core_resource, id, -EINVAL, "unknown resouce %d", id);
+	pw_resource_error(resource, -EINVAL, "unknown resource %d", id);
 	goto done;
 }
 

src/pipewire/global.c

@@ -265,7 +265,7 @@ pw_global_bind(struct pw_global *global, struct pw_client *client, uint32_t perm
 
       wrong_version:
 	res = -EINVAL;
-	pw_core_resource_error(client->core_resource, id,
+	pw_core_resource_errorf(client->core_resource, id,
 			res, "id %d: interface version %d < %d",
 			id, global->version, version);
 	return res;

src/pipewire/interfaces.h

@@ -29,6 +29,8 @@
 extern "C" {
 #endif
 
+#include <stdarg.h>
+
 #include <spa/utils/defs.h>
 #include <spa/node/command.h>
 #include <spa/param/param.h>
@@ -204,15 +206,19 @@ struct pw_core_proxy_events {
 	 * Fatal error event
          *
          * The error event is sent out when a fatal (non-recoverable)
-         * error has occurred. The id argument is the object where
+         * error has occurred. The id argument is the proxy object where
          * the error occurred, most often in response to a request to that
          * object. The message is a brief description of the error,
          * for (debugging) convenience.
+	 *
+	 * This event is usually also emited on the proxy object with
+	 * \a id.
+	 *
          * \param id object where the error occurred
          * \param res error code
-         * \param error error description
+         * \param message error description
 	 */
-	void (*error) (void *object, uint32_t id, int res, const char *error, ...);
+	void (*error) (void *object, uint32_t id, int res, const char *message);
 	/**
 	 * Remove an object ID
          *
@@ -221,6 +227,7 @@ struct pw_core_proxy_events {
          * this event to acknowledge that it has seen the delete request.
          * When the client receives this event, it will know that it can
          * safely reuse the object ID.
+	 *
          * \param id deleted object ID
 	 */
 	void (*remove_id) (void *object, uint32_t id);
@@ -247,6 +254,23 @@ pw_core_proxy_add_listener(struct pw_core_proxy *core,
 #define pw_core_resource_remove_id(r,...)    pw_resource_notify(r,struct pw_core_proxy_events,remove_id,__VA_ARGS__)
 #define pw_core_resource_info(r,...)         pw_resource_notify(r,struct pw_core_proxy_events,info,__VA_ARGS__)
 
+static inline void
+pw_core_resource_errorv(struct pw_resource *resource, uint32_t id, int res, const char *message, va_list args)
+{
+	char buffer[1024];
+	vsnprintf(buffer, sizeof(buffer), message, args);
+	buffer[1023] = '\0';
+	pw_core_resource_error(resource, id, res, buffer);
+}
+
+static inline void
+pw_core_resource_errorf(struct pw_resource *resource, uint32_t id, int res, const char *message, ...)
+{
+        va_list args;
+	va_start(args, message);
+	pw_core_resource_errorv(resource, id, res, message, args);
+	va_end(args);
+}
 
 #define PW_VERSION_REGISTRY			0
 
@@ -747,9 +771,9 @@ struct pw_client_proxy_methods {
 	 *
 	 * \param id the global id to report the error on
 	 * \param res an errno style error code
-	 * \param error an error string
+	 * \param message an error string
 	 */
-	void (*error) (void *object, uint32_t id, int res, const char *error);
+	void (*error) (void *object, uint32_t id, int res, const char *message);
 	/**
 	 * Update client properties
 	 *
@@ -785,9 +809,9 @@ struct pw_client_proxy_methods {
 
 /** Client permissions */
 static inline void
-pw_client_proxy_error(struct pw_client_proxy *client, uint32_t id, int res, const char *error)
+pw_client_proxy_error(struct pw_client_proxy *client, uint32_t id, int res, const char *message)
 {
-	pw_proxy_do((struct pw_proxy*)client, struct pw_client_proxy_methods, error, id, res, error);
+	pw_proxy_do((struct pw_proxy*)client, struct pw_client_proxy_methods, error, id, res, message);
 }
 
 static inline void

src/pipewire/private.h

@@ -538,6 +538,7 @@ struct pw_resource {
 
 #define pw_proxy_events_emit(o,m,v,...) spa_hook_list_call(&o->listener_list, struct pw_proxy_events, m, v, ##__VA_ARGS__)
 #define pw_proxy_events_destroy(p)	pw_proxy_events_emit(p, destroy, 0)
+#define pw_proxy_events_error(p,r,m)	pw_proxy_events_emit(p, error, 0, r, m)
 
 struct pw_proxy {
 	struct pw_remote *remote;	/**< the owner remote of this proxy */

src/pipewire/proxy.h

@@ -110,6 +110,9 @@ struct pw_proxy_events {
 
 	/** The proxy is destroyed */
         void (*destroy) (void *data);
+
+	/** an error occured on the proxy */
+        void (*error) (void *data, int res, const char *message);
 };
 
 /** Make a new proxy object. The id can be used to bind to a remote object and

src/pipewire/remote.c

@@ -99,21 +99,35 @@ pw_remote_update_state(struct pw_remote *remote, enum pw_remote_state state, con
 }
 
 
+static void core_event_error(void *data, uint32_t id, int res, const char *message)
+{
+	struct pw_remote *this = data;
+	struct pw_proxy *proxy;
+
+	pw_log_debug("remote %p: object error %u: %d (%s): %s", this, id,
+			res, spa_strerror(res), message);
+
+	proxy = pw_map_lookup(&this->objects, id);
+	if (proxy)
+		pw_proxy_events_error(proxy, res, message);
+}
+
 static void core_event_remove_id(void *data, uint32_t id)
 {
 	struct pw_remote *this = data;
 	struct pw_proxy *proxy;
 
+	pw_log_debug("remote %p: object remove %u", this, id);
 	proxy = pw_map_lookup(&this->objects, id);
-	if (proxy) {
-		pw_log_debug("remote %p: object remove %u", this, id);
+	if (proxy)
 		pw_proxy_destroy(proxy);
-	}
+
 	pw_map_remove(&this->objects, id);
 }
 
 static const struct pw_core_proxy_events core_proxy_events = {
 	PW_VERSION_CORE_PROXY_EVENTS,
+	.error = core_event_error,
 	.remove_id = core_event_remove_id,
 };
 

src/pipewire/resource.c

@@ -170,10 +170,13 @@ const struct pw_protocol_marshal *pw_resource_get_marshal(struct pw_resource *re
 }
 
 SPA_EXPORT
-void pw_resource_error(struct pw_resource *resource, uint32_t id, int result, const char *error)
+void pw_resource_error(struct pw_resource *resource, int result, const char *error, ...)
 {
-	if (resource->client->core_resource)
-		pw_core_resource_error(resource->client->core_resource, id, result, error);
+	va_list ap;
+	va_start(ap, error);
+	if (resource->client->core_resource == NULL)
+		pw_core_resource_errorv(resource->client->core_resource, resource->id, result, error, ap);
+	va_end(ap);
 }
 
 SPA_EXPORT

src/pipewire/resource.h

@@ -117,7 +117,7 @@ void pw_resource_add_override(struct pw_resource *resource,
 			      void *data);
 
 /** Generate an error for a resource */
-void pw_resource_error(struct pw_resource *resource, uint32_t id, int result, const char *error);
+void pw_resource_error(struct pw_resource *resource, int result, const char *error, ...);
 
 /** Get the implementation list of a resource */
 struct spa_hook_list *pw_resource_get_implementation(struct pw_resource *resource);

src/tests/test-interfaces.c

@@ -51,7 +51,7 @@ static void test_core_abi(void)
 	struct {
 		uint32_t version;
 	        void (*done) (void *object, uint32_t seq);
-	        void (*error) (void *object, uint32_t id, int res, const char *error, ...);
+	        void (*error) (void *object, uint32_t id, int res, const char *error);
 	        void (*remove_id) (void *object, uint32_t id);
 		void (*info) (void *object, const struct pw_core_info *info);
 	} events = { PW_VERSION_CORE_PROXY_EVENTS, };