Skip to content

fix(auth): show a temporary server error when token refresh gets 503 dependency_unavailable - #695

Open
timmyconnect wants to merge 1 commit into
Silo-Server:mainfrom
timmyconnect:fix/refresh-dependency-unavailable
Open

timmyconnect wants to merge 1 commit into
Silo-Server:mainfrom
timmyconnect:fix/refresh-dependency-unavailable

Conversation

@timmyconnect

@timmyconnect timmyconnect commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Closes #602
Related issue: Silo-Server/silo-server#1951
Validation tasks: none

When the Silo server cannot reach its database, it answers POST /api/v2/auth/refresh with 503 dependency_unavailable. The app keeps its tokens, but the requests that were waiting on that refresh failed with their original 401, so iPhone, iPad, Apple TV and Mac showed "Your session has expired. Sign in again to continue." to a user who was still signed in. This change makes those requests fail with the 503, which reads as a temporary server problem.

Approach

The refresh path already hands 503 provider_unavailable to the waiting requests as their error. dependency_unavailable now takes the same route on both the ordinary and the scoped refresh. As with a provider outage, a request whose access token has not yet expired is still sent with that token.

The approver-bearer path (freshAccessToken(serverId:)) is unchanged: it keeps .providerUnavailable for the sign-in provider only, and a database outage stays .unreachable there, so the provider-specific wording does not appear for a database outage.

Retry-After is not honoured; the issue lists it as optional.

Validation

  • iOS unit tests (iPhone 17 Pro simulator, iOS 26.5): the external sign-in test class, including one new test for this case, 40 passed, 0 failed.
  • tvOS and macOS: not built or tested. The change is in shared networking code.
  • Not run against a live server with its database down.

Evidence

Evidence: none. The error message does change, but it was verified by unit test only; no before-and-after capture was taken, because reproducing it needs a server with its database down.

Risks

None identified. Only a 503 whose problem type is dependency_unavailable is handled differently; the session is kept in both the old and the new behaviour.

Checklist

  • I read and can explain the complete diff.
  • This pull request addresses one concern.
  • The Evidence section shows every change a user can see, or says there is none.

AI Disclosure

  • Harness: Claude Code (desktop app)
  • Tool(s): Xcode xcodebuild, xcodegen, GitHub CLI
  • Model(s): claude-opus-5-5
  • Involvement: AI-authored; not yet reviewed by a human
  • Adversarial review: n/a

🤖 Generated with Claude Code

Note

Treat 503 dependency_unavailable refresh failures as temporary server errors in HTTPClient

  • Adds isRefreshOutage and isOutageRefresh classifiers in HTTPClient.swift. They recognize both provider_unavailable and dependency_unavailable 503 problem responses as outages.
  • refreshScopedTokens and refreshTokens now return these 503s as outage failures instead of invalidating the session. Waiting requests receive the 503 problem.
  • Requests with a non-expired bearer token continue with that token when refresh returns a dependency_unavailable 503.
  • Adds a shared problemIdentifier parser that extracts the final component of the problem type identifier; the provider-only predicate now uses it.
  • Adds ExternalSignInTests.testDatabaseOutageOnRefreshReadsAsATemporaryServerProblem covering the 503 surfacing and token preservation.
  • Behavioral Change: a 503 dependency_unavailable during refresh no longer clears the session or the saved tokens; it is surfaced as a temporary server error.

Macroscope summarized b657854.

@silo-kody

silo-kody Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Silo Kody — review complete

Review finished. Check the inline comments for findings and verify each suggestion against the code and tests.

Reviewing changes in Silo
  • Include the related issue, expected behavior, and validation steps in the PR description.
  • For API changes, describe the effect on Apple and Android clients and Jellyfin compatibility.
  • For plugin changes, identify the affected SDK contract, plugin, and catalog entry.
  • Follow this repository's AGENTS.md and CONTRIBUTING.md.
  • Request another review with @kody start-review in a PR comment.
  • React with 👍 or 👎 to give feedback on individual suggestions.
Review settings
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ❌

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9eca5e32-2918-41bb-9a5d-16b68f1c0b42
📥 Commits

Reviewing files that changed from the base of the PR and between 7af0d58 and b657854.

📒 Files selected for processing (2)
  • iosApp/Tests/ExternalSignInTests.swift
  • iosApp/iosApp/Networking/HTTPClient.swift

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Refresh handling now recognizes 503 dependency_unavailable responses as outages, like provider_unavailable. Waiting requests receive the 503 problem, and the tests verify that saved tokens remain unchanged.

Changes

Refresh outage handling

Layer / File(s) Summary
Classify refresh outages
iosApp/iosApp/Networking/HTTPClient.swift
Refresh failures use a shared unavailable case. The classifier recognizes 503 provider_unavailable and dependency_unavailable responses.
Apply outage handling to refresh paths
iosApp/iosApp/Networking/HTTPClient.swift, iosApp/Tests/ExternalSignInTests.swift
Scoped and ordinary refresh paths preserve an unexpired bearer for recognized outages and return the 503 problem to waiting requests. A test checks the dependency_unavailable response and verifies that both tokens remain unchanged.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: quick104

Merge Risk: ⚪ Minimal · up to b6578

Waiting requests will now get the 503 temporary server problem when the database is down, rather than a misleading session-expired message. The change is small and tested. It carries no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: treating 503 dependency_unavailable token refresh failures as temporary server errors.
Description check ✅ Passed The description directly explains the problem, implementation, affected refresh paths, token behavior, testing, and known validation limits.
Linked Issues check ✅ Passed Issue #602 requires a 503 dependency_unavailable refresh response to reach waiting requests as a temporary server error and to preserve the session. HTTPClient.swift classifies `dependency_unavail…
Out of Scope Changes check ✅ Passed The changes stay within issue #602. The HTTPClient.swift classifier refactor supports refresh-outage handling, and the added external sign-in test verifies the required behavior. The changes do not …
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Quick104 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Note

Grok commenting on Quick's behalf.

Evidence incomplete. The PR says Evidence: none, but it also says the error message a signed-in user sees changes (from "Your session has expired. Sign in again to continue." to a temporary server error). none only fits when nothing user-visible changes, and the Evidence checklist box is unchecked.

Missing: a before-and-after capture of that error on iOS (iPhone simulator is fine) when token refresh returns 503 dependency_unavailable. A local server with its database stopped, or a stubbed refresh response, is enough to reproduce it. Unit tests are great to have but don't replace the capture. tvOS and macOS captures aren't required for this one since the change is in shared networking code.

@Quick104 Quick104 added priority: P2 Limited scope, workaround exists, or polish impact: usability Core flow broken or severely blocked labels Oct 8, 2026 — with Cursor
…dependency_unavailable

Requests waiting on a refresh that met a database outage failed with
their original 401, which reads as "session expired" while the user is
still signed in. They now fail with the 503 itself, as they already do
for provider_unavailable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@timmyconnect
timmyconnect force-pushed the fix/refresh-dependency-unavailable branch from ea60412 to b657854 Compare October 8, 2026 19:18
@silo-kody

silo-kody Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Silo Kody — review complete

Review finished. Check the inline comments for findings and verify each suggestion against the code and tests.

Reviewing changes in Silo
  • Include the related issue, expected behavior, and validation steps in the PR description.
  • For API changes, describe the effect on Apple and Android clients and Jellyfin compatibility.
  • For plugin changes, identify the affected SDK contract, plugin, and catalog entry.
  • Follow this repository's AGENTS.md and CONTRIBUTING.md.
  • Request another review with @kody start-review in a PR comment.
  • React with 👍 or 👎 to give feedback on individual suggestions.
Review settings
Review Options

The following review options are enabled or disabled:

Options Enabled
Bug ✅
Performance ✅
Security ✅
Business Logic ❌

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

Labels

impact: usability Core flow broken or severely blocked priority: P2 Limited scope, workaround exists, or polish

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show a temporary server error, not "session expired", when token refresh gets 503 dependency_unavailable

2 participants