GNOME Bugzilla – Bug 776139
Alert if error saving the password in the secret manager
Last modified: 2019-12-05 02:03:36 UTC
It might be a dev-only issue, but nevertheless here is an issue I have when running geary through another user. Basically I'm running Geary with "xhost +local && sudo su - another_user 'geary --debug'". However doing that, this custom user does not have access to various dbus objects including org.freedesktop.Secret. I am fine with that, so each time I open Geary it asks me for the user password. However if the "remember password" box is ticked, Geary will try to save it and will fail with the following error: > [msg] 15:28:12 2,317884 Remote error from secret service: org.freedesktop.Secret.Error.IsLocked: Cannot create an item in a locked collection > [deb] 15:28:12 0,000017 secret-mediator.vala:94: Unable to store password for "org.yorba.geary imap_username:account_01" in libsecret keyring And since this exception is NOT catched in secret-mediator.vala (set_password_async), it is thrown until geary controller catch it in connect_account_async. It then ends the program with: > [deb] 15:34:23 0,000074 geary-controller.vala:1117: Unable to open account Gmail:account_01: Cannot create an item in a locked collection Again, I can see various solutions: 1) Catch error in secret-mediator.vala and silently ignore it or do something else? (Error popup?) but continuing the program execution since it is a non fatal error. 2) Fix my dev setup to have dbus access. I didn't find anything so far though 3) Something else.
Created attachment 342576 [details] [review] patch proposal v1 Here is a patch attempt to fix that.
Review of attachment 342576 [details] [review]: We definitely shouldn't crash, but I don't know if popping up a dialog is great either, since like you, the user may deliberately not have the keyring available. This might be a good use case for an in-app notification (Bug 774442) in the future, but for now definitely catch the error and maybe just print debug or warning message? You could also show something on the existing status bar, but that might be a lot of work for a corner case.
Well I did a ErrorAlert with Bug 774442 in mind actually - I think it's important to inform the user that the password was not saved and that something is going wrong. But obviously as is, this popup is a bit annoying (mainly when you modify some account settings, the popup will appear twice - one for IMAP, another one for SMTP). Let's do it in 2 row then, adding a debug log for now but keeping it open until Bug 774442 is resolved?
Created attachment 343385 [details] [review] patch proposal v2
Created attachment 343386 [details] [review] patch2: display inapp notification To be applied after Bug 774442 lands.
Bumping in case you missed in Mike :).
Review of attachment 343385 [details] [review]: This also looks good, ta! Pushed to master as 8666594.
Re-titling to reflect pending patch.
Review of attachment 343386 [details] [review]: Okay, less reluctant about this but only if it is using the in-app notification (Bug 774442) framework to do so. I'll marking this as needs-work to integrate it into that and will add a dep on the bug from this one.
*** Bug 743839 has been marked as a duplicate of this bug. ***
See also Gitlab issue #30: https://gitlab.gnome.org/GNOME/geary/issues/30
As of 3.32 the controller is catching errors and reporting them using the standard mechanism to do so, so closing this as fixed.