Skip to content

fix: stop 500s when an account has no usable PDS credentials - #19

Merged
CountableNewt merged 1 commit into
mainfrom
fix/pds-credential-500
Aug 8, 2026
Merged

CountableNewt merged 1 commit into
mainfrom
fix/pds-credential-500

Conversation

@CountableNewt

Copy link
Copy Markdown
Contributor

Fixes the 500 on GET /v1/accounts/:did/schedules reported on the dev instance, and adds the reconnect handling the fix makes necessary.

Content-identical to what is running on dev (git diff origin/dev HEAD is empty). Cherry-picked onto main rather than merging dev, because main squash-merges — a dev -> main PR bases at 64952ad and spuriously re-applies #15/#16/#17.

Root cause

Listing schedules resolves brand permissions first, which walks every managed account and calls listRecords for each. authenticatedSession sat outside the do/catch that falls back to cached records, so any account without an OAuth session row threw notConfigured and the request became an opaque 500 — for every user, not just the account at fault.

Dev hit this constantly because dev/seed inserts six accounts with no OAuth sessions. Prod is exposed too, just less often: any real account whose token is revoked or expires produces the same 500 for everyone.

The bug was invisible to tests because InMemoryPDSClient returns [:] for unknown DIDs instead of throwing.

Changes

Read pathlistRecords and getRecord now wrap the session lookup, so a missing or rejected credential reads through to the local cache the same way an unreachable PDS already did.

Detection — because reads now degrade quietly, a dead token would otherwise look healthy until the next publish. PDSClientError.isAuthFailure is now the single definition of "credentials are gone" (notConfigured, 401/403, invalid_grant), shared with ScheduleWorker.classify so the two can't drift. A read that falls back for auth reasons marks the account needs_reauth; transient failures (503, 429) deliberately don't.

Write path — writes still can't succeed without credentials, so auth failures map to 409 account_needs_reauth instead of a 500. Deliberately not 401: the Skej session is valid, only the PDS credential is dead, and a 401 reads as "signed out" and would bounce the user to sign-in.

DiagnosabilityErrorMiddleware swallowed unhandled errors silently, which is why this surfaced as a bare 500 with nothing to go on. It now logs method, path, and the reflected error.

UIneeds_reauth was previously set only by the worker and read by nothing. requestJSON now throws SkejApiError carrying the error code; a banner names the stale account and offers a Reconnect that starts OAuth with the handle prefilled; both account pickers mark stale accounts; the settings page gains a status badge and its own reconnect card.

Verification

  • 70 Swift tests, 21 web tests, lint and typecheck green — re-run on this branch on top of main.
  • Regression tests added at the defect site; both failed with Caught error: .notConfigured before the fix.
  • Exercised end to end in a browser: flipped an account to needs_reauth, confirmed banner and picker warning, clicked Reconnect, confirmed return to active and banner cleared.
  • Running on dev / api.testing.skej.at and verified there.

Prod notes

Prod runs SKEJ_PRO_ENABLED=false. The Connected Account card sits outside the Pro gate so both reauth surfaces work, and /v1/accounts filters to self — so prod won't see the seeded-demo-account noise dev does.

No migrations, no new env vars, no config changes.

🤖 Generated with Claude Code

Listing schedules resolves brand permissions first, which walks every
managed account and calls listRecords for each. authenticatedSession sat
outside the do/catch that falls back to cached records, so any account
without an OAuth session row threw notConfigured and the request became
an opaque 500 -- for every user, not just the account at fault. Dev hits
this because dev/seed inserts six accounts with no OAuth sessions.

Move the session lookup inside the fallback in listRecords and getRecord
so a missing or rejected credential reads through to the local cache, the
same way an unreachable PDS already did.

Because reads now degrade quietly, add the detection that makes that safe:
PDSClientError.isAuthFailure is the single definition of "credentials are
gone" (shared with ScheduleWorker.classify so they cannot drift), and a
read that falls back for auth reasons marks the account needs_reauth.
Transient failures deliberately do not.

Writes still cannot succeed without credentials, so map auth failures to
409 account_needs_reauth instead of a 500. Not 401 -- the Skej session is
valid, only the PDS credential is dead, and a 401 reads as "signed out".

ErrorMiddleware also logs unhandled errors now; it swallowed them
silently, which is why this surfaced as a bare 500 with nothing to go on.

Surface all of this in the UI, since needs_reauth was previously set only
by the worker and read by nothing. requestJSON throws SkejApiError
carrying the error code, a banner names the stale account and offers a
Reconnect that starts OAuth with the handle prefilled, both account
pickers mark stale accounts, and the settings page gains a status badge
and its own reconnect card.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@CountableNewt
CountableNewt merged commit dd86863 into main Aug 8, 2026
5 checks passed
@CountableNewt
CountableNewt deleted the fix/pds-credential-500 branch August 8, 2026 03:55
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.

1 participant