hypfvieh / dbus-java

Improved version of java DBus library provided by freedesktop.org (https://dbus.freedesktop.org/doc/dbus-java/)
https://hypfvieh.github.io/dbus-java/
MIT License
185 stars 73 forks source link

DBus.getConnection() throws RuntimeException #128

Closed infeo closed 3 years ago

infeo commented 3 years ago

This is a question.

To establish a connection to the DBus, one has to call DBus.getConnection(DBusBusType _bustype) throws DBusException (or one its sibling methods.

Within this method, at two spots a RuntimeExeption is thrown instead of the expected DBusException: https://github.com/hypfvieh/dbus-java/blob/7f6d908d0cb55e26336624c0cc0d88c3c8877977/dbus-java/src/main/java/org/freedesktop/dbus/connections/impl/DBusConnection.java#L259-L261 and https://github.com/hypfvieh/dbus-java/blob/7f6d908d0cb55e26336624c0cc0d88c3c8877977/dbus-java/src/main/java/org/freedesktop/dbus/connections/impl/DBusConnection.java#L269-L271

My question is now: Why? There is already an expected/ checked exception which could be used, namely the DBusException. This leads to errors in downstream libraries not anticipating the undocumented exceptions.

hypfvieh commented 3 years ago

The reason is, that the method passes Supplier<String> to another method. These Suppliers are created using a Lambda expression, which are not allowed to throw any checked exception.

I agree that this is not a really good solution. I will change that in the next version, removing the Supplier stuff (does not help or make any sense here at all). I will then throw a DBusConnectionException which is a new exception derived from DBusException, so any existing code should work fine.

overheadhunter commented 3 years ago

The reason is, that the method passes Supplier<String> to another method. These Suppliers are created using a Lambda expression, which are not allowed to throw any checked exception.

Just fyi, this is not true.

@FunctionalInterface
interface ThrowingSupplier<T> {
    T get() throws Exception;
}

// ...

ThrowingSupplier<String> foo = () -> { throw new Exception(); } // runs just fine
hypfvieh commented 3 years ago

Yes it would be allowed, if it would use a custom ThrowingSupplier, but it doesn't. Therefore, throwing anything else than RuntimeException is not possible.

Anyways, it also doesn't make any sense to me to comment on already fixed issues. As you might have seen I removed the Supplier stuff completely, so no need for another solution here

overheadhunter commented 3 years ago

if it would use a custom ThrowingSupplier, but it doesn't

It was only used in a private method, therefore a custom supplier would be easy to use.

Anyways, it also doesn't make any sense to me to comment on already fixed issues. As you might have seen I removed the Supplier stuff completely, so no need for another solution here

I saw it the moment I checked out the code. If the issue had been closed, I would have known this is already fixed. So choose your words carefully here. Didn't mean to hurt your feelings...