Skip to content

fix(api): renew the session once, and not at all for guests - #136

Merged
aquie00t merged 1 commit into
mainfrom
fix/single-flight-refresh
Aug 26, 2026
Merged

fix(api): renew the session once, and not at all for guests#136
aquie00t merged 1 commit into
mainfrom
fix/single-flight-refresh

Conversation

@aquie00t

Copy link
Copy Markdown
Contributor

Digging into the API client, since articles now lean on it harder than anything else does. Two defects in the 401 path — the first has a user-visible symptom that reads as a bug in articles.

Opening an article can sign you out

The public branch called refresh directly, escaping the single-flight guard the authenticated path uses.

Opening an article fires several public reads at once — the article, its comments, the trending rail. With a stale token each one takes that branch, and each asked for its own refresh. Measured on main:

expected 3 to be 1

Refresh is limited to five a minute, and where the refresh token rotates, the second and third calls present one the first has already spent. They fail — and a failed refresh runs the session-expired handler. So the symptom is being logged out for opening an article, which looks nothing like a bug in the API client.

Every caller now shares one in-flight refresh, the authenticated path included.

A guest gets asked to sign in

A public request that carried no token was taking the same path. It was already anonymous, so its 401 is the endpoint's own answer, not a stale session. The client replayed the identical request anyway, asked to renew a session that was never opened, and on failure reported it expired — which opens the sign-in modal at a reader who never signed in.

That case falls through to ordinary error handling now. Also measured on main: one refresh where there should be none.

Reviewing

Both are pre-existing — neither was introduced by the articles work — but articles is what makes them likely, because it is the first feature to fire three public reads on one page load. That is worth keeping in mind when judging severity: the code was fine when one public read per page was the norm.

The single-flight helper is deliberately small and sits beside the existing queue rather than replacing it. The queue still serialises authenticated retries; it just no longer has a second, unguarded path running alongside it.

Verification

pnpm build, tsc -b, pnpm lint clean. 478 unit tests across 66 files and 29 Playwright tests pass. Both new tests fail on main.

docs/QA.md records why both tests exist, since the reasoning is not visible from the assertions alone.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Hu1QuLxS84vdf1gmzoGtWP

Two defects in the API client's 401 handling, both made much easier to hit
by articles.

The public branch called refresh directly, escaping the single-flight guard
the authenticated path uses. Opening an article fires several public reads
at once — the article, its comments, the trending rail — and with a stale
token each one asked for its own refresh. Measured: three concurrent public
401s produced three refresh calls. Refresh is limited to five a minute, and
where the refresh token rotates the second and third present one the first
has already spent: they fail, and a failed refresh signs the reader out. So
the visible symptom is being logged out for opening an article.

Every caller now shares one in-flight refresh, the authenticated path
included.

A public request that carried no token was also taking this path. It was
already anonymous, so its 401 is the endpoint's own answer rather than a
stale session — but the client replayed the identical request, asked to
renew a session that was never opened, and on failure reported it expired,
which opens the sign-in modal at a reader who never signed in. That case
now falls through to ordinary error handling.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hu1QuLxS84vdf1gmzoGtWP
@cloudflare-workers-and-pages

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
tdn-client 428c324 Commit Preview URL

Branch Preview URL
Aug 26 2026, 04:04 AM

@aquie00t
aquie00t merged commit 983ea25 into main Aug 26, 2026
8 checks passed
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