Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fail safely if unable to get player status (Media Control) #678

Merged
merged 1 commit into from
Feb 15, 2025

Conversation

sudoAlphaX
Copy link
Contributor

@sudoAlphaX sudoAlphaX commented Feb 15, 2025

Description

In come cases where there exists a registered MPRIS player that
doesn't have support for playerstatus, safeeyes errors and doesnt
launch a break when the Media Control plugin is enabled.

This patch adds a check to the mediacontrol plugin that checks
if "PlaybackStatus" cached_property is not None, and only then attempts
to get media playing status.

Steps to reproduce:

  1. Install playerctl
  2. Launch playerctld
  3. Do not play any media (close all media playing apps)
  4. Try to launch a break using safeeyes

Error:

  File "/usr/lib/python3.13/site-packages/safeeyes/utility.py", line 106, in <lambda>
    GLib.idle_add(lambda: target_function(*args, **kwargs))
                          ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.13/site-packages/safeeyes/core.py", line 277, in __fire_start_break
    self.start_break.fire(break_obj)
    ~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
  File "/usr/lib/python3.13/site-packages/safeeyes/model.py", line 293, in fire
    if not handler(*args, **keywargs):
           ~~~~~~~^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.13/site-packages/safeeyes/safeeyes.py", line 356, in start_break
    actions = self.plugins_manager.get_break_screen_tray_actions(break_obj)
  File "/usr/lib/python3.13/site-packages/safeeyes/plugin_manager.py", line 209, in get_break_screen_tray_actions
    action = plugin['module'].get_tray_action(break_obj)
  File "/usr/lib/python3.13/site-packages/safeeyes/plugins/mediacontrol/plugin.py", line 89, in get_tray_action
    players = __active_players()
  File "/usr/lib/python3.13/site-packages/safeeyes/plugins/mediacontrol/plugin.py", line 62, in __active_players
    status = player.get_cached_property('PlaybackStatus').unpack().lower()
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'unpack'

Expected Behaviour:

Doesn't crash

In come cases where there exists a registered MPRIS player but that
doesn't have support for playerstatus, safeeyes errors and doesnt go
launch a break when the Media Control plugin is enabled.

This patch adds a check too the mediacontrol plugin such that it checks
if "PlaybackStatus" cached_property is not None, and only then attempts
to get media playing status.

Steps to reproduce:

1. Install playerctl
2. Launch playerctld
3. Do not play any media (close all media playing apps)
4. Try to launch a break using safeeyes

Error:

```
  File "/usr/lib/python3.13/site-packages/safeeyes/utility.py", line 106, in <lambda>
    GLib.idle_add(lambda: target_function(*args, **kwargs))
                          ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.13/site-packages/safeeyes/core.py", line 277, in __fire_start_break
    self.start_break.fire(break_obj)
    ~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^
  File "/usr/lib/python3.13/site-packages/safeeyes/model.py", line 293, in fire
    if not handler(*args, **keywargs):
           ~~~~~~~^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.13/site-packages/safeeyes/safeeyes.py", line 356, in start_break
    actions = self.plugins_manager.get_break_screen_tray_actions(break_obj)
  File "/usr/lib/python3.13/site-packages/safeeyes/plugin_manager.py", line 209, in get_break_screen_tray_actions
    action = plugin['module'].get_tray_action(break_obj)
  File "/usr/lib/python3.13/site-packages/safeeyes/plugins/mediacontrol/plugin.py", line 89, in get_tray_action
    players = __active_players()
  File "/usr/lib/python3.13/site-packages/safeeyes/plugins/mediacontrol/plugin.py", line 62, in __active_players
    status = player.get_cached_property('PlaybackStatus').unpack().lower()
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'unpack'
```

Expected Behaviour:

Doesn't crash
Copy link
Collaborator

@deltragon deltragon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can tell, the specification does require this property - but I might be reading that wrong, and either way, it doesn't hurt to be more crash-resilient.
LGTM, thanks for the PR.

@deltragon deltragon merged commit 6a0be4a into slgobinath:master Feb 15, 2025
1 check passed
@sudoAlphaX
Copy link
Contributor Author

That's what even I thought, but it seems something is weird with playerctl causing it to be a MPRIS player.

The problem is, it doesnt show up in playerctl status or `playerctl metdata', leading one to believe it is not a player. But applications (specifically python, havent tested with others) report playerctld as a player.

Trackma is a list tracker application I use written in python. Here is what it reports:

image

I will check further why playerctld is doing something like this.

Thanks for merging btw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants