fix(availability-sync): keep seasons when the media server is unreachable#3102
Conversation
…able
A non-404 error while checking a show in Plex/Jellyfin sets existsInPlex /
existsInJellyfin = true to protect the show ("Preventing removal"), but the
returned season map is left empty. The caller seeds every available season to
"false" and only flips it to "true" on an affirmative find, so an empty map is
treated as "all seasons absent" and seasonUpdater marks them DELETED -- even
though the media still exists and we only failed to reach the server. The
preventSeasonSearch guard previously just skipped the (already-empty-on-error)
season loop, so it did not change this outcome.
Mirror the show-level guard at the season level: when the lookup errors, assume
the previously-available seasons still exist rather than returning an empty map.
A genuine 404 still falls through to normal detection, and a successful-but-empty
response still deletes as before.
Adds a regression test covering a Plex connection failure with no Sonarr coverage.
Closes seerr-team#1729
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughFixes season-map building in Plex and Jellyfin TV sync paths so filtered seasons are always recorded when season checks are suppressed; adds Jest regression tests that simulate non-404 connection refusals and assert shows and seasons remain AVAILABLE. ChangesSeason Availability Sync Fix
Sequence Diagram(s)sequenceDiagram
participant AvailabilitySync
participant Plex
participant Jellyfin
participant TMDB
participant Sonarr
AvailabilitySync->>Plex: filter TV seasons by 4K status / build seasons list
AvailabilitySync->>Jellyfin: filter TV seasons by 4K status / build seasons list
alt preventSeasonSearch == true
AvailabilitySync->>AvailabilitySync: mark filtered seasons as existing in seasonsMap
else preventSeasonSearch == false
AvailabilitySync->>Plex: call seasonExistsInPlex for each season
Plex-->>AvailabilitySync: season existence result
AvailabilitySync->>Jellyfin: call seasonExistsInJellyfin for each season
Jellyfin-->>AvailabilitySync: season existence result
end
AvailabilitySync->>TMDB: fetch show/season metadata
AvailabilitySync->>Sonarr: attempt series lookup (may 404)
Sonarr-->>AvailabilitySync: lookup result
AvailabilitySync->>AvailabilitySync: finalize statuses for show and seasons
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/lib/availabilitySync.test.ts (1)
902-977: ⚡ Quick winAdd the matching Jellyfin non-404 outage regression case.
This new test locks the Plex path well, but this PR also changes the same fallback contract in
mediaExistsInJellyfin. Adding the analogous Jellyfin test (metadata/season fetch throws non-404, Sonarr 404, TMDB seasons present) would protect cross-provider behavior from drifting.Suggested test shape
+ it('should not delete seasons when Jellyfin fetch fails with a non-404 error', async () => { + configureJellyfin(); + configureSonarr([{ syncEnabled: true }]); + // seed TV media with AVAILABLE seasons + // mock getItemData/getSeasons to throw non-404 connection error + // mock Sonarr getSeriesById -> 404 + // mock TMDB seasons present + // run sync and assert all seasons + show remain AVAILABLE + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/lib/availabilitySync.test.ts` around lines 902 - 977, Add a parallel regression test for Jellyfin mirroring the Plex case: create a test similar to the Plex "should not delete seasons when the Plex season fetch fails with a non-404 error" but exercising mediaExistsInJellyfin instead — configure the show with MediaType.TV and a Jellyfin ratingKey, make the Jellyfin metadata/children fetch implementations throw a non-404 connection error, make getSeriesByIdImpl throw a 404, have getTvShowImpl return TMDB seasons with episodes, run availabilitySync.run, and assert the show's status and each Season.status remain MediaStatus.AVAILABLE; use the existing Plex test as the template and reference symbols mediaExistsInJellyfin, availabilitySync.run, getSeriesByIdImpl, and getTvShowImpl to locate where to add the new case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@server/lib/availabilitySync.test.ts`:
- Around line 902-977: Add a parallel regression test for Jellyfin mirroring the
Plex case: create a test similar to the Plex "should not delete seasons when the
Plex season fetch fails with a non-404 error" but exercising
mediaExistsInJellyfin instead — configure the show with MediaType.TV and a
Jellyfin ratingKey, make the Jellyfin metadata/children fetch implementations
throw a non-404 connection error, make getSeriesByIdImpl throw a 404, have
getTvShowImpl return TMDB seasons with episodes, run availabilitySync.run, and
assert the show's status and each Season.status remain MediaStatus.AVAILABLE;
use the existing Plex test as the template and reference symbols
mediaExistsInJellyfin, availabilitySync.run, getSeriesByIdImpl, and
getTvShowImpl to locate where to add the new case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: befdb0d2-3c91-4be3-b653-ba2abdbd8372
📒 Files selected for processing (2)
server/lib/availabilitySync.test.tsserver/lib/availabilitySync.ts
Mirror of the Plex outage regression test for the Jellyfin/Emby path, since the fix applies the same fallback contract to mediaExistsInJellyfin. Verified it fails without the change (seasons become DELETED) and passes with it. Addresses CodeRabbit review feedback on seerr-team#3102. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@OmarB97 Rule number 1 from our
Don’t copy/paste unnecessary sentences. The CI handles tests no need to share results, we won’t trust them. |
|
Removed the generated PR/comment text. The bug is that a Plex/Jellyfin outage preserves the show but leaves the season map empty, so seasons get marked deleted. This PR keeps already-available seasons available when the media server lookup fails with a non-404 error. Let me know if you have any other issues/concerns. |
|
addressed comments above. captured the full relevant context in each of those comments without being overly verbose. |
|
@0xSysR3ll could you please test if you can reproduce the root issue of this? And test if this PR fixes it if you can reproduce? |
5 participants
Description
Fixes #1729.
This PR stops availability sync from marking TV seasons as deleted when Plex or Jellyfin is temporarily unreachable. If the media-server lookup fails with a non-404 error, seasons that were already available stay available instead of being treated as missing.
The net effect of this is that instead of Seerr showing the show that DOES exist on Plex as deleted (even though it's not deleted), NOW it will show as available (synced).
How Has This Been Tested?
Added regression coverage for Plex and Jellyfin outage cases.
Screenshots / Logs (if applicable)
N/A
Checklist:
pnpm buildpnpm i18n:extractAI Disclosure
AI assistance was used while working on this PR. I reviewed the changes before submitting them.
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests