Skip to content

Commit 525b842

Browse files
committed
fix(core): wire token refresh into run shape streams and bound re-minting
Pass the refresh resolver to the runShapeStream factory so subscribeToRun, subscribeToRunsWithTag and subscribeToBatch recover from an expired token. Only re-arm the refresh after a connection delivers a record, keep the auth error terminal when the refresher throws, and don't report a 401 the refresh recovered from.
1 parent 764f1f3 commit 525b842

4 files changed

Lines changed: 252 additions & 9 deletions

File tree

packages/core/src/v3/apiClient/index.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,17 @@ export class ApiClient {
299299
};
300300
}
301301

302+
/** As {@link ApiClient.#resolveStreamHeaders}, for the leaner realtime header set. */
303+
#resolveRealtimeHeaders(): (() => Promise<Record<string, string>>) | undefined {
304+
const refreshAccessToken = this.refreshAccessToken;
305+
if (!refreshAccessToken) return undefined;
306+
307+
return async () => {
308+
const accessToken = await refreshAccessTokenOnce(refreshAccessToken);
309+
return { ...this.#getRealtimeHeaders(), Authorization: `Bearer ${accessToken}` };
310+
};
311+
}
312+
302313
async getRunResult(
303314
runId: string,
304315
requestOptions?: ZodFetchOptions
@@ -1685,6 +1696,7 @@ export class ApiClient {
16851696
closeOnComplete:
16861697
typeof options?.closeOnComplete === "boolean" ? options.closeOnComplete : true,
16871698
headers: this.#getRealtimeHeaders(),
1699+
resolveHeaders: this.#resolveRealtimeHeaders(),
16881700
client: this,
16891701
signal: options?.signal,
16901702
onFetchError: options?.onFetchError,
@@ -1708,6 +1720,7 @@ export class ApiClient {
17081720
{
17091721
closeOnComplete: false,
17101722
headers: this.#getRealtimeHeaders(),
1723+
resolveHeaders: this.#resolveRealtimeHeaders(),
17111724
client: this,
17121725
signal: options?.signal,
17131726
onFetchError: options?.onFetchError,
@@ -1734,6 +1747,7 @@ export class ApiClient {
17341747
{
17351748
closeOnComplete: false,
17361749
headers: this.#getRealtimeHeaders(),
1750+
resolveHeaders: this.#resolveRealtimeHeaders(),
17371751
client: this,
17381752
signal: options?.signal,
17391753
onFetchError: options?.onFetchError,
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import { describe, expect, it } from "vitest";
2+
import { refreshAccessTokenOnce } from "./refreshAccessToken.js";
3+
4+
describe("refreshAccessTokenOnce", () => {
5+
it("shares one in-flight mint between concurrent callers", async () => {
6+
let calls = 0;
7+
let release: (token: string) => void = () => {};
8+
const refresh = () => {
9+
calls++;
10+
return new Promise<string>((resolve) => {
11+
release = resolve;
12+
});
13+
};
14+
15+
const results = Promise.all([
16+
refreshAccessTokenOnce(refresh),
17+
refreshAccessTokenOnce(refresh),
18+
refreshAccessTokenOnce(refresh),
19+
]);
20+
release("fresh");
21+
22+
expect(await results).toEqual(["fresh", "fresh", "fresh"]);
23+
expect(calls).toBe(1);
24+
});
25+
26+
it("does not share a mint between different refreshers", async () => {
27+
const a = async () => "a";
28+
const b = async () => "b";
29+
30+
expect(await Promise.all([refreshAccessTokenOnce(a), refreshAccessTokenOnce(b)])).toEqual([
31+
"a",
32+
"b",
33+
]);
34+
});
35+
36+
it("mints again once the previous call has settled", async () => {
37+
let calls = 0;
38+
const refresh = async () => `token-${++calls}`;
39+
40+
expect(await refreshAccessTokenOnce(refresh)).toBe("token-1");
41+
expect(await refreshAccessTokenOnce(refresh)).toBe("token-2");
42+
});
43+
44+
it("rejects every concurrent caller and does not poison later calls", async () => {
45+
let calls = 0;
46+
const refresh = async () => {
47+
calls++;
48+
if (calls === 1) throw new Error("mint failed");
49+
return "recovered";
50+
};
51+
52+
const first = refreshAccessTokenOnce(refresh);
53+
const second = refreshAccessTokenOnce(refresh);
54+
55+
await expect(first).rejects.toThrow("mint failed");
56+
await expect(second).rejects.toThrow("mint failed");
57+
expect(calls).toBe(1);
58+
59+
// The failed mint was evicted, so the next call tries again.
60+
expect(await refreshAccessTokenOnce(refresh)).toBe("recovered");
61+
expect(calls).toBe(2);
62+
});
63+
});

packages/core/src/v3/apiClient/runStream.test.ts

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,33 @@ describe("SSEStreamSubscription retry behavior", () => {
4343
});
4444
}
4545

46+
/** An accepted connection that dies before delivering a single record. */
47+
function makeDroppedResponse() {
48+
const body = new ReadableStream<Uint8Array>({
49+
start(controller) {
50+
controller.error(new Error("connection dropped"));
51+
},
52+
});
53+
return new Response(body, {
54+
status: 200,
55+
headers: { "Content-Type": "text/event-stream", "X-Stream-Version": "v1" },
56+
});
57+
}
58+
59+
/** One delivered record, then the connection dies. */
60+
function makeChunkThenDropResponse() {
61+
const body = new ReadableStream<Uint8Array>({
62+
start(controller) {
63+
controller.enqueue(new TextEncoder().encode(`id: 1\ndata: {"hello":1}\n\n`));
64+
setTimeout(() => controller.error(new Error("connection dropped")), 20);
65+
},
66+
});
67+
return new Response(body, {
68+
status: 200,
69+
headers: { "Content-Type": "text/event-stream", "X-Stream-Version": "v1" },
70+
});
71+
}
72+
4673
// Drain a ReadableStream<SSEStreamPart> until it closes or errors.
4774
// Returns received chunks plus terminal state.
4875
async function drain(stream: ReadableStream<{ id: string; chunk: unknown }>) {
@@ -496,6 +523,127 @@ describe("SSEStreamSubscription retry behavior", () => {
496523
expect(result.error).toBeDefined();
497524
});
498525

526+
it("retries a 403 once with the headers from resolveHeaders", async () => {
527+
const seenTokens: Array<string | null> = [];
528+
globalThis.fetch = vi.fn().mockImplementation(async (_url: string, init: RequestInit) => {
529+
const token = new Headers(init.headers).get("Authorization");
530+
seenTokens.push(token);
531+
if (token !== "Bearer fresh") return new Response("forbidden", { status: 403 });
532+
return makeSSEResponse();
533+
});
534+
535+
const sub = new SSEStreamSubscription("http://example.test/sse", {
536+
headers: { Authorization: "Bearer expired" },
537+
retryDelayMs: 1,
538+
maxRetryDelayMs: 5,
539+
resolveHeaders: async () => ({ Authorization: "Bearer fresh" }),
540+
});
541+
542+
const result = await sub.subscribe().then(drain);
543+
expect(seenTokens).toEqual(["Bearer expired", "Bearer fresh"]);
544+
expect(result.error).toBeUndefined();
545+
expect(result.chunks).toHaveLength(1);
546+
});
547+
548+
it("does not report a 401 that the refresh recovered from", async () => {
549+
let attempts = 0;
550+
globalThis.fetch = vi.fn().mockImplementation(async () => {
551+
attempts++;
552+
if (attempts === 1) return new Response("unauthorized", { status: 401 });
553+
return makeSSEResponse();
554+
});
555+
556+
const errors: Error[] = [];
557+
const sub = new SSEStreamSubscription("http://example.test/sse", {
558+
headers: { Authorization: "Bearer expired" },
559+
retryDelayMs: 1,
560+
maxRetryDelayMs: 5,
561+
onError: (e) => errors.push(e),
562+
resolveHeaders: async () => ({ Authorization: "Bearer fresh" }),
563+
});
564+
565+
const result = await sub.subscribe().then(drain);
566+
expect(errors).toHaveLength(0);
567+
expect(result.error).toBeUndefined();
568+
});
569+
570+
it("terminates on a 401 when the refresher itself throws", async () => {
571+
let attempts = 0;
572+
globalThis.fetch = vi.fn().mockImplementation(async () => {
573+
attempts++;
574+
return new Response("unauthorized", { status: 401 });
575+
});
576+
577+
const errors: Error[] = [];
578+
const sub = new SSEStreamSubscription("http://example.test/sse", {
579+
headers: { Authorization: "Bearer expired" },
580+
retryDelayMs: 1,
581+
maxRetryDelayMs: 5,
582+
onError: (e) => errors.push(e),
583+
resolveHeaders: async () => {
584+
throw new Error("mint failed");
585+
},
586+
});
587+
588+
const result = await sub.subscribe().then(drain);
589+
expect(attempts).toBe(1);
590+
expect(errors).toHaveLength(1);
591+
expect(result.error).toBeDefined();
592+
});
593+
594+
it("does not re-mint for a connection that is accepted but delivers nothing", async () => {
595+
let attempts = 0;
596+
globalThis.fetch = vi.fn().mockImplementation(async () => {
597+
attempts++;
598+
// Accept the second attempt, then drop it without sending a record.
599+
if (attempts === 2) return makeDroppedResponse();
600+
return new Response("unauthorized", { status: 401 });
601+
});
602+
603+
let refreshes = 0;
604+
const sub = new SSEStreamSubscription("http://example.test/sse", {
605+
headers: { Authorization: "Bearer expired" },
606+
retryDelayMs: 1,
607+
maxRetryDelayMs: 5,
608+
resolveHeaders: async () => {
609+
refreshes++;
610+
return { Authorization: `Bearer fresh-${refreshes}` };
611+
},
612+
});
613+
614+
const result = await sub.subscribe().then(drain);
615+
expect(refreshes).toBe(1);
616+
expect(attempts).toBe(3);
617+
expect(result.error).toBeDefined();
618+
});
619+
620+
it("allows another refresh once a connection has delivered a record", async () => {
621+
let attempts = 0;
622+
globalThis.fetch = vi.fn().mockImplementation(async () => {
623+
attempts++;
624+
if (attempts === 2) return makeChunkThenDropResponse();
625+
if (attempts === 4) return makeSSEResponse();
626+
return new Response("unauthorized", { status: 401 });
627+
});
628+
629+
let refreshes = 0;
630+
const sub = new SSEStreamSubscription("http://example.test/sse", {
631+
headers: { Authorization: "Bearer expired" },
632+
retryDelayMs: 1,
633+
maxRetryDelayMs: 5,
634+
resolveHeaders: async () => {
635+
refreshes++;
636+
return { Authorization: `Bearer fresh-${refreshes}` };
637+
},
638+
});
639+
640+
const result = await sub.subscribe().then(drain);
641+
expect(refreshes).toBe(2);
642+
expect(attempts).toBe(4);
643+
expect(result.error).toBeUndefined();
644+
expect(result.chunks).toHaveLength(2);
645+
});
646+
499647
it("retries on 503 (caller-tunable nonRetryableStatuses)", async () => {
500648
let attempts = 0;
501649
globalThis.fetch = vi.fn().mockImplementation(async () => {

packages/core/src/v3/apiClient/runStream.ts

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ export type RunStreamCallback<TRunTypes extends AnyRunTypes> = (
8282

8383
export type RunShapeStreamOptions = {
8484
headers?: Record<string, string>;
85+
resolveHeaders?: () => Promise<Record<string, string>>;
8586
fetchClient?: typeof fetch;
8687
closeOnComplete?: boolean;
8788
signal?: AbortSignal;
@@ -114,6 +115,7 @@ export function runShapeStream<TRunTypes extends AnyRunTypes>(
114115
getEnvVar("TRIGGER_STREAM_URL", getEnvVar("TRIGGER_API_URL")) ?? "https://api.trigger.dev",
115116
{
116117
headers: options?.headers,
118+
resolveHeaders: options?.resolveHeaders,
117119
signal: abortController.signal,
118120
}
119121
);
@@ -420,12 +422,16 @@ export class SSEStreamSubscription implements StreamSubscription {
420422
"Could not subscribe to stream",
421423
Object.fromEntries(response.headers)
422424
);
423-
this.options.onError?.(error);
424425
if (this.nonRetryableStatuses.has(response.status)) {
426+
this.options.onError?.(error);
425427
controller.error(error);
426428
return;
427429
}
428-
await this.refreshHeadersForAuthError(response.status);
430+
// Only surface the error if we can't recover from it — a 401 that a
431+
// token refresh fixes shouldn't reach the consumer.
432+
if (!(await this.refreshHeadersForAuthError(response.status))) {
433+
this.options.onError?.(error);
434+
}
429435
throw error;
430436
}
431437

@@ -438,7 +444,6 @@ export class SSEStreamSubscription implements StreamSubscription {
438444
const streamVersion = response.headers.get("X-Stream-Version") ?? "v1";
439445
this.sessionSettled = response.headers.get("X-Session-Settled") === "true";
440446
this.retryCount = 0; // reset on success
441-
this.authRefreshed = false;
442447
armStall();
443448

444449
// Dedup window for record ids. Bounded with FIFO eviction so a
@@ -554,6 +559,10 @@ export class SSEStreamSubscription implements StreamSubscription {
554559
}
555560

556561
armStall(); // any chunk (including server keepalives) resets the silence timer
562+
// The connection is genuinely live, so a later expiry may refresh
563+
// again. Gated on a delivered chunk: a server that accepts then
564+
// immediately drops must not win an unbounded mint loop.
565+
this.authRefreshed = false;
557566
controller.enqueue(value);
558567
}
559568
} catch (error) {
@@ -590,16 +599,25 @@ export class SSEStreamSubscription implements StreamSubscription {
590599

591600
/**
592601
* Re-resolve the headers after a 401/403 so the retry carries a fresh token.
593-
* At most once per connection: if the refreshed token is rejected too, the
594-
* auth error stays terminal.
602+
* At most once per live connection: if the refreshed token is rejected too,
603+
* the auth error stays terminal. Returns true when a retry should follow.
595604
*/
596-
private async refreshHeadersForAuthError(status: number): Promise<void> {
597-
if (status !== 401 && status !== 403) return;
598-
if (!this.options.resolveHeaders || this.authRefreshed) return;
605+
private async refreshHeadersForAuthError(status: number): Promise<boolean> {
606+
if (status !== 401 && status !== 403) return false;
607+
if (!this.options.resolveHeaders || this.authRefreshed) return false;
599608

600609
this.authRefreshed = true;
601-
this.currentHeaders = await this.options.resolveHeaders();
610+
611+
try {
612+
this.currentHeaders = await this.options.resolveHeaders();
613+
} catch {
614+
// A refresher that can't mint leaves us with the rejected token —
615+
// keep the auth error terminal rather than retrying with it.
616+
return false;
617+
}
618+
602619
this.retryAfterAuthRefresh = true;
620+
return true;
603621
}
604622

605623
private async retryConnection(

0 commit comments

Comments
 (0)