Skip to content

feat(settings): unify connection-test error handling across services#2396

Merged
enoch85 merged 143 commits into
Maintainerr:mainfrom
enoch85:feat/jellyfin-connection-error-handling
Mar 24, 2026
Merged

feat(settings): unify connection-test error handling across services#2396
enoch85 merged 143 commits into
Maintainerr:mainfrom
enoch85:feat/jellyfin-connection-error-handling

Conversation

@enoch85

@enoch85 enoch85 commented Feb 21, 2026

Copy link
Copy Markdown
Collaborator

What was done

Why

  • Makes connection-test errors consistent and DRY across backend + frontend.
  • Preserves deep debug context in logs while returning concise, actionable messages to users.
  • Removes fragile sentinel patterns and duplicate loading-state teardown paths, reducing UI edge-case bugs.

enoch85 and others added 30 commits January 31, 2026 00:24
* feat: Add Jellyfin support with abstraction layer

- Add MediaServerService abstraction for Plex/Jellyfin
- Implement JellyfinService with full API integration
- Update rules engine to support multiple media servers
- Add Jellyfin settings UI and configuration
- Refactor collections to use MediaServerService
- Add comprehensive type safety improvements
- Update database schema for media server support

* snc against this branch instead

* feat(api): add deprecated /api/plex legacy wrapper for backward compatibility

- Add PlexApiLegacyController with all 19 original endpoints
- All endpoints delegate to MediaServerFactory abstraction
- Add deprecation headers (X-Deprecated, Deprecation, Link)
- Uses MediaServerSetupGuard (works with both Plex and Jellyfin)
- Maps old route patterns to new abstraction interface
- Single file design for easy removal when legacy support ends

To remove legacy support: delete plex-api-legacy.controller.ts and
remove PlexApiLegacyController from plex-api.module.ts

* chore: merge main branch changes

- Use leftJoinAndSelect for rules (allows groups without rules)
- Delete unused CollectionDetail/index.tsx
- Use useRuleGroupForCollection hook instead of manual fetch
- Conditionally show Test Media button only when useRules=true
- Simplify addCollectionToDB and RemoveCollectionFromDB methods

* fix: update SchemaSync migration to use mediaServerId instead of plexId

The SchemaSync migration from main referenced plexId (integer) but our
JellyfinSupport migration already renamed this to mediaServerId (varchar).
Updated both UP and DOWN methods to use the correct column name and type.

* regenerated with typeorm

* fix: standardize quotation marks and formatting in SchemaSync migration

* generate clean typeorm

* fix(ui): filter out empty/invalid rules to prevent crash

When useRules=false, the backend creates a rule with empty ruleJson.
These rules have no firstVal, causing TypeError when shouldFilterApp
tries to access rule.firstVal[0].

Refactored to remove all useEffect usage:
- Use useSyncExternalStore for scroll detection (React 19 pattern)
- Move rule filtering to event handlers (updateLibraryId, handleUpdateArrAction)
- Extract helper functions outside component (parseValidRules, shouldFilterApp, filterRulesForArrSettings)

This is cleaner and more aligned with modern React patterns.

* fix: complete merge of main branch rule editor fixes

This commit completes the merge from main that was started in 0923d01.
The original merge included the leftJoinAndSelect changes but missed
removing the empty rule creation code from rules.service.ts.

Backend (rules.service.ts):
- Remove empty rule creation in createRuleGroup and updateRuleGroup
- When useRules=false, the backend was creating rules with empty ruleJson
- This aligns with main's fix from commit 730adb5 (Maintainerr#2270)
- The RemoveEmptyRules migration (already present) cleans up existing data

Frontend (RuleFormPage.tsx):
- Add key={id} prop to AddModal component
- This ensures the form fully remounts when navigating between rule groups
- Replaces the need for useEffect-based form reset logic

Frontend (AddModal/index.tsx):
- Remove parseValidRules helper (no longer needed since empty rules won't exist)
- Remove defensive firstVal check in filterRulesForArrSettings
- Simplify rules state initialization to direct parsing

The root cause was that commit 0923d01 merged the query changes
(leftJoinAndSelect for rules) but not the corresponding code removal
that prevents empty rules from being created in the first place.

* replace more useEffect usage

* prettier

* refactor(ui): replace JS touch detection with CSS media queries

- Remove unused useInteraction, useIsTouch hooks and InteractionContext
- Add CSS @media (hover: hover/none) for touch-friendly MediaCard
- Simplify MediaCard: click always opens modal, CSS handles hover states
- Reduces bundle size by ~150 lines of JS with 15 lines of CSS

* fix click behaviour on mobile

* fix bug on about page showing 0 items in collections

* add support for retrieving seasons and episode view counts

* retrieve admin user ID for UserData fields in getSeasons and getItems

* fix: make watchedAt optional in WatchRecord interface

* add temporary debug for viewdate

* more temp debug

* possible fix for viewdate season/episodes

* prettier

* feat: add getPlaylistItems method to JellyfinAdapterService and update JellyfinGetterService to utilize it

* fix cd/ci tests
Added a warning about the development branch and Jellyfin support.
When upgrading from pre-Jellyfin versions, media_server_type is null.
Instead of defaulting to Plex everywhere (which caused noisy errors if
Plex was down), auto-detect the server type from existing credentials
during settings init and persist it.

- settings.service.ts: auto-detect Jellyfin/Plex from credentials on init
- media-server.factory.ts: throw instead of defaulting to Plex when
  no server type is configured; distinguish "not configured" from
  "unreachable" in startup logging
- collections.service.ts: update return type to match factory change
…ching

Handle partial failures gracefully in Jellyfin adapter. If any single
user query fails when fetching watch history or play counts, the other
successful queries are still processed instead of losing all data.
Query parameters are received as strings. Without ParseIntPipe, numeric
comparisons and math operations work on strings, producing incorrect results.

Added ParseIntPipe with optional: true to:
- media-server.controller.ts: page, limit parameters
- plex-api-legacy.controller.ts: amount parameter
- rules.controller.ts: rulegroupId, typeId parameters

Also refactored rules.controller.ts to use individual @query parameters
instead of inline object types for better type safety.
The Plex adapter's resetMetadataCache() was a no-op despite PlexApiService
having this capability. Now properly delegates to plexApi.resetMetadataCache()
when an itemId is provided.
Instead of hardcoding progress to 100, use the actual PlayedPercentage
from Jellyfin's UserData when available. Falls back to 100 if not set.

Note: Plex API doesn't expose progress percentage in watch history,
so it remains hardcoded to 100 (which is correct for completed watches).
The testResult state was never cleared when URL or API key inputs changed.
Users could modify credentials and save with a stale success indicator.
Now clears test result and testedSettings when credentials are modified.
The factory already handled errors properly, but the warning message
didn't include the actual error details for debugging. Now includes
the error message in the log output.
Remove leftover debug console.log statement from addImportExclusion.
Add comprehensive unit tests for PlexAdapterService covering:
- Lifecycle management (isSetup, initialize, uninitialize)
- Server type identification (returns PLEX)
- Feature detection (LABELS, PLAYLISTS, COLLECTION_VISIBILITY, etc.)
- Cache management with resetMetadataCache delegation
- Server status mapping to MediaServerStatus
- User fetching and mapping to MediaUser
- Library fetching and mapping to MediaLibrary
- Library content queries with Jellyfin ID detection
- Watch history retrieval and mapping to WatchRecord
- Collection operations (create, delete)
- Search content delegation

This brings the test count from 392 to 419 tests.
Jellyfin provides ratings through CommunityRating (typically TMDB,
0-10 scale) and CriticRating (typically Rotten Tomatoes, 0-100 scale
normalized to 0-10).

Map these to the existing rating rule properties:
- rating_imdb/rating_tmdb: Use CommunityRating (TMDB-sourced)
- rating_rottenTomatoesCritic: Use CriticRating
- rating_rottenTomatoesAudience: Use CommunityRating as approximation
- All *Show variants: Traverse to parent/grandparent for show metadata

This enables Jellyfin users to create rules based on ratings, which
previously returned null for all external rating properties.
Jellyfin genres don't have IDs, so we were using array index which was
fragile and could change between requests or items. Now using djb2 hash
algorithm to generate stable IDs from genre names.

Fixes review item #11.
During initial setup, the Plex and Jellyfin tabs were both visible
before the user selected a media server type. Now only the General
tab is shown until a selection is made.

Fixes review item #14.
…switch

Added a switchInProgress flag that:
- Prevents concurrent switch operations from starting
- Blocks factory.getService() calls during the switch
- Is always released in a finally block

This prevents API requests from routing to the wrong adapter during
the window between settings update and server initialization.

Fixes review item #8.
…apters

- Document error handling contract in IMediaServerService interface:
  - Read operations return empty/undefined on failure
  - Write operations throw with descriptive message
- Add try/catch with logging to Plex adapter write operations
- Improve error messages to include context (collection ID, item ID, etc.)

Fixes review item #10.
The lock added to MediaServerFactory caused a circular dependency
between MediaServerModule and SettingsModule. Removed the lock
entirely as it was over-engineering - the switch operation is
rare and short-lived.
- Hide Plex and Tautulli rules when on Jellyfin media server
- Hide Jellyfin rules when on Plex media server
- Use MediaServerType enum from contracts instead of string literals
- Replace string literals 'plex' and 'jellyfin' with MediaServerType enum
- Update Settings/index.tsx to use enum for tab visibility
- Update MediaServerSelector to use enum for all comparisons
@enoch85

enoch85 commented Feb 21, 2026

Copy link
Copy Markdown
Collaborator Author

@ydkmlt84 This is ready now.

Once we merge jellyfin-dev to main, I'll change the base here to main instead, and you can review/merge/whatever.

@enoch85 enoch85 self-assigned this Feb 25, 2026
@enoch85 enoch85 changed the base branch from jellyfin-dev to main February 27, 2026 19:44
enoch85 and others added 11 commits February 27, 2026 19:49
…nerr#2330 merge

Keep favorites feature additions (getItemFavoritedBy, favoritedBy/sw_favoritedBy
rule properties and getter cases) while accepting upstream's improved log message.
… into feat/jellyfin-connection-error-handling
…tests

Connection tests now use a shared CONNECTION_TEST_TIMEOUT_MS constant (3s)
instead of hardcoded 10s values. The testConnections() method runs all
service checks concurrently via Promise.all instead of sequentially.
…ndling branch

Resolve conflicts from the Seerr migration (PR Maintainerr#2397) which unified
Overseerr and Jellyseerr into a single Seerr abstraction. Accept
deletions of old Jellyseerr-specific files, carry over connection-test
utilities to the unified Seerr service, and preserve parallel test
execution in settings.
@enoch85

enoch85 commented Feb 28, 2026

Copy link
Copy Markdown
Collaborator Author

(now rebased on main)

enoch85 added a commit that referenced this pull request Feb 28, 2026
PR #2396 - feat(settings): unify connection-test error handling across services
- Added shared backend/frontend error helpers for connection tests
- Reduced connection test timeout to 3s and parallelized tests
- Normalized user-facing error messages across all service test endpoints

PR #2429 - fix: emit collection notification events only after successful operations
- Moved event emissions to after operations complete
- Only emits events for items that were truly added/removed
- Pass touchedMediaServerIds to prevent incorrect removals during sync

PR #2436 - fix(server): resolve Seerr addUser returning NULL
- Use username fields directly from Seerr API response instead of media server ID lookup
- Use workspace-local typeorm migration cli
- Use initial setup terminology for media server success log
- Added 11 unit tests for addUser covering all user types and edge cases
@enoch85 enoch85 removed the request for review from ydkmlt84 March 24, 2026 16:57
@enoch85 enoch85 merged commit 7ae0c4c into Maintainerr:main Mar 24, 2026
9 checks passed
@enoch85 enoch85 deleted the feat/jellyfin-connection-error-handling branch March 24, 2026 17:58
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 3.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants