fix(bluesky): Sign in with Bluesky on Workers — translate resolver redirect:'error' → 'manual'#47
Merged
Conversation
…Workers edge
Sign in with Bluesky failed on prod for every handle, deterministically, before
the browser ever left the site (?bsky=error&reason=authorize). Root cause,
confirmed by capturing the swallowed exception under wrangler dev:
@atproto/oauth-client's identity/handle/DID resolvers (did:web, did:plc, and the
XRPC + well-known handle resolvers) all issue fetch(url, { redirect: 'error' }),
and Cloudflare's edge runtime rejects redirect:'error' with a TypeError
("won't be implemented ... at the edge; use manual and check the response status
code") — so authorize() throws at identity resolution, before reaching the PDS.
See github.com/bluesky-social/atproto#3292.
Fix: inject an edgeFetch as the OAuthClient's Fetch — OAuthClient threads it to
every resolver it builds — that rewrites redirect:'error' to redirect:'manual' on
the init BEFORE any Request is constructed (constructing one with 'error' is
itself what throws). The resolvers already reject any non-200 response, and a
manual-mode redirect surfaces as a non-200 opaqueredirect, so the "fail on an
unexpected redirect" security intent is preserved. Zero new dependencies; the
single makeClient factory means the callback token-exchange path is covered too.
Verified under wrangler dev: with the fix the captured failure moves PAST the
redirect TypeError to the real network call (blocked only by this container's
egress allowlist, which production does not have). Full end-to-end success is the
operator's prod re-test. The unit test pins the redirect rewrite + passthrough;
the live PDS round-trip cannot run in the offline fake-KV harness.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVPyHAtQ1cdK76RpFasafP
…ng it
The start handler caught blueskyAuthorizeUrl()'s error with a bare `catch {}` and
showed only a generic banner — which is exactly why the redirect:'error' defect
fixed in the preceding commit was invisible to anyone diagnosing it. Log the
error name/message/cause so it surfaces in Workers Logs (observability is already
enabled in wrangler.jsonc). This pre-authorization path handles no token, cookie,
or key material, so there is nothing secret to redact; the user still sees only
the generic bsky=error banner.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LVPyHAtQ1cdK76RpFasafP
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
wecanjustbuildthings | 092e57d | Jul 04 2026, 04:40 AM |
MartinMontero
marked this pull request as ready for review
July 4, 2026 04:52
Merged
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes the live prod defect: Sign in with Bluesky failed for every user, deterministically, with "Sign-in didn't finish — please try again." Nostr sign-in was unaffected.
Two commits:
fix(bluesky)— the root-cause fix + a regression test.observability(bluesky)— un-swallow the barecatch {}that hid the failure.Root cause (confirmed, not inferred)
@atproto/oauth-client@0.7.3's identity/handle/DID resolvers (did:web,did:plc, the XRPC and well-known handle resolvers) all issuefetch(url, { redirect: 'error' }). Cloudflare's edge runtime rejectsredirect: 'error'withTypeError: … "error" won't be implemented … at the edge; use "manual" and check the response status code. Soauthorize()threw at the identity-resolution phase — before the browser ever left the site, before any PDS contact — and the handler's barecatch {}reduced it to a generic banner. (Upstream: bluesky-social/atproto#3292.)Evidence table (all read-only, pre-fix):
?bsky=error&reason=authorize, deterministic across two handles → start-side throwredirect_urisbyte-match;BLUESKY_PRIVATE_KEY_JWKloads as valid ES256; general Worker egress works (npmjs)bsky_state:entries → throw precedes state-saveFailed to resolve identity→ causeTypeError: Invalid redirect value … at the edgeThe fix
Inject an
edgeFetchas theOAuthClient'sFetch(which it threads to every resolver): it rewritesredirect: 'error'→redirect: 'manual'on the init before anyRequestis constructed — constructing one with'error'is itself what throws. The resolvers already reject any non-200 response, and a manual-mode redirect surfaces as a non-200 opaqueredirect, so the "fail on an unexpected redirect" security intent is preserved. Zero new dependencies. The singlemakeClientfactory means the callback token-exchange path is covered by the same fix.Verification
TypeError; after the fix, that TypeError is gone and the flow proceeds to the real network call (which fails here only because this diagnosis container's egress allowlist blocksbsky.social— a constraint production does not have). That transition is the proof the redirect barrier is cleared.'error'→'manual') and passthrough of every other init/no-initcase.verify:allgreen;enforcegreen; 285 tests, 0 fail.Type of change
Required checks
npm run check· [x]npm run enforce· [x]npm test(285) · [x]npm run build— all green locally.For catalog entries / recipes
Not applicable.
Deploy semantics & your post-merge re-test
Workers Builds is live — merging this IS a production deploy. After you merge, please re-test on prod:
I'll then re-run the read-only checks (admin negative contract unchanged;
/console/CSP single-header; status endpoints 200), and admin smoke [3] (Bluesky reuse-then-elevate) is unblocked pending identities.Draft on purpose — do not mark ready-for-review, do not merge; merge is yours alone and is a production deploy.
🤖 Generated with Claude Code
https://claude.ai/code/session_01LVPyHAtQ1cdK76RpFasafP
Generated by Claude Code