Error caused by dbus.Error with invalid name is unhandled

[alexg-axis]

I recently debugged an issue where a client would never receive a response from a method call if said method call returned a dbus.Error. The client would basically just time out.

I attached a debugger and started looking for the cause and found this:

1. An error is returned when encoding the error

dbus/transport_unix.go

Line 204 in e523abc

return err

2. The error bubbles

dbus/conn.go

Line 807 in e523abc

return h.conn.SendMessage(msg)

3. The error bubbles

dbus/conn.go

Line 495 in e523abc

err := conn.outHandler.sendAndIfClosed(msg, ifClosed)

4. The error bubbles

dbus/conn.go

Line 503 in e523abc

func (conn *Conn) handleSendError(msg *Message, err error) {

5. The error is silently dropped

dbus/conn.go

Line 511 in e523abc

conn.serialGen.RetireSerial(msg.serial)

The cause was that the error's name was not a valid name. It was akin to failed to perform method rather than org.freedesktop.DBus.Error.InvalidArgs.

For v6 I suggest for that validation to happen in dbus.NewError, with an error returned if it fails. But for v5 it would be really nice to surface this error one way or another.

[guelfey]

Fair point. The existing code only generates an error reply on FormatError, but the validity check in validateHeader actually returns an InvalidMessageError...

For this specific case, this is straightforward to fix (add to the type switch) such that at least the client receives a reply. But indeed we also had other issues where the "server layer" of the library had no straightforward way to communicate to the application code of the server that it was doing something wrong. In a possible v6 I would anyway try to find a clearer separation between the "high-level" code (the DefaultHandler etc.) and the low-level code that performs the actual message encoding/decoding.