raise ClickException when OAuthRefreshException occurs during watch command

[simonc56]

Catch trakt OAuthRefreshException during the watch command and terminates the process with Critical error.
Before this PR, the script would continue trying to scrobble with invalid oauth token and user had to check logs to see if auth error happened and manually restart script or container. Now, it terminates automatically because this error is Critical and docker container can be restarted by restart policy.

To be sure we raise the error, a watch FatalErrorState is shared in the EventDispatcher and the BackgroundTask. The scrobble actions are executed in background with the dispatcher so when they raise an OAuthRefreshException it could not be catched normally in watch.py. Hence the centralized fatal error state. Maybe it's overkill, any better suggestion is welcome.

I was helped by GPT-5.4 AI to code this feature (but not to write this PR message 😉).
I tested it and the main objective is reached : PlexTraktSync terminates with CRITICAL error when refresh token fails during watch command running.

INFO     OAuth token has expired, refreshing now...                                                                                                                                                   
ERROR    400 - Unable to refresh expired OAuth token (invalid_grant) The provided authorization grant is invalid, expired, revoked, does not match the redirection URI used in the authorization      
         request, or was issued to another client.                                                                                                                                                    
ERROR    AlertListener Msg Error: Unauthorized - OAuth token refresh failed: invalid_grant - The provided authorization grant is invalid, expired, revoked, does not match the redirection URI used in
         the authorization request, or was issued to another client.                                                                                                                                  
CRITICAL Error running watch command: Trakt error: Unable to refresh token: Unauthorized - OAuth token refresh failed: invalid_grant - The provided authorization grant is invalid, expired, revoked, 
         does not match the redirection URI used in the authorization request, or was issued to another client.

This PR needs this pytrakt PR merged too :

• raise OAuthRefreshException when refresh token fails glensc/python-pytrakt#102

PS: When the python process stops with ClickException, the docker container sees the main process finished with code 1. Thus, container manager can restart the container if needed and eventually a new valid oauth token can be read from .pytrakt.json file (if updated by another PTS instance).

fixes #2323

[glensc]

046fbef commit message doesn't match the changes in it.

the commit should be split to two based on their changes.

[glensc]

idk. this sounds, so wrong. background task has channels, so send your messages to it, if the problem you're solving is from two different processes.

But I don't have capacity right now to do this myself.

[Copilot]

Pull request overview

This PR aims to make the watch command terminate when Trakt token refresh fails (OAuthRefreshException), instead of continuing to run and repeatedly failing scrobbles until manually restarted. It introduces a shared “fatal error” state used to propagate OAuth refresh failures from background/event-dispatch contexts back to the main watch loop, and converts the failure into a CLI-facing error.

Changes:

• Add a thread-safe FatalErrorState and plumb it through the watch WebSocket listener, event dispatcher, and background task runner.
• Record OAuthRefreshException in background/event contexts and re-surface it in the main watch loop (then wrap as ClickException).
• Add tests and a test helper to validate fatal-error propagation behavior.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/test_watch_fatal_error.py	New tests covering fatal error propagation from background task and websocket listener.
tests/test_events.py	Adds coverage for dispatcher behavior when an OAuth refresh exception occurs.
tests/conftest.py	Adds a helper to construct an OAuthRefreshException for tests.
plextraktsync/watch/WebSocketListener.py	Adds fatal error checks and passes fatal state into the dispatcher.
plextraktsync/watch/FatalErrorState.py	Introduces a shared, thread-safe fatal error container.
plextraktsync/watch/EventDispatcher.py	Records OAuthRefreshException into fatal state and re-raises it.
plextraktsync/queue/BackgroundTask.py	Records OAuthRefreshException into fatal state and checks/raises fatal state during processing.
plextraktsync/media/MediaFactory.py	Ensures OAuthRefreshException is not swallowed by generic Trakt error handling.
plextraktsync/factory/Factory.py	Adds watch_fatal_error and wires it into websocket listener and background queue.
plextraktsync/commands/watch.py	Catches OAuthRefreshException from watch loop and raises a ClickException.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[glensc]

This seems to solve the symptom, but the implementation feels broader than the requirement. The issue appears to need watch mode to terminate on OAuthRefreshException rather than continue looping. Could we keep this fix more localized by re-raising OAuthRefreshException from watch-related code paths and handling it once in commands/watch.py, instead of introducing a shared FatalErrorState and plumbing it through Factory and generic BackgroundTask infrastructure? If background/callback contexts truly require propagation, maybe that logic can stay inside WebSocketListener rather than becoming shared cross-component state.

[glensc]

Merge readiness

not obviously wrong, but likely over-engineered.

• Correctness: plausible
• Risk: medium, because it changes shared plumbing
• Maintainability: worse than a localized watch-only fix
• Recommendation: request simplification unless the author can show that the exception really crosses thread/callback boundaries in a way that forces this design

[glensc]

i'l give copilot a try to make it more simple

[simonc56]

i'l give copilot a try to make it more simple

I agree the FatalErrorState is overkill to catch the sub-processes OauthRefreshException error but I didn't know how to do this tricky part.
If there is a better way and we can remove it, let's go. Thank you for your help.

[simonc56]

It looks complicated to me, here is the plan from GPT-5.4 when asked to simplify the PR and try to remove the fatal error state logic to keep a simple re-raise logic if possible :

Plan: Simplify Watch OAuth Failure Path

The PR can be simplified, but not all the way down to plain re-raise everywhere.

The key finding is that there really are thread boundaries here: PlexAPI alert callbacks are started through an AlertListener, and the Trakt queue work runs on a daemon thread. That means a full removal of the fatal-error bridge would drop real failures on the floor. The narrower simplification is to keep one shared bridge for off-thread failures, but remove the extra propagation logic from the generic event dispatcher.

Plan

1. Keep watch.py as the only CLI-facing error conversion point. The watch command should still clear the watch fatal state before starting, and still turn an OAuth refresh failure into a ClickException once the main loop sees it.
2. Simplify the websocket path by moving callback-thread capture into WebSocketListener.py. The listener should wrap its callback into the dispatcher, catch OAuthRefreshException there, record it in the existing fatal state, and let the main listen loop re-surface it on the next poll.
3. Remove dispatcher-level shared-state plumbing from EventDispatcher.py. It should no longer accept or mutate a fatal-error object; it only needs to keep re-raising OAuthRefreshException instead of swallowing it.
4. Keep the daemon-thread bridge in BackgroundTask.py. That path cannot bubble directly to the watch command, so the thread-safe fatal state is still required there.
5. Trim only the now-unused wiring in Factory.py. The listener and queue should still share the same fatal state, but the dispatcher should stop knowing about it.
6. Update tests so they match the smaller design: keep queue propagation coverage in test_watch_fatal_error.py, keep dispatcher re-raise coverage in test_events.py, and shift websocket fatal-state expectations to the listener layer instead of the dispatcher. Reuse the helper in conftest.py.

Scope

Included:

• Removing EventDispatcher’s fatal-state responsibility
• Keeping watch termination on OAuth refresh failure
• Preserving the queue-thread handoff that is still required

Excluded:

• Removing FatalErrorState.py entirely
• Refactoring queue ownership out of the factory
• Changing broader Trakt queue behavior outside this watch failure path

Verification

1. Run the focused tests in test_events.py and tests/test_watch_fatal_error.py.
2. Then run the nearest related watch and threading tests to catch regressions in callback or queue behavior.
3. Manually confirm the final control flow has one user-facing catch in the watch command, one websocket callback bridge in the listener, and one daemon-thread bridge in the background task.

[glensc]

pytrakt has been updated to 4.3.0:

• Bump pytrakt from 4.2.3 to 4.3.0 #2493