Skip to content

Commit 0dd7496

Browse files
Brooooooklynclaude
andcommitted
fix(cli): treat non-retryable 4xx (incl. 426 cli_outdated) as fatal while following
Reviewer (Codex #244) flagged that a permanent platform error like the version gate's 426 cli_outdated would be retried forever under `void build logs -f` (isFatal matched only 401/403/404). The gate isn't wired to the build-logs routes today so it's unreachable, but harden defensively to match the rest of the CLI, which always treats 426 as terminal: extract an exported isFatalFollowError that classifies any 4xx except 429 (rate limit, transient) as fatal. Surfaces the verbatim upgrade message and exits instead of looping. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 4bb838a commit 0dd7496

2 files changed

Lines changed: 86 additions & 9 deletions

File tree

‎packages/void/src/cli/build-cmd.ts‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,21 @@ function sleep(ms: number): Promise<void> {
1313
return new Promise((resolve) => setTimeout(resolve, ms));
1414
}
1515

16+
/**
17+
* Classifies a `getBuildLogs` failure during a follow/first-page fetch as fatal
18+
* (surface + exit) vs transient (retry). A client error (4xx) is permanent —
19+
* the build is gone, access was revoked, or the CLI is too old (426
20+
* `cli_outdated`) — so retrying can't fix it; it is fatal, matching how the rest
21+
* of the CLI surfaces 426 (see `deploy-errors.ts`, `github-cmd.ts`). The sole
22+
* exception is 429 (rate limit), which IS transient. Everything else
23+
* (5xx/timeout/DO-read blip) is transient and retried through.
24+
*/
25+
export function isFatalFollowError(err: unknown): boolean {
26+
return (
27+
err instanceof PlatformApiError && err.status >= 400 && err.status < 500 && err.status !== 429
28+
);
29+
}
30+
1631
/**
1732
* Pure follow-loop: repeatedly fetch pages since a cursor, write each new
1833
* line, and stop when the build reaches a terminal status. `fetchPage`,
@@ -121,12 +136,8 @@ async function runBuildLogsCommand(
121136
? (line: string): void => appendFileSync(args.output!, `${line}\n`)
122137
: (line: string): void => console.log(line);
123138

124-
// Auth/not-found errors are fatal (the build is gone or access was revoked);
125-
// everything else is a transient blip we retry through. Hoisted so the SAME
139+
// `isFatalFollowError` decides fatal vs transient (see its doc). The SAME
126140
// policy governs both the first-page fetch below and the follow loop.
127-
const isFatal = (err: unknown): boolean =>
128-
err instanceof PlatformApiError &&
129-
(err.status === 401 || err.status === 403 || err.status === 404);
130141
const onRetry = (err: unknown): void =>
131142
log.warn(
132143
`build: transient error while following (${err instanceof Error ? err.message : String(err)}); retrying…`,
@@ -142,7 +153,7 @@ async function runBuildLogsCommand(
142153
first = await client.getBuildLogs(projectId, buildId, 0);
143154
break;
144155
} catch (err) {
145-
if (isFatal(err)) {
156+
if (isFatalFollowError(err)) {
146157
if (err instanceof PlatformApiError && err.status === 404) {
147158
log.error(`build: Build '${buildId}' not found for this project.`);
148159
} else {
@@ -187,7 +198,7 @@ async function runBuildLogsCommand(
187198
sleep,
188199
since: first.lastSeq,
189200
status,
190-
isFatal,
201+
isFatal: isFatalFollowError,
191202
onRetry,
192203
}));
193204
}

‎packages/void/test/unit/build-logs-follow.test.ts‎

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,36 @@
11
import { describe, expect, it } from 'vite-plus/test';
2-
import { followBuildLogs } from '../../src/cli/build-cmd.ts';
3-
import type { BuildLogsResponse } from '../../src/cli/client.ts';
2+
import { followBuildLogs, isFatalFollowError } from '../../src/cli/build-cmd.ts';
3+
import { PlatformApiError, type BuildLogsResponse } from '../../src/cli/client.ts';
44

55
function page(partial: Partial<BuildLogsResponse>): BuildLogsResponse {
66
return { lines: [], lastSeq: 0, status: 'building', terminal: false, external: null, ...partial };
77
}
88

9+
function apiError(status: number, body = '{}'): PlatformApiError {
10+
return new PlatformApiError('build logs', status, body);
11+
}
12+
13+
describe('isFatalFollowError', () => {
14+
it('treats every 4xx except 429 as fatal (including 426 cli_outdated)', () => {
15+
// 426 is the reviewer's case: a too-old CLI must exit, not retry forever.
16+
for (const status of [400, 401, 403, 404, 409, 410, 422, 426]) {
17+
expect(isFatalFollowError(apiError(status))).toBe(true);
18+
}
19+
});
20+
21+
it('treats 429 (rate limit) as transient — the tail must survive it', () => {
22+
expect(isFatalFollowError(apiError(429))).toBe(false);
23+
});
24+
25+
it('treats 5xx and non-PlatformApiError failures as transient', () => {
26+
expect(isFatalFollowError(apiError(500))).toBe(false);
27+
expect(isFatalFollowError(apiError(502))).toBe(false);
28+
expect(isFatalFollowError(apiError(503))).toBe(false);
29+
expect(isFatalFollowError(new Error('network blip'))).toBe(false);
30+
expect(isFatalFollowError('nope')).toBe(false);
31+
});
32+
});
33+
934
describe('followBuildLogs', () => {
1035
it('polls from the cursor, writes new lines in order, and stops when terminal', async () => {
1136
const pages: Array<BuildLogsResponse> = [
@@ -97,4 +122,45 @@ describe('followBuildLogs', () => {
97122
}),
98123
).rejects.toThrow('unauthorized');
99124
});
125+
126+
it('under the real predicate, a 426 cli_outdated rethrows (surfacing the upgrade message)', async () => {
127+
await expect(
128+
followBuildLogs({
129+
fetchPage: () =>
130+
Promise.reject(
131+
apiError(426, '{"error":"cli_outdated","message":"Upgrade: npm i -g void@latest"}'),
132+
),
133+
write: () => {},
134+
sleep: () => Promise.resolve(),
135+
since: 1,
136+
status: 'building',
137+
pollMs: 0,
138+
isFatal: isFatalFollowError,
139+
}),
140+
).rejects.toThrow('Upgrade: npm i -g void@latest');
141+
});
142+
143+
it('under the real predicate, a 429 is retried (not fatal)', async () => {
144+
let call = 0;
145+
const retries: Array<unknown> = [];
146+
const result = await followBuildLogs({
147+
fetchPage: () => {
148+
call += 1;
149+
if (call === 1) {
150+
return Promise.reject(apiError(429, '{"error":"rate_limited"}'));
151+
}
152+
return Promise.resolve(page({ lastSeq: 5, status: 'complete', terminal: true }));
153+
},
154+
write: () => {},
155+
sleep: () => Promise.resolve(),
156+
since: 4,
157+
status: 'building',
158+
pollMs: 0,
159+
isFatal: isFatalFollowError,
160+
onRetry: (err) => retries.push(err),
161+
});
162+
163+
expect(retries).toHaveLength(1); // the 429 was retried, not thrown
164+
expect(result).toEqual({ status: 'complete', terminal: true });
165+
});
100166
});

0 commit comments

Comments
 (0)