fix: avoid duplicate cache headers per server client - #283
fix: avoid duplicate cache headers per server client#283dhruvxvaishnav wants to merge 3 commits into
Conversation
mandarini
left a comment
There was a problem hiding this comment.
Hi @dhruvxvaishnav, thank you so much for this! The fix looks great and the regression test is exactly what was needed. One thing before I merge: since the headers are now sent only once per client instance, could you make that constraint explicit in the SetAllCookies JSDoc in types.ts? Something along the lines of: the cache headers are delivered on the first cookie write from a server client, so a client must be created fresh for each request, otherwise later responses will miss these headers. Right now the doc says the object is empty on later calls but not why reusing a client across requests would be a problem.
Thanks again for taking this on!
|
Thank you for the clear feedback! I updated the SetAllCookies JSDoc to state that a server client delivers cache headers only with its first cookie write and must be created fresh for every request; otherwise later responses could miss the required cache headers. I also clarified that subsequent calls from the same client receive an empty headers object. Local formatting, linting, builds, all 109 tests, and git diff checks pass. |
mandarini
left a comment
There was a problem hiding this comment.
Hi @dhruvxvaishnav! The updated SetAllCookies JSDoc is exactly what was needed, thank you for turning that around so quickly. The fix and the regression test both look solid.
One thing before this can merge. In src/cookies.ts, hasSentHeaders is set to true before originalSetAll resolves:
setAll = async (setCookies, headers) => {
const shouldSendHeaders = !hasSentHeaders && Object.keys(headers).length > 0;
if (shouldSendHeaders) {
hasSentHeaders = true;
}
await originalSetAll(setCookies, shouldSendHeaders ? headers : {});
};If that first call to originalSetAll throws, the flag is already flipped, and every later cookie write in the same request would silently lose the Cache-Control/Expires/Pragma headers with no error at all. Could you move the flag flip to after the await succeeds, so it only reflects a write that actually went through?
|
@mandarini Thanks for catching this. I moved the |
What kind of change does this PR introduce?
Bug fix.
Fixes #279.
What is the current behavior?
Starting an email PKCE flow with
signInWithOtp()writes the per-flow verifier, flow index, and legacy verifier cookies in separate batches. Each batch currently carries the same cache-prevention headers tosetAll.Frameworks that overwrite repeated response headers tolerate this, but SvelteKit's
event.setHeaders()rejects a second assignment of the same header during one request, causing the auth operation to fail with"Cache-Control" header is already set.What is the new behavior?
Every cookie batch is still forwarded to
setAll, while non-empty cache-prevention headers are forwarded only once for the lifetime of a request-scoped server client. The headers are marked as sent only aftersetAllsucceeds, so a failed write leaves them available for the next attempt. Later calls after a successful write receive an empty headers object because the response has already been marked as non-cacheable.The integration regression test exercises the full
createServerClient().auth.signInWithOtp()path with a strict SvelteKit-style callback. It verifies that all three cookie batches are preserved without assigning any response header twice. A focused unit test verifies that when the firstsetAllcall rejects, the next call still receives the cache headers.Additional context
The cache headers were introduced in #176 to prevent auth responses from being cached by CDNs. This change preserves that protection because response headers apply to the whole response rather than to an individual cookie batch.
Validation:
pnpm exec prettier --check src/cookies.ts src/cookies.spec.tspnpm exec eslintpnpm buildpnpm exec vitest run(110 tests passed)git diff --checkAI assistance disclosure: OpenAI Codex assisted with codebase exploration, implementation support, regression-test preparation, code review, and validation. I remain responsible for understanding and maintaining this contribution.