Skip to content

Commit dcf24dc

Browse files
Robdel12Robdel12
authored
🐛 Don't fail CI when Vizzly API returns 5xx errors (#190)
## Summary - Add HTTP status code to API error context so commands can check it - Gracefully handle 5xx errors (500, 502, 503, 530 Cloudflare tunnel, etc.) in all CI-facing commands - Warn user but exit 0 so their CI doesn't fail due to Vizzly infrastructure issues - 4xx client errors still fail as expected ## Motivation Users were seeing their CI fail with errors like: ``` Test run failed: API request failed: 530 - <!doctype html>...Cloudflare Tunnel error... ``` This is outside the user's control - their tests ran fine, only the visual comparison couldn't happen because Vizzly's API was temporarily unavailable. ## Changes - `src/api/client.js` - Add `status` to error context - `src/commands/run.js` - Handle 5xx gracefully - `src/commands/upload.js` - Handle 5xx gracefully - `src/commands/status.js` - Handle 5xx gracefully + add DI for testability - `src/commands/finalize.js` - Handle 5xx gracefully - New/updated tests for all affected commands ## Test plan - [x] `npm test` passes (1817 tests) - [x] `npm run lint` passes - [ ] Manual test with simulated 5xx error
1 parent 3d3aff5 commit dcf24dc

11 files changed

Lines changed: 435 additions & 9 deletions

File tree

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ node_modules/
44
# Build outputs
55
dist/
66
!test-site/dist/
7+
.build/
78

89
# Coverage reports
910
coverage/

‎src/api/client.js‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,9 @@ export function createApiClient(options = {}) {
103103

104104
// Other errors
105105
let error = parseApiError(response.status, errorBody, url);
106-
throw new VizzlyError(error.message, error.code);
106+
throw new VizzlyError(error.message, error.code, {
107+
status: error.status,
108+
});
107109
}
108110

109111
return response.json();

‎src/commands/finalize.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,14 @@ export async function finalizeCommand(
9595
return { success: true, result };
9696
} catch (error) {
9797
output.stopSpinner();
98+
99+
// Don't fail CI for Vizzly infrastructure issues (5xx errors)
100+
let status = error.context?.status;
101+
if (status >= 500) {
102+
output.warn('Vizzly API unavailable - finalize skipped.');
103+
return { success: true, result: { skipped: true } };
104+
}
105+
98106
output.error('Failed to finalize parallel build', error);
99107
exit(1);
100108
return { success: false, error };

‎src/commands/run.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,16 @@ export async function runCommand(
397397
} catch (error) {
398398
output.stopSpinner();
399399

400+
// Don't fail CI for Vizzly infrastructure issues (5xx errors)
401+
let status = error.context?.status;
402+
if (status >= 500) {
403+
output.warn(
404+
'Vizzly API unavailable - visual tests skipped. Your tests still ran.'
405+
);
406+
output.debug('api', 'API error details:', { error: error.message });
407+
return { success: true, result: { skipped: true } };
408+
}
409+
400410
// Provide more context about where the error occurred
401411
let errorContext = 'Test run failed';
402412
if (error.message?.includes('build')) {

‎src/commands/status.js‎

Lines changed: 37 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,38 @@
33
* Uses functional API operations directly
44
*/
55

6-
import { createApiClient, getBuild, getPreviewInfo } from '../api/index.js';
7-
import { loadConfig } from '../utils/config-loader.js';
8-
import { getApiUrl } from '../utils/environment-config.js';
9-
import * as output from '../utils/output.js';
6+
import {
7+
createApiClient as defaultCreateApiClient,
8+
getBuild as defaultGetBuild,
9+
getPreviewInfo as defaultGetPreviewInfo,
10+
} from '../api/index.js';
11+
import { loadConfig as defaultLoadConfig } from '../utils/config-loader.js';
12+
import { getApiUrl as defaultGetApiUrl } from '../utils/environment-config.js';
13+
import * as defaultOutput from '../utils/output.js';
1014

1115
/**
1216
* Status command implementation
1317
* @param {string} buildId - Build ID to check status for
1418
* @param {Object} options - Command options
1519
* @param {Object} globalOptions - Global CLI options
20+
* @param {Object} deps - Dependencies for testing
1621
*/
17-
export async function statusCommand(buildId, options = {}, globalOptions = {}) {
22+
export async function statusCommand(
23+
buildId,
24+
options = {},
25+
globalOptions = {},
26+
deps = {}
27+
) {
28+
let {
29+
loadConfig = defaultLoadConfig,
30+
createApiClient = defaultCreateApiClient,
31+
getBuild = defaultGetBuild,
32+
getPreviewInfo = defaultGetPreviewInfo,
33+
getApiUrl = defaultGetApiUrl,
34+
output = defaultOutput,
35+
exit = code => process.exit(code),
36+
} = deps;
37+
1838
output.configure({
1939
json: globalOptions.json,
2040
verbose: globalOptions.verbose,
@@ -31,7 +51,7 @@ export async function statusCommand(buildId, options = {}, globalOptions = {}) {
3151
output.error(
3252
'API token required. Use --token or set VIZZLY_TOKEN environment variable'
3353
);
34-
process.exit(1);
54+
exit(1);
3555
}
3656

3757
// Get build details via functional API
@@ -232,11 +252,20 @@ export async function statusCommand(buildId, options = {}, globalOptions = {}) {
232252

233253
// Exit with appropriate code based on build status
234254
if (build.status === 'failed' || build.failed_jobs > 0) {
235-
process.exit(1);
255+
exit(1);
236256
}
237257
} catch (error) {
258+
// Don't fail CI for Vizzly infrastructure issues (5xx errors)
259+
let status = error.context?.status;
260+
if (status >= 500) {
261+
output.warn('Vizzly API unavailable - status check skipped.');
262+
output.cleanup();
263+
return { success: true, result: { skipped: true } };
264+
}
265+
238266
output.error('Failed to get build status', error);
239-
process.exit(1);
267+
exit(1);
268+
return { success: false, error };
240269
}
241270
}
242271

‎src/commands/upload.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,16 @@ export async function uploadCommand(
263263
output.cleanup();
264264
return { success: true, result };
265265
} catch (error) {
266+
// Don't fail CI for Vizzly infrastructure issues (5xx errors)
267+
let status = error.context?.status;
268+
if (status >= 500) {
269+
output.warn(
270+
'Vizzly API unavailable - upload skipped. Your tests still ran.'
271+
);
272+
output.cleanup();
273+
return { success: true, result: { skipped: true } };
274+
}
275+
266276
// Mark build as failed if we have a buildId and config
267277
if (buildId && config) {
268278
try {

‎tests/api/client.test.js‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,50 @@ describe('api/client', () => {
210210
);
211211
});
212212

213+
it('includes status code in error context for 5xx errors', async () => {
214+
let client = createApiClient({
215+
token: 'test-token',
216+
baseUrl: 'https://api.test',
217+
});
218+
219+
mockFetch.mock.mockImplementation(async () => ({
220+
ok: false,
221+
status: 503,
222+
headers: new Map(),
223+
text: async () => 'Service Unavailable',
224+
}));
225+
226+
await assert.rejects(
227+
() => client.request('/api/test'),
228+
error => {
229+
assert.strictEqual(error.context?.status, 503);
230+
return true;
231+
}
232+
);
233+
});
234+
235+
it('includes status code in error context for 4xx errors', async () => {
236+
let client = createApiClient({
237+
token: 'test-token',
238+
baseUrl: 'https://api.test',
239+
});
240+
241+
mockFetch.mock.mockImplementation(async () => ({
242+
ok: false,
243+
status: 404,
244+
headers: new Map(),
245+
text: async () => 'Not Found',
246+
}));
247+
248+
await assert.rejects(
249+
() => client.request('/api/test'),
250+
error => {
251+
assert.strictEqual(error.context?.status, 404);
252+
return true;
253+
}
254+
);
255+
});
256+
213257
it('throws VizzlyError for 403 forbidden', async () => {
214258
let client = createApiClient({
215259
token: 'test-token',

‎tests/commands/finalize.test.js‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ function createMockOutput() {
1616
info: msg => calls.push({ method: 'info', args: [msg] }),
1717
debug: (msg, data) => calls.push({ method: 'debug', args: [msg, data] }),
1818
error: (msg, err) => calls.push({ method: 'error', args: [msg, err] }),
19+
warn: msg => calls.push({ method: 'warn', args: [msg] }),
1920
success: msg => calls.push({ method: 'success', args: [msg] }),
2021
data: d => calls.push({ method: 'data', args: [d] }),
2122
startSpinner: msg => calls.push({ method: 'startSpinner', args: [msg] }),
@@ -301,5 +302,42 @@ describe('commands/finalize', () => {
301302
assert.strictEqual(capturedOptions.token, 'option-token');
302303
assert.strictEqual(capturedOptions.verbose, true);
303304
});
305+
306+
it('does not fail CI when API returns 5xx error', async () => {
307+
let output = createMockOutput();
308+
let exitCode = null;
309+
310+
let apiError = new Error('API request failed: 502 - Bad Gateway');
311+
apiError.context = { status: 502 };
312+
313+
let result = await finalizeCommand(
314+
'parallel-123',
315+
{},
316+
{},
317+
{
318+
loadConfig: async () => ({
319+
apiKey: 'test-token',
320+
apiUrl: 'https://api.test',
321+
}),
322+
createApiClient: () => ({ request: async () => ({}) }),
323+
finalizeParallelBuild: async () => {
324+
throw apiError;
325+
},
326+
output,
327+
exit: code => {
328+
exitCode = code;
329+
},
330+
}
331+
);
332+
333+
assert.strictEqual(result.success, true);
334+
assert.strictEqual(result.result.skipped, true);
335+
assert.strictEqual(exitCode, null);
336+
assert.ok(
337+
output.calls.some(
338+
c => c.method === 'warn' && c.args[0].includes('API unavailable')
339+
)
340+
);
341+
});
304342
});
305343
});

‎tests/commands/run.test.js‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,102 @@ describe('commands/run', () => {
787787
);
788788
});
789789

790+
it('does not fail CI when API returns 5xx error', async () => {
791+
let output = createMockOutput();
792+
let exitCode = null;
793+
794+
// Simulate a 5xx API error (e.g., Cloudflare 530)
795+
let apiError = new Error('API request failed: 530');
796+
apiError.context = { status: 530 };
797+
798+
let result = await runCommand(
799+
'npm test',
800+
{},
801+
{},
802+
{
803+
loadConfig: async () => {
804+
throw apiError;
805+
},
806+
output,
807+
exit: code => {
808+
exitCode = code;
809+
},
810+
processOn: () => {},
811+
processRemoveListener: () => {},
812+
}
813+
);
814+
815+
// Should succeed (not fail CI)
816+
assert.strictEqual(result.success, true);
817+
assert.strictEqual(result.result.skipped, true);
818+
assert.strictEqual(exitCode, null); // exit(1) should NOT be called
819+
820+
// Should show warning
821+
assert.ok(
822+
output.calls.some(
823+
c => c.method === 'warn' && c.args[0].includes('API unavailable')
824+
)
825+
);
826+
});
827+
828+
it('does not fail CI when API returns 500 error', async () => {
829+
let output = createMockOutput();
830+
let exitCode = null;
831+
832+
let apiError = new Error(
833+
'API request failed: 500 - Internal Server Error'
834+
);
835+
apiError.context = { status: 500 };
836+
837+
let result = await runCommand(
838+
'npm test',
839+
{},
840+
{},
841+
{
842+
loadConfig: async () => {
843+
throw apiError;
844+
},
845+
output,
846+
exit: code => {
847+
exitCode = code;
848+
},
849+
processOn: () => {},
850+
processRemoveListener: () => {},
851+
}
852+
);
853+
854+
assert.strictEqual(result.success, true);
855+
assert.strictEqual(exitCode, null);
856+
});
857+
858+
it('still fails CI for 4xx client errors', async () => {
859+
let output = createMockOutput();
860+
let exitCode = null;
861+
862+
let apiError = new Error('API request failed: 400 - Bad Request');
863+
apiError.context = { status: 400 };
864+
865+
let result = await runCommand(
866+
'npm test',
867+
{},
868+
{},
869+
{
870+
loadConfig: async () => {
871+
throw apiError;
872+
},
873+
output,
874+
exit: code => {
875+
exitCode = code;
876+
},
877+
processOn: () => {},
878+
processRemoveListener: () => {},
879+
}
880+
);
881+
882+
assert.strictEqual(result.success, false);
883+
assert.strictEqual(exitCode, 1);
884+
});
885+
790886
it('SIGINT handler triggers cleanup and exit', async () => {
791887
let output = createMockOutput();
792888
let handlers = {};

0 commit comments

Comments
 (0)