maximbaz / yubikey-touch-detector

A tool to detect when your YubiKey is waiting for a touch (to send notification or display a visual indicator on the screen)
ISC License
415 stars 31 forks source link

Notification can't be closed on qtile. #33

Closed ftpd closed 2 years ago

ftpd commented 2 years ago

When I use the detector under qtile, it works fine, but it can't close the notification. The error is:

ERRO[2022-11-06T16:47:36+01:00] Cannot close notification: The service interface raised an error: list index out of range.
['  File "/f/.local/share/virtualenvs/qtile/lib/python3.10/site-packages/dbus_next/message_bus.py", line 712, in _process_message\n    handler(msg, send_reply)\n', '  File "/f/.local/share/virtualenvs/qtile/lib/python3.10/site-packages/dbus_next/message_bus.py", line 731, in handler\n    result = method.fn(interface, *args)\n', '  File "/f/.local/share/virtualenvs/qtile/lib/python3.10/site-packages/libqtile/notify.py", line 88, in CloseNotification\n    self.manager.close(nid)\n', '  File "/f/.local/share/virtualenvs/qtile/lib/python3.10/site-packages/libqtile/notify.py", line 203, in close\n    notif = self.notifications[nid]\n']

The file mentioned in the stacktrace is https://github.com/qtile/qtile/blob/master/libqtile/widget/notify.py.

My knowledge about python (and coding in general) is very limited, so I would like to ask if you see any way it can be a detector's fault, or should I go directly to the qtile team?

maximbaz commented 2 years ago

It's a bit hard to say whether there's an actual bug in yubikey-touch-detector that's involved, but qtile team would most probably want to make their project less resilient to such crashes, so it might be worth reporting to them in any case.

Here's what happens: when a "close notification" request is being processed, their code tries to find the corresponding notification by its index. It is possible that the notification with such index cannot be found, maybe because a user closed it by hand, or maybe because a wrong index is provided. Right now they don't handle such case, and so the app crashes. It should be trivial to check if such notification exists, and handle it in one or another way (maybe simply ignore, maybe log a warning).

Back to this tool, what do you experience when you see this issue? Was notification already closed when yubikey-touch-detector tries to close it (manually by you, or auto-closed on some timeout?). Does notification remain visible all the way until qtile crashes?

ftpd commented 2 years ago

The notification was not closed. I've set the timeout of the Notifier widget to 10 minutes on purpose to check it. After touching the yubiky and getting the error, notification is still there: https://imgur.com/a/IGfyHhQ, see the top right corner.

When I use dunst, the notification is handled properly - it closes when it should.

maximbaz commented 2 years ago

Okay, thanks for details, given that on your specific setup you can see that dunst has no problem, it is likely that qtile, in addition to handling the case explained above, would also want to at least investigate what is wrong with closing a notification (if they only fix the case above, then it won't crash anymore, but notification would probably remain open). We use dbus to request showing and hiding a notification, you can refer them to this code snippet if necessary: https://github.com/maximbaz/yubikey-touch-detector/blob/main/notifier/libnotify.go

ftpd commented 2 years ago

One important thing: qtile doesn't crash. There is no mention of the whole situation in the qtile's log, everything works fine - except the notification is not closed.

ftpd commented 2 years ago

Confirmed, it was a thing on the qtile's side. It's now fixed in one PR and (hopefully) will be merged to master soon.

Thanks for your help, @maximbaz (and thanks for the great tool which makes my life a lot easier!), I'm closing the issue.