Repository: godbus/dbus (go)

Issue:
Export should allow the export of methods that do not end in dbus.Error
One may wish to be able to export objects from packages that do not import the 'dbus' library, and whose methods do not use the dbus.Error return type. When exporting these functions, a wrapper can be generated that will return dbus.Error for them for D-Bus specific failures, and will encapsulate the existing 'error' return if it exists and is the last return from a method. If an 'error' is not returned then the wrapper should only encapsulate D-Bus specific failures.

There is some risk in doing this as it would be a subtle API change that could cause unintended methods to be exported if one is using dbus.Error as a flag that this method should be exported on D-Bus. My suggestion would be to have an 'ExportAll' variant for exporting all methods from an object that are exported to packages.

If no one has any objections I will get this implemented.


Pull request:
Add ExportAll option so objects can ignore *Error requirement
I allowed developers to end with `error` so they can still return errors if that is desirable.

Fixes #55 

Patch:
diff --git a/dbus.go b/dbus.go
index 3d758f1..ddf3b7a 100644
--- a/dbus.go
+++ b/dbus.go
@@ -28,6 +28,7 @@ var (
 	interfaceType   = reflect.TypeOf((*interface{})(nil)).Elem()
 	unixFDType      = reflect.TypeOf(UnixFD(0))
 	unixFDIndexType = reflect.TypeOf(UnixFDIndex(0))
+	errType         = reflect.TypeOf((*error)(nil)).Elem()
 )
 
 // An InvalidTypeError signals that a value which cannot be represented in the
diff --git a/default_handler.go b/default_handler.go
index 6d8bf32..13132c6 100644
--- a/default_handler.go
+++ b/default_handler.go
@@ -126,14 +126,28 @@ func (m exportedMethod) Call(args ...interface{}) ([]interface{}, error) {
 	}
 
 	ret := m.Value.Call(params)
-
-	err := ret[t.NumOut()-1].Interface().(*Error)
-	ret = ret[:t.NumOut()-1]
+	var err error
+	nilErr := false // The reflection will find almost-nils, let's only pass back clean ones!
+	if t.NumOut() > 0 {
+		if e, ok := ret[t.NumOut()-1].Interface().(*Error); ok { // godbus *Error
+			nilErr = ret[t.NumOut()-1].IsNil()
+			ret = ret[:t.NumOut()-1]
+			err = e
+		} else if ret[t.NumOut()-1].Type().Implements(errType) { // Go error
+			i := ret[t.NumOut()-1].Interface()
+			if i == nil {
+				nilErr = ret[t.NumOut()-1].IsNil()
+			} else {
+				err = i.(error)
+			}
+			ret = ret[:t.NumOut()-1]
+		}
+	}
 	out := make([]interface{}, len(ret))
 	for i, val := range ret {
 		out[i] = val.Interface()
 	}
-	if err == nil {
+	if nilErr || err == nil {
 		//concrete type to interface nil is a special case
 		return out, nil
 	}
diff --git a/export.go b/export.go
index c277ab1..1093b01 100644
--- a/export.go
+++ b/export.go
@@ -69,6 +69,22 @@ func getMethods(in interface{}, mapping map[string]string) map[string]reflect.Va
 	return methods
 }
 
+func getAllMethods(in interface{}, mapping map[string]string) map[string]reflect.Value {
+	if in == nil {
+		return nil
+	}
+	methods := make(map[string]reflect.Value)
+	val := reflect.ValueOf(in)
+	typ := val.Type()
+	for i := 0; i < typ.NumMethod(); i++ {
+		methtype := typ.Method(i)
+		method := val.Method(i)
+		// map names while building table
+		methods[computeMethodName(methtype.Name, mapping)] = method
+	}
+	return methods
+}
+
 func standardMethodArgumentDecode(m Method, sender string, msg *Message, body []interface{}) ([]interface{}, error) {
 	pointers := make([]interface{}, m.NumArguments())
 	decode := make([]interface{}, 0, len(body))
@@ -247,6 +263,18 @@ func (conn *Conn) Export(v interface{}, path ObjectPath, iface string) error {
 	return conn.ExportWithMap(v, nil, path, iface)
 }
 
+// ExportAll registers all exported methods defined by the given object on
+// the message bus.
+//
+// Unlike Export there is no requirement to have the last parameter as type
+// *Error. If you want to be able to return error then you can append an error
+// type parameter to your method signature. If the error returned is not nil,
+// it is sent back to the caller as an error. Otherwise, a method reply is
+// sent with the other return values as its body.
+func (conn *Conn) ExportAll(v interface{}, path ObjectPath, iface string) error {
+	return conn.export(getAllMethods(v, nil), path, iface, false)
+}
+
 // ExportWithMap works exactly like Export but provides the ability to remap
 // method names (e.g. export a lower-case method).
 //


Test patch:
diff --git a/export_test.go b/export_test.go
index ce13a8d..36a205c 100644
--- a/export_test.go
+++ b/export_test.go
@@ -34,6 +34,24 @@ func (export badExport) Foo(param string) string {
 	return "bar"
 }
 
+type errorExport struct {
+	message Message
+}
+
+func (export *errorExport) Run(message Message, param string) (string, error) {
+	export.message = message
+	return "pass", nil
+}
+
+type noErrorExport struct {
+	message Message
+}
+
+func (export *noErrorExport) Run(message Message, param string) (string) {
+	export.message = message
+	return "cool"
+}
+
 // Test typical Export usage.
 func TestExport(t *testing.T) {
 	connection, err := ConnectSessionBus()
@@ -73,6 +91,64 @@ func TestExport(t *testing.T) {
 	}
 }
 
+// Test that Exported handlers can use a go error type.
+func TestExport_goerror(t *testing.T) {
+	connection, err := ConnectSessionBus()
+	if err != nil {
+		t.Fatalf("Unexpected error connecting to session bus: %s", err)
+	}
+	defer connection.Close()
+
+	name := connection.Names()[0]
+
+	export := &errorExport{}
+	connection.ExportAll(export, "/org/guelfey/DBus/Test", "org.guelfey.DBus.Test")
+	object := connection.Object(name, "/org/guelfey/DBus/Test")
+
+	var response string
+	err = object.Call("org.guelfey.DBus.Test.Run", 0, "qux").Store(&response)
+	if err != nil {
+		t.Errorf("Unexpected error calling Foo: %s", err)
+	}
+
+	if response != "pass" {
+		t.Errorf(`Response was %s, expected "foo"`, response)
+	}
+
+	if export.message.serial == 0 {
+		t.Error("Expected a valid message to be given to handler")
+	}
+}
+
+// Test that Exported handlers can have no error.
+func TestExport_noerror(t *testing.T) {
+	connection, err := ConnectSessionBus()
+	if err != nil {
+		t.Fatalf("Unexpected error connecting to session bus: %s", err)
+	}
+	defer connection.Close()
+
+	name := connection.Names()[0]
+
+	export := &noErrorExport{}
+	connection.ExportAll(export, "/org/guelfey/DBus/Test", "org.guelfey.DBus.Test")
+	object := connection.Object(name, "/org/guelfey/DBus/Test")
+
+	var response string
+	err = object.Call("org.guelfey.DBus.Test.Run", 0, "qux").Store(&response)
+	if err != nil {
+		t.Errorf("Unexpected error calling Foo: %s", err)
+	}
+
+	if response != "cool" {
+		t.Errorf(`Response was %s, expected "foo"`, response)
+	}
+
+	if export.message.serial == 0 {
+		t.Error("Expected a valid message to be given to handler")
+	}
+}
+
 // Test that Exported handlers can obtain raw message.
 func TestExport_message(t *testing.T) {
 	connection, err := ConnectSessionBus()