dbus2 / zbus-old

Rust D-Bus crate.
https://gitlab.freedesktop.org/dbus/zbus
Other
49 stars 13 forks source link

dbus_interface requires zbus::fdo::Error #165

Closed zeenix closed 1 year ago

zeenix commented 3 years ago

In GitLab by @swsnr on Apr 14, 2021, 08:04

I tried to implement a DBus interface with the dbus_interface macro, and then return custom errors. I declared

    use zbus::DBusError;

    #[derive(DBusError, Debug)]
    #[dbus_error(prefix = "foo.Error")]
    pub enum Error {
        ZBus(zbus::Error),
        ThingsFailed(String),
    }

    pub type Result<T> = std::result::Result<T, Error>;

But attempting to use the Result as return type for a method inside a dbus_interface impl made the compiler complain about a missing From from Error to zbus::fdo::Error.

When adding that From the code compiles but the custom error type is lost; returning an error from my method ends up in a org.freedesktop.DBus.Error instead of my custom error name.

I was assuming it ought to work because Interface::call returns a generic zbus::Error, but I wasn't able to make it work, so do I understand correctly that currently we can't use custom errors with dbus_interface?

If not what am I doing wrong?

zeenix commented 3 years ago

so do I understand correctly that currently we can't use custom errors with dbus_interface?

Seems that's unfortunately the case currently. You could have used zbus::Error::MethodError (either directly or impl. a From<YourError> to zbus::Error using it) but seems @elmarco designed that for client-side only and hence it expects the Message (the error was created from) as well. I think I'll change that but since that would be an API break, it'll have to be part of the 2.0 release (which is currently in beta stages).

For the moment, see if you can make use of the fdo errors, there is plenty of them and most often there is one that would work for everyone.

zeenix commented 3 years ago

In GitLab by @swsnr on Apr 14, 2021, 18:55

Never mind :slight_smile: FDO errors are fine for my use case; I was really just wondering if I had done anything wrong.

As far as I am concerned you can close this issue :slight_smile:

zeenix commented 3 years ago

Glad to hear, they really do cover quite a lot of cases so I'm not surprised.

Having said that, I still think we should support the custom error types in interface implementations.

zeenix commented 3 years ago

In GitLab by @rosslannen on Aug 17, 2021, 20:33

Is there a timeline or an implementation plan for this feature being added into the beta releases? This is something that is needed for a project I am working on.

The project is implementing a GATT service using the BlueZ D-Bus interface. The APIs involve implementing the org.bluez.GattCharacteristic1 interface, and then exporting those objects. According to the API, the only errors permitted to send are:

Each of these correspond to a specific ATT error code (defined in the bluetooth spec) that is sent to the bluetooth client. The spec we are implementing the service to requires us to send back the ATT error code that matches org.bluez.Error.Failed (0x80) on certain error conditions, but there is no way with zbus currently to do this. All of the fdo::Error variants would trigger a ATT_ERROR_READ_NOT_PERMITTED (0x02) or ATT_ERROR_WRITE_NOT_PERMITTED (0x03) code to be sent, which does not work for our client application.

We are currently using v2.0.0-beta.3 for our application. Is there a way on this version to hack around this issue and get the correct error values sent? I have tried already using the zbus::Error::MethodError above with a dummy message value, but it looks like it panics when the zbus::fdo::Error::ZBus variant is encountered on the zbus::fdo::Error::reply() method.

If there is another way to temporarily get around this issue, it would be greatly appreciated.

Also, as a possible implementation change for future beta releases - could the generated call() method for interfaces directly use the error type returned from the method and call reply() on that error, rather than converting to a zbus::fdo::Error first? That would make it compatible with any error creating using the DBusError derive. If this is something you would like to see I would love to contribute, and could have a PR up within the week.

zeenix commented 3 years ago

Is there a timeline or an implementation plan for this feature being added into the beta releases?

Not really, sorry.

This is something that is needed for a project I am working on.

Thanks for letting us know so we can privatize it better. :)

Is there a way on this version to hack around this issue and get the correct error values sent?

Providing a From<YourError> for zbus::Error doesn't help?

I have tried already using the zbus::Error::MethodError above with a dummy message value, but it looks like it panics when the zbus::fdo::Error::ZBus variant is encountered on the zbus::fdo::Error::reply() method.

Oh, it'd be interesting to know more, even though zbus::Error::MethodError was merged into zbus::Error in the last 2.0 beta (2.0.0-beta.6).

If there is another way to temporarily get around this issue, it would be greatly appreciated.

I'm not sure. If providing a conversion from your error to zbus::Error doesn't help, I'm out of ideas for workarounds. :(

Also, as a possible implementation change for future beta releases - could the generated call() method for interfaces directly use the error type returned from the method and call reply() on that error, rather than converting to a zbus::fdo::Error first? That would make it compatible with any error creating using the DBusError derive. If this is something you would like to see I would love to contribute, and could have a PR up within the week.

Yeah, that would be good and interestingly @danieldg recently added some utility API that I think should make this possible. Right @danieldg?

zeenix commented 3 years ago

In GitLab by @danieldg on Aug 18, 2021, 01:49

I don't think I added anything for this dealing with the Interface code.

zeenix commented 3 years ago

@danieldg I was referring to the ResultAdapter so users can use their own Result types but I guess that's not relevant here.

zeenix commented 3 years ago

mentioned in commit 3ad964b262e05d37ce6144d20d7e61a581ee7eff