slgobinath / SafeEyes

Protect your eyes from eye strain using this simple and beautiful, yet extensible break reminder
http://slgobinath.github.io/SafeEyes/
GNU General Public License v3.0
1.41k stars 159 forks source link

[2.1.7, 2.1.8, Git `master`] Plugin `smartpause` calls `disable_safeeyes` with unsupported signature, causing display of a backtrace #580

Closed hartwork closed 1 week ago

hartwork commented 2 weeks ago

Describe the bug I witnessed this backtrace at runtime:

Exception in thread WorkThread:
Traceback (most recent call last):
  File "/usr/lib/python3.10/threading.py", line 1016, in _bootstrap_inner
    self.run()
  File "/usr/lib/python3.10/threading.py", line 953, in run
    self._target(*self._args, **self._kwargs)
  File "/usr/lib/python3.10/site-packages/safeeyes/plugins/smartpause/plugin.py", line 197, in __start_idle_monitor
    disable_safeeyes(None, True)
TypeError: SafeEyes.__init__.<locals>.<lambda>() takes 1 positional argument but 2 were given

The suppose cause is that code… https://github.com/slgobinath/SafeEyes/blob/5cd9b8a428f79b6c4041c9a7321ae24fda5e65ce/safeeyes/safeeyes.py#L74-L75 …only supports a single argument while… https://github.com/slgobinath/SafeEyes/blob/5cd9b8a428f79b6c4041c9a7321ae24fda5e65ce/safeeyes/safeeyes.py#L242 …supports more.

Could be a regression from commit 903d407faf58871fd84756ec76d6b5c2543f6540 .

CC @AdamPS

To Reproduce Steps to reproduce the behavior:

  1. Install 2.1.8
  2. Run safeeyes from the terminal
  3. Wait a bit
  4. See the backtrace above

Expected behavior No backtrace

Desktop (please complete the following information):

Debug Log n/a

Screenshots n/a

archisman-panigrahi commented 2 weeks ago

Is this issue present in releases with python 3.12? Or is it python version independent. For maximum stability, I am planning to only include the new version of safe eyes (2.1.8 or later) for Ubuntu 24.04 or later. For previous versions of Ubuntu, Safe Eyes is already available on the official repositories.

archisman-panigrahi commented 2 weeks ago

How to reproduce this crash?

archisman-panigrahi commented 2 weeks ago

I ran safeeyes in terminal in Kubuntu 24.04, and when I enable smartpause, safeeyes prints the following in the terminal, but it does not crash.

screen saver extension not supported

I am using KDE 5.27 with Wayland.

deltragon commented 2 weeks ago

Oh, I actually ran into this issue in https://github.com/slgobinath/SafeEyes/pull/576 - the first commit there (https://github.com/slgobinath/SafeEyes/pull/576/commits/7c241f1292ddec6f17b4a50ae38ff2bc79703ab6) contains a fix. I will pull it out into a separate PR today in the evening.

deltragon commented 2 weeks ago

I ran safeeyes in terminal in Kubuntu 24.04, and when I enable smartpause, safeeyes prints the following in the terminal, but it does not crash.

screen saver extension not supported

I am using KDE 5.27 with Wayland.

This needs to be tested on either GNOME or Sway Wayland, or on X11 - KDE Wayland currently does not support smartpause at all, really. (It uses xprintidle, which ends up always being idle, effectively.) This is what I am trying to fix in https://github.com/slgobinath/SafeEyes/pull/576.

(The message about screen saver not supported is unrelated, it's from the Do Not Disturb plugin.)

hartwork commented 2 weeks ago

How to reproduce this crash?

@archisman-panigrahi quoting myself from above:

To Reproduce Steps to reproduce the behavior:

  1. Install 2.1.8

  2. Run safeeyes from the terminal

  3. Wait a bit

  4. See the backtrace above

The smartpause plug-in needs to be enabled of course. And I think not doing anything else in that time helps if the plugin wants to disable SafeEyes only when the machine is idle. My understanding of the plugin is superficial at the moment, though.

hartwork commented 2 weeks ago

Oh, I actually ran into this issue in #576 - the first commit there (7c241f1) contains a fix. I will pull it out into a separate PR today in the evening.

@deltragon cool, thank you!

deltragon commented 2 weeks ago

I have a fix up in https://github.com/slgobinath/SafeEyes/pull/582. It would be good if you could test it, and confirm it resolves the issue.

AdamPS commented 3 days ago

Sorry for creating the problem and not responding earlier. I have only just found time to come back to safeeyes and I'm glad that someone has fixed it many thanks.