added checks for system-only DBus #4
Conversation
|
|
| DBusConnection conn = null; | ||
| try { | ||
| conn = DBusConnection.getConnection(DBusConnection.DBusBusType.SESSION); | ||
| } catch (RuntimeException e) { |
There was a problem hiding this comment.
DBusConnection.getConnection throws a DBusException doesn't it? I'd suggest to catch that instead.
There was a problem hiding this comment.
It can throw a DBusException on certain errors but in the case of a non-existent session DBus it is only throwing a RuntimeException as per this line
Caused by: java.lang.RuntimeException: Cannot Resolve Session Bus Address
Using a DBusException in place of it results in the same error as before and the catch is never executed. Maybe I should catch both, though?
There may be a better way to check before the exception is thrown... Let me look into it a bit more.
EDIT: I added in a more specific check in the new push and prettied the code up a bit, trying to conform to the other code. I also took out the stack trace output for the missing session DBus, as it isn't necessary when handled correctly. Let me know what your thoughts are on it.
EDIT 2: Sorry to keep editing, but looking at the source for dbus-java it should be throwing a DBusException. I'm not sure why it isn't in this case. I'm going to try to compile it myself and see how it handles.
| @@ -18,7 +18,7 @@ public boolean isSupported() { | |||
| try (@SuppressWarnings("unused") SimpleCollection keyring = new SimpleCollection()) { | |||
| // seems like we're able to access the keyring. | |||
| return true; | |||
| } catch (IOException | RuntimeException e) { | |||
| } catch (IOException | ExceptionInInitializerError | RuntimeException e) { | |||
There was a problem hiding this comment.
Why did you add the ExceptionInInitializerError here? I think secret-service throws an IOException if something goes wrong?
There was a problem hiding this comment.
That's based upon the actual code running on my setup. With the Session DBus non-existent it threw an ExceptionInInitializerError upon running, so I just added it to the list so it marks it as not supported. Without that, it crashes similarly to the original issue, but with SecretService in the backtrace.
|
A follow up on the DBusException. It seems the version used in cryptomator is https://github.com/hypfvieh/dbus-java and for whatever reason, this version throws a RuntimeException instead of a DBusExcepiton when throwing "Cannot Resolve Session Bus Address" From line 256-258 in org.freedesktop.dbus.connections.impl.DBusConnection That also means the issue might just be a missing address file or misconfigured dbus for the other user, instead of not having a session dbus, but this pull request should still fix the issue either way. |
|
I released the secret-service 1.3.0, which should address this issue. |
|
@swiesend I tested your pull request and it definitely didn't fix the issue on my system. I tried it straight from your repo and with only my changes to KDEWallet, but without checking for the "ExceptionInInitializerError" I still get |
|
I opened an issue in the upstream dbus-java library regarding the RuntimeException: The next release contains a change such that the static methods do not throw it anymore :D @swiesend @purejava It might be worthwile checking, if that version can be used in your libraries but the maintainer mentions possible breaking changes. |
I run a Linux distribution that doesn't implement a session DBus, but does have a system-wide DBus. This change simply checks if the DBus connection fails and tries to connect to the system DBus. The ExceptionInInitializerError exception is also thrown by the SecretServiceKeychain because of this. I added that check to notify that it isn't supported.
It fixes this issue for me cryptomator/cryptomator#1479