Skip to content

Commit a85abd2

Browse files
jorgemoyaclaude
andcommitted
fix(cli) - Don't frame clear user-actionable errors as bugs to report
A clear 4xx API response (validation error, not-found, not-enabled, conflict) or bad command input was printed through the top-level handler's bug-report path, appending a Correlation ID and "share this with BigCommerce support" to an already-actionable message. Add a reusable UserActionableError base class that the top-level handler prints plainly (UnauthorizedError now extends it). Adopt it across the API clients for their 4xx / input-validation cases: - domains: 4xx responses, "API not enabled", ownership-verification errors - project: validation errors, "not enabled", project-not-found - logs: "not enabled", project-not-found, invalid query, bad time-window input - channel: channel-site auth/scope failures - auth/login: missing or unvalidatable credentials 5xx server-side failures and unexpected errors stay plain Errors and keep the Correlation ID + support framing. Refs LTRAC-1115 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f500436 commit a85abd2

11 files changed

Lines changed: 125 additions & 41 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@bigcommerce/catalyst": patch
3+
---
4+
5+
Stop framing clear, user-actionable CLI errors as bugs to report. Validation errors, not-found and not-enabled responses, conflicts, and bad command input now print just the message and exit, instead of appending a Correlation ID and a "share this with BigCommerce support" prompt. That framing is now reserved for genuine server-side (5xx) failures and unexpected errors. Applies across the `domains`, `project`, `channel`, `logs`, and `auth` commands via a shared `UserActionableError` type.

packages/catalyst/src/cli/commands/domains.spec.ts

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { afterAll, afterEach, beforeAll, describe, expect, MockInstance, test, v
66

77
import { server } from '../../../tests/mocks/node';
88
import { claimDomain, createDomain, deleteDomain, getDomain, listDomains } from '../lib/domains';
9+
import { UserActionableError } from '../lib/errors';
910
import { consola } from '../lib/logger';
1011
import { mkTempDir } from '../lib/mk-temp-dir';
1112
import { getProjectConfig, ProjectConfigSchema } from '../lib/project-config';
@@ -272,13 +273,50 @@ describe('domain API client', () => {
272273
),
273274
);
274275

275-
await expect(
276-
createDomain(domain, projectUuid, storeHash, accessToken, apiHost),
277-
).rejects.toThrow(
276+
const error = await createDomain(domain, projectUuid, storeHash, accessToken, apiHost).catch(
277+
(err: unknown) => err,
278+
);
279+
280+
expect(error).toBeInstanceOf(Error);
281+
// A 5xx is a server-side failure worth escalating, so it stays a plain Error
282+
// and keeps the top-level Correlation ID + support framing.
283+
expect(error).not.toBeInstanceOf(UserActionableError);
284+
expect(error).toHaveProperty(
285+
'message',
278286
'Failed to add domain: 502 Bad Gateway. This is a server-side response from the Domains API.',
279287
);
280288
});
281289

290+
test('treats a 4xx conflict as user-actionable (no support/correlation framing)', async () => {
291+
server.use(
292+
http.post(
293+
'https://:apiHost/stores/:storeHash/v3/infrastructure/projects/:projectUuid/domains/:domain/claim',
294+
() =>
295+
HttpResponse.json(
296+
{
297+
status: 409,
298+
title:
299+
'The domain is already bound to another project in this store. Use the transfer endpoint to move it.',
300+
errors: {
301+
domain: `'${domain}' is already bound to another project in this store.`,
302+
},
303+
},
304+
{ status: 409 },
305+
),
306+
),
307+
);
308+
309+
const error = await claimDomain(domain, projectUuid, storeHash, accessToken, apiHost).catch(
310+
(err: unknown) => err,
311+
);
312+
313+
expect(error).toBeInstanceOf(UserActionableError);
314+
expect(error).toHaveProperty(
315+
'message',
316+
expect.stringContaining('already bound to another project in this store'),
317+
);
318+
});
319+
282320
test('lists domains for a project', async () => {
283321
await expect(listDomains(projectUuid, storeHash, accessToken, apiHost)).resolves.toEqual([
284322
{

packages/catalyst/src/cli/index.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/usr/bin/env node
2-
import { UnauthorizedError } from './lib/auth-errors';
2+
import { UserActionableError } from './lib/errors';
33
import { consola } from './lib/logger';
44
import { getTelemetry } from './lib/telemetry';
55
import { program } from './program';
@@ -22,9 +22,11 @@ const handleFatalError = async (error: unknown) => {
2222
// Don't mask the original error
2323
}
2424

25-
// An invalid/expired token is user-actionable, not a bug to report — print the
26-
// re-auth guidance without the "share your Correlation ID with support" noise.
27-
if (error instanceof UnauthorizedError) {
25+
// A user-actionable error (invalid/expired token, a clear 4xx validation or
26+
// conflict response, a feature that isn't enabled) already tells the user what
27+
// to do — print it without the "share your Correlation ID with support" noise
28+
// that only helps for genuine bugs and server-side failures.
29+
if (error instanceof UserActionableError) {
2830
consola.error(errorMessage);
2931
process.exit(1);
3032
}

packages/catalyst/src/cli/lib/auth-errors.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { UserActionableError } from './errors';
2+
13
// Thrown when an authenticated API call comes back 401 Unauthorized — i.e. the
24
// access token was sent but rejected. This covers an expired/revoked token as
35
// well as a token that was never valid (e.g. a typo'd `--access-token` flag or
@@ -10,7 +12,7 @@
1012
// let this propagate to the top-level handler in `index.ts`, which prints the
1113
// message without the generic "share your Correlation ID with support"
1214
// bug-report framing.
13-
export class UnauthorizedError extends Error {
15+
export class UnauthorizedError extends UserActionableError {
1416
constructor() {
1517
super(
1618
'Your access token is invalid or has expired.\n' +

packages/catalyst/src/cli/lib/channels.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { z } from 'zod';
22

33
import { assertAuthorized } from './auth-errors';
4+
import { UserActionableError } from './errors';
45
import { getTelemetry } from './telemetry';
56

67
// `origin` is the CLI-API gateway (configured via `--cli-api-origin`, default
@@ -239,7 +240,7 @@ export async function updateChannelSiteUrl(
239240
);
240241

241242
if (response.status === 401 || response.status === 403) {
242-
throw new Error(
243+
throw new UserActionableError(
243244
`Failed to update channel site (${response.status}). Re-run \`catalyst auth login\` to refresh your access token with the store_channel_settings scope.`,
244245
);
245246
}

packages/catalyst/src/cli/lib/domains.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { z } from 'zod';
22

33
import { assertAuthorized } from './auth-errors';
4+
import { UserActionableError } from './errors';
45
import { formatV3Error } from './observability';
56
import { getTelemetry } from './telemetry';
67

@@ -32,7 +33,7 @@ const ownershipVerificationMetaSchema = z.object({
3233
// domain bound to another store has not been verified. Carries the TXT record
3334
// the caller must publish so the command can render actionable next steps
3435
// instead of an opaque error.
35-
export class DomainOwnershipVerificationError extends Error {
36+
export class DomainOwnershipVerificationError extends UserActionableError {
3637
readonly ownershipVerification: OwnershipVerification;
3738

3839
constructor(message: string, ownershipVerification: OwnershipVerification) {
@@ -111,7 +112,7 @@ async function assertDomainResponse(response: Response, action: string): Promise
111112
assertAuthorized(response);
112113

113114
if (response.status === 403) {
114-
throw new Error(DOMAINS_API_NOT_ENABLED);
115+
throw new UserActionableError(DOMAINS_API_NOT_ENABLED);
115116
}
116117

117118
if (response.ok) {
@@ -126,7 +127,14 @@ async function assertDomainResponse(response: Response, action: string): Promise
126127
throw new DomainOwnershipVerificationError(message, ownershipVerification);
127128
}
128129

129-
throw new Error(message);
130+
// 5xx responses are server-side failures worth escalating, so keep the
131+
// Correlation ID + support framing. A 4xx (validation, not-found, conflict)
132+
// is a clear, user-actionable response — surface just the message.
133+
if (response.status >= 500) {
134+
throw new Error(message);
135+
}
136+
137+
throw new UserActionableError(message);
130138
}
131139

132140
export async function createDomain(
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Base class for errors that are the user's to fix and whose message already
2+
// says everything they need (bad input, a naming/ownership conflict, invalid
3+
// credentials, a feature that isn't enabled). The top-level handler in
4+
// `index.ts` prints these plainly and exits non-zero WITHOUT the "share your
5+
// Correlation ID with BigCommerce support" bug-report framing that unexpected
6+
// and server-side errors get — a clear 4xx response is not a bug to report.
7+
export class UserActionableError extends Error {
8+
constructor(message: string) {
9+
super(message);
10+
this.name = 'UserActionableError';
11+
}
12+
}

packages/catalyst/src/cli/lib/login.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import yoctoSpinner from 'yocto-spinner';
55
import { z } from 'zod';
66

77
import { DEVICE_OAUTH_SCOPES, requestDeviceCode, waitForDeviceToken } from './auth';
8+
import { UserActionableError } from './errors';
89
import { consola } from './logger';
910

1011
export interface LoginResult {
@@ -92,14 +93,14 @@ async function manualLogin(apiHost: string): Promise<LoginResult> {
9293
const storeHash = storeHashInput.trim();
9394

9495
if (!storeHash) {
95-
throw new Error('Store hash is required.');
96+
throw new UserActionableError('Store hash is required.');
9697
}
9798

9899
const accessTokenInput = await password({ message: 'Access token:', mask: true });
99100
const accessToken = accessTokenInput.trim();
100101

101102
if (!accessToken) {
102-
throw new Error('Access token is required.');
103+
throw new UserActionableError('Access token is required.');
103104
}
104105

105106
const spinner = yoctoSpinner().start('Validating credentials...');
@@ -113,7 +114,7 @@ async function manualLogin(apiHost: string): Promise<LoginResult> {
113114

114115
const message = error instanceof Error ? error.message : String(error);
115116

116-
throw new Error(
117+
throw new UserActionableError(
117118
`Could not validate credentials (${message}). Double-check your store hash and access token, then try again.`,
118119
);
119120
}

packages/catalyst/src/cli/lib/observability.spec.ts

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { afterAll, afterEach, beforeAll, describe, expect, test, vi } from 'vite
33

44
import { server } from '../../../tests/mocks/node';
55

6+
import { UserActionableError } from './errors';
67
import { consola } from './logger';
78
import { formatLogEntry, formatV3Error, queryLogs, resolveTimeWindow } from './observability';
89

@@ -34,6 +35,10 @@ describe('resolveTimeWindow', () => {
3435
);
3536
});
3637

38+
test('bad time-window input is user-actionable (no support/correlation framing)', () => {
39+
expect(() => resolveTimeWindow({ since: 'yesterday' })).toThrow(UserActionableError);
40+
});
41+
3742
test('defaults end to now when omitted', () => {
3843
expect(resolveTimeWindow({ start: '2026-06-11T00:00:00Z' }, nowMs)).toEqual({
3944
start: '2026-06-11T00:00:00Z',
@@ -418,12 +423,14 @@ describe('queryLogs', () => {
418423
test('maps 404 to a project-not-found error', async () => {
419424
server.use(http.get(logsUrl, () => new HttpResponse(null, { status: 404 })));
420425

421-
await expect(
422-
queryLogs(projectUuid, storeHash, accessToken, apiHost, {
423-
start: '2026-06-01T00:00:00Z',
424-
end: '2026-06-02T00:00:00Z',
425-
}),
426-
).rejects.toThrow('Project not found');
426+
const error = await queryLogs(projectUuid, storeHash, accessToken, apiHost, {
427+
start: '2026-06-01T00:00:00Z',
428+
end: '2026-06-02T00:00:00Z',
429+
}).catch((err: unknown) => err);
430+
431+
// A 4xx is a clear, user-actionable response — no Correlation ID/support framing.
432+
expect(error).toBeInstanceOf(UserActionableError);
433+
expect(error).toHaveProperty('message', expect.stringContaining('Project not found'));
427434
});
428435

429436
test('surfaces the v3 error message on 422', async () => {
@@ -453,11 +460,15 @@ describe('queryLogs', () => {
453460
http.get(logsUrl, () => new HttpResponse(null, { status: 500, statusText: 'Server Error' })),
454461
);
455462

456-
await expect(
457-
queryLogs(projectUuid, storeHash, accessToken, apiHost, {
458-
start: '2026-06-01T00:00:00Z',
459-
end: '2026-06-02T00:00:00Z',
460-
}),
461-
).rejects.toThrow('Failed to fetch logs: 500 Server Error');
463+
const error = await queryLogs(projectUuid, storeHash, accessToken, apiHost, {
464+
start: '2026-06-01T00:00:00Z',
465+
end: '2026-06-02T00:00:00Z',
466+
}).catch((err: unknown) => err);
467+
468+
// A 5xx is a server-side failure worth escalating, so it stays a plain Error
469+
// and keeps the top-level Correlation ID + support framing.
470+
expect(error).toBeInstanceOf(Error);
471+
expect(error).not.toBeInstanceOf(UserActionableError);
472+
expect(error).toHaveProperty('message', 'Failed to fetch logs: 500 Server Error');
462473
});
463474
});

packages/catalyst/src/cli/lib/observability.ts

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { colorize } from 'consola/utils';
22
import { z } from 'zod';
33

44
import { assertAuthorized } from './auth-errors';
5+
import { UserActionableError } from './errors';
56
import { getTelemetry } from './telemetry';
67

78
export const LOG_LEVELS = ['debug', 'info', 'warn', 'error'] as const;
@@ -111,7 +112,7 @@ export function resolveTimeWindow(
111112
endMs = parseTimeInput(end);
112113

113114
if (endMs === null) {
114-
throw new Error(
115+
throw new UserActionableError(
115116
`Invalid --end value "${end}". Provide an ISO-8601 timestamp or a Unix epoch (seconds).`,
116117
);
117118
}
@@ -124,7 +125,7 @@ export function resolveTimeWindow(
124125
const durationMs = parseDuration(since);
125126

126127
if (durationMs === null) {
127-
throw new Error(
128+
throw new UserActionableError(
128129
`Invalid --since value "${since}". Provide a duration like 30m, 6h, or 2d (units: s, m, h, d).`,
129130
);
130131
}
@@ -135,20 +136,22 @@ export function resolveTimeWindow(
135136
startMs = parseTimeInput(start);
136137

137138
if (startMs === null) {
138-
throw new Error(
139+
throw new UserActionableError(
139140
`Invalid --start value "${start}". Provide an ISO-8601 timestamp or a Unix epoch (seconds).`,
140141
);
141142
}
142143
} else {
143-
throw new Error('Provide a time window with --since <duration> or --start <time>.');
144+
throw new UserActionableError(
145+
'Provide a time window with --since <duration> or --start <time>.',
146+
);
144147
}
145148

146149
if (startMs > endMs) {
147-
throw new Error('Invalid time window: --start must be before or equal to --end.');
150+
throw new UserActionableError('Invalid time window: --start must be before or equal to --end.');
148151
}
149152

150153
if (endMs - startMs > SEVEN_DAYS_MS) {
151-
throw new Error('Invalid time window: the range must not exceed 7 days.');
154+
throw new UserActionableError('Invalid time window: the range must not exceed 7 days.');
152155
}
153156

154157
return { start, end };
@@ -246,21 +249,21 @@ export async function queryLogs(
246249
assertAuthorized(response);
247250

248251
if (response.status === 403) {
249-
throw new Error(
252+
throw new UserActionableError(
250253
'Infrastructure Logs API not enabled. If you are part of the beta, contact support@bigcommerce.com to enable it.',
251254
);
252255
}
253256

254257
if (response.status === 404) {
255-
throw new Error('Project not found. Check the project UUID.');
258+
throw new UserActionableError('Project not found. Check the project UUID.');
256259
}
257260

258261
// 400 (bad UUID) and 422 (invalid window/filter) both carry the field-keyed
259262
// v3 error envelope — surface its message rather than the bare status.
260263
if (response.status === 400 || response.status === 422) {
261264
const body: unknown = await response.json().catch(() => null);
262265

263-
throw new Error(formatV3Error(body) ?? 'Invalid log query.');
266+
throw new UserActionableError(formatV3Error(body) ?? 'Invalid log query.');
264267
}
265268

266269
if (!response.ok) {

0 commit comments

Comments
 (0)