2005-06-26 Colin Walters <walters@verbum.org>

* glib/dbus-glib.c (dbus_set_g_error): Delete.
	(dbus_g_error_set): New public function from its ashes; used by
	both service-side method implementation and GLib bindings
	internals.
	(dbus_g_error_has_name, dbus_g_error_get_name): New function.
	(_dbus_glib_test): Add some tests.

	* test/glib/test-dbus-glib.c (main): Test dbus_g_error_has_name.

	* test/glib/test-service-glib.c (my_object_throw_error): Use
	dbus_g_error_set.

	* glib/dbus-gobject.c (gerror_to_dbus_error_message): Handle
	errors thrown by dbus_g_error_set.

	* glib/dbus-gmain.c (dbus_g_bus_get): Change to dbus_g_error_set.

	* glib/dbus-gparser.c (validate_signature): Ditto.

	* glib/dbus-gproxy.c (dbus_g_proxy_new_for_name_owner)
	(dbus_g_proxy_end_call_internal): Ditto.

	* glib/Makefile.am: Generate dbus-glib-error-switch.h, which
	converts DBUS_ERROR_x to DBUS_GERROR_x.
	(libdbus_glib_1_la_SOURCES, BUILT_SOURCES, CLEANFILES): Add it.

	* doc/TODO: Remove error TODO.

	* doc/dbus-tutorial.xml: Update with documentation about error
	handling.

	* dbus/make-dbus-glib-error-enum.sh: Tighten up regexp to make
	sure we only change DBUS_ERROR to DBUS_GERROR, not all ERROR to
	GERROR.  Also add DBUS_GERROR_REMOTE_EXCEPTION.

ChangeLog

@@ -1,3 +1,40 @@
+2005-06-26  Colin Walters  <walters@verbum.org>
+
+	* glib/dbus-glib.c (dbus_set_g_error): Delete.
+	(dbus_g_error_set): New public function from its ashes; used by
+	both service-side method implementation and GLib bindings
+	internals.
+	(dbus_g_error_has_name, dbus_g_error_get_name): New function.
+	(_dbus_glib_test): Add some tests.
+
+	* test/glib/test-dbus-glib.c (main): Test dbus_g_error_has_name.
+
+	* test/glib/test-service-glib.c (my_object_throw_error): Use
+	dbus_g_error_set.
+
+	* glib/dbus-gobject.c (gerror_to_dbus_error_message): Handle
+	errors thrown by dbus_g_error_set.
+
+	* glib/dbus-gmain.c (dbus_g_bus_get): Change to dbus_g_error_set.
+
+	* glib/dbus-gparser.c (validate_signature): Ditto.
+
+	* glib/dbus-gproxy.c (dbus_g_proxy_new_for_name_owner) 
+	(dbus_g_proxy_end_call_internal): Ditto.
+
+	* glib/Makefile.am: Generate dbus-glib-error-switch.h, which
+	converts DBUS_ERROR_x to DBUS_GERROR_x.
+	(libdbus_glib_1_la_SOURCES, BUILT_SOURCES, CLEANFILES): Add it.
+
+	* doc/TODO: Remove error TODO.
+
+	* doc/dbus-tutorial.xml: Update with documentation about error
+	handling.
+
+	* dbus/make-dbus-glib-error-enum.sh: Tighten up regexp to make
+	sure we only change DBUS_ERROR to DBUS_GERROR, not all ERROR to
+	GERROR.  Also add DBUS_GERROR_REMOTE_EXCEPTION.
+
 2005-06-22  Colin Walters  <walters@verbum.org>
 	
 	Patch from Ross Burton <ross@openedhand.com>

dbus/dbus-glib.h

@@ -81,6 +81,13 @@ typedef enum
 #include <dbus/dbus-glib-error-enum.h>    
 } DBusGError;
 
+void             dbus_g_error_set         (GError     **error,
+					   const char  *name,
+					   const char  *msg);
+gboolean         dbus_g_error_has_name    (GError      *error,
+					   const char  *name);
+const char *     dbus_g_error_get_name    (GError      *error);
+
 void             dbus_g_thread_init (void);
 DBusGConnection* dbus_g_bus_get     (DBusBusType   type,
                                      GError      **error);

dbus/make-dbus-glib-error-enum.sh

@@ -11,11 +11,12 @@ die()
 }
 
 cat $SRC | grep '#define DBUS_ERROR' | sed -e 's/#define //g' | \
-  sed -e 's/".*//g' | sed -e 's/_ERROR/_GERROR/g' | sed -e 's/ *$/,/g' > $DEST.tmp
+  sed -e 's/".*//g' | sed -e 's/DBUS_ERROR/DBUS_GERROR/g' | sed -e 's/ *$/,/g' > $DEST.tmp
 
 if ! test -s $DEST.tmp ; then
     die "$DEST.tmp is empty, something went wrong, see any preceding error message"
 fi
+echo "DBUS_GERROR_REMOTE_EXCEPTION" >> $DEST.tmp
 
 echo "#ifndef DBUS_INSIDE_DBUS_GLIB_H" >> $DEST.tmp
 echo '#error "' "$DEST" 'may only be included by dbus-glib.h"' >> $DEST.tmp

doc/TODO

@@ -33,8 +33,6 @@ Important for 1.0 GLib Bindings
  - Fix TYPE_OBJECT_PATH marshalling; see:
    http://lists.freedesktop.org/archives/dbus/2005-June/002774.html
 
- - Fix errors - need to get specific error back, not UnmappedError crap
-
  - DBusGProxy doesn't emit "destroy" when it should
 
 Might as Well for 1.0

doc/dbus-tutorial.xml

@@ -772,8 +772,13 @@ main (int argc, char **argv)
   if (!dbus_g_proxy_call (proxy, "ListNames", &error, G_TYPE_INVALID,
                           G_TYPE_STRV, &name_list, G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete ListNames call: %s\n",
+      /* Just do demonstrate remote exceptions versus regular GError */
-                  error->message);
+      if (error->domain == DBUS_GERROR && error->code == DBUS_GERROR_REMOTE_EXCEPTION)
+        g_printerr ("Caught remote method exception %s: %s",
+	            dbus_g_error_get_name (error),
+	            error->message);
+      else
+        g_printerr ("Error: %s\n", error->message);
       g_error_free (error);
       exit (1);
     }
@@ -830,10 +835,60 @@ main (int argc, char **argv)
       <para>
 	You may connect to signals using
 	<literal>dbus_g_proxy_add_signal</literal> and
-	<literal>dbus_g_proxy_connect_signal</literal>.  At the
+	<literal>dbus_g_proxy_connect_signal</literal>.  You must
-	moment, <literal>dbus_g_proxy_add_signal</literal> requires
+	invoke <literal>dbus_g_proxy_add_signal</literal> to specify
-	the D-BUS types of the remote object; this will likely be
+	the signature of your signal handlers; you may then invoke
-	changed later.
+	<literal>dbus_g_proxy_connect_signal</literal> multiple times.
+      </para>
+      <para>
+	Note that it will often be the case that there is no builtin
+	marshaller for the type signature of a remote signal.  In that
+	case, you must generate a marshaller yourself by using
+	<application>glib-genmarshal</application>, and then register
+	it using <literal>dbus_g_object_register_marshaller</literal>.
+      </para>
+    </sect2>
+    <sect2 id="glib-error-handling">
+      <title>Error handling and remote exceptions</title>
+      <para>
+	All of the GLib binding methods such as
+	<literal>dbus_g_proxy_end_call</literal> return a
+	<literal>GError</literal>.  This <literal>GError</literal> can
+	represent two different things:
+      <itemizedlist>
+	<listitem>
+	  <para>
+	    An internal D-BUS error, such as an out-of-memory
+	    condition, an I/O error, or a network timeout.  Errors
+	    generated by the D-BUS library itself have the domain
+	    <literal>DBUS_GERROR</literal>, and a corresponding code
+	    such as <literal>DBUS_GERROR_NO_MEMORY</literal>.  It will
+	    not be typical for applications to handle these errors
+	    specifically.
+	  </para>
+	</listitem>
+	<listitem>
+	  <para>
+	    A remote D-BUS exception, thrown by the peer, bus, or
+	    service.  D-BUS remote exceptions have both a textual
+	    "name" and a "message".  The GLib bindings store this
+	    information in the <literal>GError</literal>, but some
+	    special rules apply.
+	  </para>
+	  <para>
+	    The set error will have the domain
+	    <literal>DBUS_GERROR</literal> as above, and will also
+	    have the code
+	    <literal>DBUS_GERROR_REMOTE_EXCEPTION</literal>.  In order
+	    to access the remote exception name, you must use a
+	    special accessor, such as
+	    <literal>dbus_g_error_has_name</literal> or
+	    <literal>dbus_g_error_get_name</literal>.  The remote
+	    exception detailed message is accessible via the regular
+	    GError <literal>message</literal> member.
+	  </para>
+	</listitem>
+      </itemizedlist>
       </para>
     </sect2>
     <sect2 id="glib-more-examples">
@@ -850,10 +905,7 @@ main (int argc, char **argv)
 			  G_TYPE_INVALID,
 			  DBUS_TYPE_G_UCHAR_ARRAY, &amp;arr, G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete Foobar: %s\n",
+      /* Handle error */
-                  error-&gt;message);
-      g_error_free (error);
-      exit (1);
     }
    g_assert (arr != NULL);
    printf ("got back %u values", arr->len);
@@ -875,10 +927,7 @@ main (int argc, char **argv)
                           DBUS_TYPE_G_STRING_STRING_HASH, hash, G_TYPE_INVALID,
 			  G_TYPE_UINT, &amp;ret, G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete HashSize: %s\n",
+      /* Handle error */
-                  error-&gt;message);
-      g_error_free (error);
-      exit (1);
     }
   g_assert (ret == 2);
   g_hash_table_destroy (hash);
@@ -899,10 +948,7 @@ main (int argc, char **argv)
                           G_TYPE_STRING, &amp;strret,
 			  G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete GetStuff: %s\n",
+      /* Handle error */
-                  error-&gt;message);
-      g_error_free (error);
-      exit (1);
     }
   printf ("%s %s", boolret ? "TRUE" : "FALSE", strret);
   g_free (strret);
@@ -933,10 +979,7 @@ main (int argc, char **argv)
 			  G_TYPE_INVALID,
 			  G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete TwoStrArrays: %s\n",
+      /* Handle error */
-                  error-&gt;message);
-      g_error_free (error);
-      exit (1);
     }
    g_strfreev (strs_dynamic);
 </programlisting>
@@ -958,10 +1001,7 @@ main (int argc, char **argv)
                           G_TYPE_STRV, &amp;strs,
 			  G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete GetStrs: %s\n",
+      /* Handle error */
-                  error-&gt;message);
-      g_error_free (error);
-      exit (1);
     }
    for (strs_p = strs; *strs_p; strs_p++)
      printf ("got string: \"%s\"", *strs_p);
@@ -983,10 +1023,7 @@ main (int argc, char **argv)
                           G_TYPE_VALUE, &amp;val, G_TYPE_INVALID,
 			  G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete SendVariant: %s\n",
+      /* Handle error */
-                  error-&gt;message);
-      g_error_free (error);
-      exit (1);
     }
   g_assert (ret == 2);
   g_value_unset (&amp;val);
@@ -1003,10 +1040,7 @@ main (int argc, char **argv)
   if (!dbus_g_proxy_call (proxy, "GetVariant", &amp;error, G_TYPE_INVALID,
                           G_TYPE_VALUE, &amp;val, G_TYPE_INVALID))
     {
-      g_printerr ("Failed to complete GetVariant: %s\n",
+      /* Handle error */
-                  error-&gt;message);
-      g_error_free (error);
-      exit (1);
     }
   if (G_VALUE_TYPE (&amp;val) == G_TYPE_STRING)
     printf ("%s\n", g_value_get_string (&amp;val));

glib/.cvsignore

@@ -6,6 +6,7 @@ Makefile.in
 *.la
 dbus-glib-test
 dbus-binding-tool
+dbus-glib-error-switch.h
 *.bb
 *.bbg
 *.da

glib/Makefile.am

@@ -4,7 +4,15 @@ INCLUDES=-I$(top_srcdir) $(DBUS_CLIENT_CFLAGS) $(DBUS_GLIB_CFLAGS) $(DBUS_GLIB_T
 
 lib_LTLIBRARIES=libdbus-glib-1.la
 
+dbus-glib-error-switch.h: $(top_srcdir)/dbus/dbus-protocol.h make-dbus-glib-error-switch.sh
+	$(srcdir)/make-dbus-glib-error-switch.sh $(top_srcdir)/dbus/dbus-protocol.h $@
+
+BUILT_SOURCES = dbus-glib-error-switch.h
+
+CLEANFILES = $(BUILT_SOURCES)
+
 libdbus_glib_1_la_SOURCES = 			\
+	dbus-glib-error-switch.h		\
 	dbus-glib.c				\
 	dbus-gmain.c				\
 	dbus-gmarshal.c				\

glib/dbus-glib.c

@@ -26,6 +26,7 @@
 #include <dbus/dbus-glib-lowlevel.h>
 #include "dbus-gtest.h"
 #include "dbus-gutils.h"
+#include <string.h>
 
 #include <libintl.h>
 #define _(x) dgettext (GETTEXT_PACKAGE, x)
@@ -148,26 +149,87 @@ dbus_g_error_quark (void)
   return quark;
 }
 
+#include "dbus-glib-error-switch.h"
 
 /**
- * Set a GError return location from a DBusError.
+ * Set a GError return location from a D-BUS error name and message.
+ * This function should only be used in the implementation of service
+ * methods.
  *
- * @todo expand the DBUS_GERROR enum and take advantage of it here
- * 
  * @param gerror location to store a GError, or #NULL
- * @param derror the DBusError
+ * @param name the D-BUS error name
+ * @param msg the D-BUS error detailed message
  */
 void
-dbus_set_g_error (GError   **gerror,
+dbus_g_error_set (GError    **gerror,
-                  DBusError *derror)
+                  const char *name,
+		  const char *msg)
 {
-  g_return_if_fail (derror != NULL);
+  int code;
-  g_return_if_fail (dbus_error_is_set (derror));
+  g_return_if_fail (name != NULL);
-  
+  g_return_if_fail (msg != NULL);
-  g_set_error (gerror, DBUS_GERROR,
+
-               DBUS_GERROR_FAILED,
+  code = dbus_error_to_gerror_code (name);
-               _("D-BUS error %s: %s"),
+  if (code == DBUS_GERROR_REMOTE_EXCEPTION)
-               derror->name, derror->message);  
+    g_set_error (gerror, DBUS_GERROR,
+		 code,
+		 "%s%c%s",
+		 msg,
+		 '\0',
+		 name);
+  else
+    g_set_error (gerror, DBUS_GERROR,
+		 code,
+		 "%s",
+		 msg);
+}
+
+/**
+ * Determine whether D-BUS error name for a remote exception matches
+ * the given name.  This function is intended to be invoked on a
+ * GError returned from an invocation of a remote method, e.g. via
+ * dbus_g_proxy_end_call.  It will silently return FALSE for errors
+ * which are not remote D-BUS exceptions (i.e. with a domain other
+ * than DBUS_GERROR or a code other than
+ * DBUS_GERROR_REMOTE_EXCEPTION).
+ *
+ * @param gerror the GError given from the remote method
+ * @param name the D-BUS error name
+ * @param msg the D-BUS error detailed message
+ * @returns TRUE iff the remote error has the given name
+ */
+gboolean
+dbus_g_error_has_name (GError *error, const char *name)
+{
+  g_return_val_if_fail (error != NULL, FALSE);
+
+  if (error->domain != DBUS_GERROR
+      || error->code != DBUS_GERROR_REMOTE_EXCEPTION)
+    return FALSE;
+
+  return !strcmp (dbus_g_error_get_name (error), name);
+}
+
+/**
+ * Return the D-BUS name for a remote exception.
+ * This function may only be invoked on a GError returned from an
+ * invocation of a remote method, e.g. via dbus_g_proxy_end_call.
+ * Moreover, you must ensure that the error's domain is DBUS_GERROR,
+ * and the code is DBUS_GERROR_REMOTE_EXCEPTION.
+ *
+ * @param gerror the GError given from the remote method
+ * @param name the D-BUS error name
+ * @param msg the D-BUS error detailed message
+ * @returns the D-BUS error name
+ */
+const char *
+dbus_g_error_get_name (GError *error)
+{
+  g_return_val_if_fail (error != NULL, NULL);
+  g_return_val_if_fail (error->domain == DBUS_GERROR, NULL);
+  g_return_val_if_fail (error->code == DBUS_GERROR_REMOTE_EXCEPTION, NULL);
+
+  return error->message + strlen (error->message) + 1;
 }
 
 /**
@@ -395,7 +457,28 @@ dbus_g_pending_call_cancel (DBusGPendingCall *call)
 gboolean
 _dbus_glib_test (const char *test_data_dir)
 {
+  DBusError err;
+  GError *gerror = NULL;
+
+  dbus_error_init (&err);
+  dbus_set_error_const (&err, DBUS_ERROR_NO_MEMORY, "Out of memory!");
+
+  dbus_g_error_set (&gerror, err.name, err.message);
+  g_assert (gerror != NULL);
+  g_assert (gerror->domain == DBUS_GERROR);
+  g_assert (gerror->code == DBUS_GERROR_NO_MEMORY);
+  g_assert (!strcmp (gerror->message, "Out of memory!"));
   
+  dbus_error_init (&err);
+  g_clear_error (&gerror);
+
+  dbus_g_error_set (&gerror, "com.example.Foo.BlahFailed", "blah failed");
+  g_assert (gerror != NULL);
+  g_assert (gerror->domain == DBUS_GERROR);
+  g_assert (gerror->code == DBUS_GERROR_REMOTE_EXCEPTION);
+  g_assert (dbus_g_error_has_name (gerror, "com.example.Foo.BlahFailed"));
+  g_assert (!strcmp (gerror->message, "blah failed"));
+
   return TRUE;
 }
 

glib/dbus-gmain.c

@@ -720,7 +720,7 @@ dbus_g_bus_get (DBusBusType     type,
   connection = dbus_bus_get (type, &derror);
   if (connection == NULL)
     {
-      dbus_set_g_error (error, &derror);
+      dbus_g_error_set (error, derror.name, derror.message);
       dbus_error_free (&derror);
       return NULL;
     }

glib/dbus-gobject.c

@@ -745,10 +745,17 @@ gerror_to_dbus_error_message (const DBusGObjectInfo *object_info,
     }
   else
     {
-      char *error_name;
+      if (error->domain == DBUS_GERROR)
-      error_name = gerror_domaincode_to_dbus_error_name (object_info, error->domain, error->code);
+	reply = dbus_message_new_error (message,
-      reply = dbus_message_new_error (message, error_name, error->message);
+					dbus_g_error_get_name (error),
-      g_free (error_name); 
+					error->message);
+      else
+	{
+	  char *error_name;
+	  error_name = gerror_domaincode_to_dbus_error_name (object_info, error->domain, error->code);
+	  reply = dbus_message_new_error (message, error_name, error->message);
+	  g_free (error_name); 
+	}
     }
   return reply;
 }

glib/dbus-gparser.c

@@ -474,7 +474,7 @@ validate_signature (const char *str,
   
   if (!dbus_signature_validate (str, &derror))
     {
-      dbus_set_g_error (error, &derror);
+      dbus_g_error_set (error, derror.name, derror.message);
       return FALSE;
     }
   return TRUE;

glib/dbus-gproxy.c

@@ -1135,7 +1135,7 @@ dbus_g_proxy_new_for_name_owner (DBusGConnection          *connection,
 
  error:
   g_assert (dbus_error_is_set (&derror));
-  dbus_set_g_error (error, &derror);
+  dbus_g_error_set (error, derror.name, derror.message);
   dbus_error_free (&derror);
 
  out:
@@ -1469,14 +1469,14 @@ dbus_g_proxy_end_call_internal (DBusGProxy       *proxy,
       break;
     case DBUS_MESSAGE_TYPE_ERROR:
       dbus_set_error_from_message (&derror, reply);
-      dbus_set_g_error (error, &derror);
+      dbus_g_error_set (error, derror.name, derror.message);
       dbus_error_free (&derror);
       goto out;
       break;
     default:
       dbus_set_error (&derror, DBUS_ERROR_FAILED,
                       "Reply was neither a method return nor an exception");
-      dbus_set_g_error (error, &derror);
+      dbus_g_error_set (error, derror.name, derror.message);
       dbus_error_free (&derror);
       goto out;
       break;

glib/make-dbus-glib-error-switch.sh

@@ -0,0 +1,29 @@
+#!/bin/sh
+
+SRC=$1
+DEST=$2
+
+die()
+{
+    echo $1 1>&2
+    /bin/rm $DEST.tmp
+    exit 1
+}
+
+echo 'static gint' > $DEST.tmp
+echo 'dbus_error_to_gerror_code (const char *derr)' >> $DEST.tmp
+echo '{' >> $DEST.tmp
+echo '  if (0) ; ' >> $DEST.tmp
+
+cat $SRC | grep '#define DBUS_ERROR' | sed -e 's/#define //g' | \
+  sed -e 's/".*//g' | \
+   (while read line; do \
+     echo '  else if (!strcmp (derr, ' "$line" ' )) '; \
+     echo '    return ' `echo $line | sed -e 's/DBUS_ERROR/DBUS_GERROR/g'` ';'; \
+    done; \
+   ) >> $DEST.tmp
+echo '  else' >> $DEST.tmp
+echo '    return DBUS_GERROR_REMOTE_EXCEPTION;' >> $DEST.tmp
+echo '}' >> $DEST.tmp
+
+mv $DEST.tmp $DEST || die "could not move $DEST.tmp to $DEST"

test/glib/test-dbus-glib.c

@@ -318,6 +318,8 @@ main (int argc, char **argv)
   error = NULL;
   if (dbus_g_proxy_end_call (proxy, call, &error, G_TYPE_INVALID) != FALSE)
     lose ("ThrowError call unexpectedly succeeded!");
+  if (!dbus_g_error_has_name (error, "org.freedesktop.DBus.Tests.MyObject.Foo"))
+    lose ("ThrowError call returned unexpected error %s", dbus_g_error_get_name (error));
 
   g_print ("ThrowError failed (as expected) returned error: %s\n", error->message);
   g_clear_error (&error);

test/glib/test-service-glib.c

@@ -237,10 +237,9 @@ my_object_increment (MyObject *obj, gint32 x, gint32 *ret, GError **error)
 gboolean
 my_object_throw_error (MyObject *obj, GError **error)
 {
-  g_set_error (error,
+  dbus_g_error_set (error,
-	       MY_OBJECT_ERROR,
+		    "org.freedesktop.DBus.Tests.MyObject.Foo",
-	       MY_OBJECT_ERROR_FOO,
+		    "this method always loses");    
-	       "this method always loses");
   return FALSE;
 }