unexpected callback argument with dbus error

[pabigot]

With 0.2.3 invoking service.getInterface(a, b, (err, value) => {...}) in a case where the service doesn't exist returns a message like this:

{
serial: 3,
destination: ':1.398',
errorName: 'org.freedesktop.DBus.Error.ServiceUnknown',
replySerial: 2,
signature: 's',
sender: 'org.freedesktop.DBus',
type: 3,
flags: 1,
body: [ 'The name net.connman was not provided by any .service files' ]
}

This results in the callback being passed an err argument that is the value of msg.body, i.e. an array with a string member.

Is this the intended behavior? I would expect (want) an Error instance, preferably with the msg errorName property preserved, as with:

diff --git a/lib/bus.js b/lib/bus.js
index c2d6b47..b54722d 100644
--- a/lib/bus.js
+++ b/lib/bus.js
@@ -112,7 +112,9 @@ module.exports = function bus(conn, opts) {
                  args = [null].concat(args); // first argument - no errors, null
                  handler.apply(props, args); // body as array of arguments
               } else {
-                 handler.call(props, args);  // body as first argument
+                 var err = new Error(args[0]);
+                 err.errorName = msg.errorName;
+                 handler.call(props, err);  // error as first argument
               }
            }
        } else if (msg.type == constants.messageType.signal) {

[pabigot]

Or, to handle #178:

var err = new Error(args[0] || msg.errorName);
err.errorName = msg.errorName;
handler.call(props, err);  // error as first argument

[pabigot]

FWIW, I see no way to fix this without breaking API. If a method invocation fails, the msg.type will be constants.messageType.error rather than ...methodReturn. Since dbus-native has chosen to only follow the error-as-first-argument callback convention when the type is methodReturn the error can't be conveyed without a patch like the following:

diff --git a/lib/bus.js b/lib/bus.js
index b42d229..db838b3 100644
--- a/lib/bus.js
+++ b/lib/bus.js
@@ -106,8 +106,12 @@ module.exports = function bus(conn, opts) {
         if (msg.type === constants.messageType.methodReturn) {
           args = [null].concat(args); // first argument - no errors, null
           handler.apply(props, args); // body as array of arguments
+        } else if (msg.type === constants.messageType.error) {
+          var error = new Error(msg.errorName || 'unspecified D-Bus error');
+          error.body = msg.body;
+          handler.call(props, error); // error as first argument
         } else {
-          handler.call(props, args); // body as first argument
+          handler.call(props, null, args); // body as second argument
         }
       }
     } else if (msg.type === constants.messageType.signal) {

[sidorares]

Hey @pabigot happy to go with this change ( probably with major version bump )

[pabigot]

@sidorares do you want a PR for this or do you have it covered? What's your timeframe for a release; I'd like to rework my applications to the new API and test so any similar callback problems can be identified and fixed too, but if it's months out I have other things to do now.