Skip to content

Commit dc11250

Browse files
committed
fix: prevent race condition in async token refresh
1 parent cf38b22 commit dc11250

File tree

4 files changed

+166
-12
lines changed

4 files changed

+166
-12
lines changed

src/cookies.spec.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,4 +1069,80 @@ describe("applyServerStorage", () => {
10691069
},
10701070
]);
10711071
});
1072+
1073+
it("should log helpful error when setAll throws after response is sent", async () => {
1074+
const errors: any[][] = [];
1075+
const originalError = console.error;
1076+
console.error = (...args: any[]) => {
1077+
errors.push(args);
1078+
};
1079+
1080+
try {
1081+
const { storage, getAll, setAll, setItems, removedItems } =
1082+
createStorageFromOptions(
1083+
{
1084+
cookieEncoding: "raw",
1085+
cookies: {
1086+
getAll: async () => [],
1087+
setAll: async () => {
1088+
// Simulate the SvelteKit error when response is already sent
1089+
throw new Error(
1090+
"Cannot use `cookies.set(...)` after the response has been generated",
1091+
);
1092+
},
1093+
},
1094+
},
1095+
true,
1096+
);
1097+
1098+
await storage.setItem("storage-key", "value");
1099+
1100+
// This should throw, but log helpful context first
1101+
await expect(
1102+
applyServerStorage(
1103+
{ getAll, setAll, setItems, removedItems },
1104+
{
1105+
cookieEncoding: "raw",
1106+
},
1107+
),
1108+
).rejects.toThrow("after the response has been generated");
1109+
1110+
expect(errors.length).toEqual(1);
1111+
expect(errors[0][0]).toContain(
1112+
"Cannot set cookies after response has been sent",
1113+
);
1114+
expect(errors[0][0]).toContain("Token refresh completed too late");
1115+
} finally {
1116+
console.error = originalError;
1117+
}
1118+
});
1119+
1120+
it("should re-throw errors that are not related to response already sent", async () => {
1121+
const { storage, getAll, setAll, setItems, removedItems } =
1122+
createStorageFromOptions(
1123+
{
1124+
cookieEncoding: "raw",
1125+
cookies: {
1126+
getAll: async () => [],
1127+
setAll: async () => {
1128+
// Simulate a different error
1129+
throw new Error("Network error or some other issue");
1130+
},
1131+
},
1132+
},
1133+
true,
1134+
);
1135+
1136+
await storage.setItem("storage-key", "value");
1137+
1138+
// This SHOULD throw because it's not a "response already sent" error
1139+
await expect(
1140+
applyServerStorage(
1141+
{ getAll, setAll, setItems, removedItems },
1142+
{
1143+
cookieEncoding: "raw",
1144+
},
1145+
),
1146+
).rejects.toThrow("Network error or some other issue");
1147+
});
10721148
});

src/cookies.ts

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -468,16 +468,40 @@ export async function applyServerStorage(
468468
delete (removeCookieOptions as any).name;
469469
delete (setCookieOptions as any).name;
470470

471-
await setAll([
472-
...removeCookies.map((name) => ({
473-
name,
474-
value: "",
475-
options: removeCookieOptions,
476-
})),
477-
...setCookies.map(({ name, value }) => ({
478-
name,
479-
value,
480-
options: setCookieOptions,
481-
})),
482-
]);
471+
try {
472+
await setAll([
473+
...removeCookies.map((name) => ({
474+
name,
475+
value: "",
476+
options: removeCookieOptions,
477+
})),
478+
...setCookies.map(({ name, value }) => ({
479+
name,
480+
value,
481+
options: setCookieOptions,
482+
})),
483+
]);
484+
} catch (error) {
485+
// Better explain the case where cookies cannot be set because the response
486+
// has already been sent. This can happen when token refresh completes
487+
// asynchronously after the SSR framework has already generated and sent
488+
// the HTTP response.
489+
if (
490+
error instanceof Error &&
491+
(error.message.includes("after the response") ||
492+
error.message.includes("response has been generated"))
493+
) {
494+
console.error(
495+
"@supabase/ssr: Cannot set cookies after response has been sent. " +
496+
"Token refresh completed too late in the request lifecycle. " +
497+
"This should be prevented by the automatic session initialization, " +
498+
"but if you're seeing this error, please report it as a bug.",
499+
);
500+
// Don't throw - this prevents crashes but tokens won't be persisted
501+
// until the next request
502+
throw error;
503+
}
504+
// Re-throw other errors as they indicate a different problem
505+
throw error;
506+
}
483507
}

src/createServerClient.spec.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,4 +380,42 @@ describe("createServerClient", () => {
380380
});
381381
});
382382
});
383+
384+
describe("proactive session initialization", () => {
385+
it("should automatically call getSession to prevent race conditions", async () => {
386+
let getSessionCalled = false;
387+
388+
const supabase = createServerClient(
389+
"https://project-ref.supabase.co",
390+
"publishable-key",
391+
{
392+
cookies: {
393+
getAll() {
394+
return [];
395+
},
396+
setAll() {},
397+
},
398+
global: {
399+
fetch: async () => {
400+
throw new Error("Should not be called in this test");
401+
},
402+
},
403+
},
404+
);
405+
406+
// Spy on getSession
407+
const originalGetSession = supabase.auth.getSession.bind(
408+
supabase.auth,
409+
);
410+
supabase.auth.getSession = async () => {
411+
getSessionCalled = true;
412+
return originalGetSession();
413+
};
414+
415+
// Wait for queue to execute
416+
await new Promise((resolve) => setTimeout(resolve, 50));
417+
418+
expect(getSessionCalled).toBe(true);
419+
});
420+
});
383421
});

src/createServerClient.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,5 +209,21 @@ export function createServerClient<
209209
}
210210
});
211211

212+
// Proactively load the session to trigger any necessary token refresh
213+
// synchronously during request processing, before the response is generated.
214+
// This prevents a race condition where async token refresh completes after
215+
// the HTTP response has already been sent, which would cause cookie setting
216+
// to fail.
217+
//
218+
// Promise.resolve().then() is used which means it executes after the current
219+
// synchronous code. This ensures the session is initialized early in the
220+
// request lifecycle without blocking the client creation.
221+
Promise.resolve().then(() => {
222+
client.auth.getSession().catch(() => {
223+
// Ignore errors - if session loading fails, the client is still usable
224+
// and subsequent auth operations will handle the error appropriately
225+
});
226+
});
227+
212228
return client;
213229
}

0 commit comments

Comments
 (0)