diff --git a/.changeset/split-observe-error-groups.md b/.changeset/split-observe-error-groups.md new file mode 100644 index 00000000000..40495fab4db --- /dev/null +++ b/.changeset/split-observe-error-groups.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': patch +--- + +Split CLI error analytics grouping by command slice and normalized error signature. diff --git a/packages/cli-kit/src/public/node/error-handler.test.ts b/packages/cli-kit/src/public/node/error-handler.test.ts index 8f101ab55d4..cf9810a9a7f 100644 --- a/packages/cli-kit/src/public/node/error-handler.test.ts +++ b/packages/cli-kit/src/public/node/error-handler.test.ts @@ -12,7 +12,7 @@ import {beforeEach, describe, expect, test, vi} from 'vitest' const onNotify = vi.fn() const capturedEventHandler = vi.fn() -let lastBugsnagEvent: {addMetadata: ReturnType} | undefined +let lastBugsnagEvent: {addMetadata: ReturnType; groupingHash?: string} | undefined vi.mock('process') vi.mock('@bugsnag/js', () => { @@ -174,15 +174,6 @@ describe('skips sending errors to Bugsnag', () => { expect(onNotify).not.toHaveBeenCalled() expect(mockOutput.debug()).toMatch('Skipping Bugsnag report') }) - - test('when error is expected', async () => { - const mockOutput = mockAndCaptureOutput() - const res = await sendErrorToBugsnag(new error.AbortError('In test'), 'expected_error') - expect(res.reported).toEqual(false) - expect(res.unhandled).toBeUndefined() - expect(onNotify).not.toHaveBeenCalled() - expect(mockOutput.debug()).toMatch('Skipping Bugsnag report for expected error') - }) }) describe('sends errors to Bugsnag', () => { @@ -209,6 +200,13 @@ describe('sends errors to Bugsnag', () => { expect(onNotify).toHaveBeenCalledWith(res.error) }) + test('processes AbortErrors as handled', async () => { + const res = await sendErrorToBugsnag(new error.AbortError('In test'), 'expected_error') + expect(res.reported).toEqual(true) + expect(res.unhandled).toEqual(false) + expect(onNotify).toHaveBeenCalledWith(res.error) + }) + test.each([null, undefined, {}, {message: 'nope'}])('deals with strange things to throw %s', async (throwable) => { const res = await sendErrorToBugsnag(throwable, 'unexpected_error') expect(res.reported).toEqual(false) @@ -303,4 +301,91 @@ describe('sends errors to Bugsnag', () => { expect(lastBugsnagEvent).toBeDefined() expect(lastBugsnagEvent!.addMetadata).toHaveBeenCalledWith('custom', {slice_name: 'cli'}) }) + + test('sets groupingHash using the command slice and analytics error taxonomy', async () => { + await metadata.addSensitiveMetadata(() => ({ + commandStartOptions: {startTime: Date.now(), startCommand: 'app deploy', startArgs: []}, + })) + + await sendErrorToBugsnag( + new Error( + 'The App Management GraphQL API responded unsuccessfully with the HTTP status 403 and errors: Unauthorized', + ), + 'unexpected_error', + ) + + expect(lastBugsnagEvent).toBeDefined() + expect(lastBugsnagEvent!.groupingHash).toMatch(/^app:authentication:/) + }) + + test('normalizes GraphQL ClientError JSON dumps before setting groupingHash', async () => { + await metadata.addSensitiveMetadata(() => ({ + commandStartOptions: {startTime: Date.now(), startCommand: 'theme dev', startArgs: []}, + })) + const payload = { + response: { + status: 200, + errors: [ + { + message: 'Access denied for themes field. Required access: read_themes access scope.', + extensions: {code: 'ACCESS_DENIED'}, + }, + ], + }, + request: {query: 'query Theme { themes { nodes { id } } }'}, + } + + await sendErrorToBugsnag(new Error(`GraphQL Error (Code: 200): ${JSON.stringify(payload)}`), 'unexpected_error') + + expect(lastBugsnagEvent).toBeDefined() + expect(lastBugsnagEvent!.groupingHash).toMatch(/^theme:permission:/) + expect(lastBugsnagEvent!.groupingHash).not.toContain(':network:') + }) + + test('groups GraphQL authentication failures separately from permission failures', async () => { + await metadata.addSensitiveMetadata(() => ({ + commandStartOptions: {startTime: Date.now(), startCommand: 'store info', startArgs: []}, + })) + const payload = { + response: { + status: 401, + errors: [{message: 'Service is not valid for authentication'}], + }, + request: {query: 'query Shop { shop { id } }'}, + } + + await sendErrorToBugsnag(new Error(`GraphQL Error (Code: 401): ${JSON.stringify(payload)}`), 'unexpected_error') + + expect(lastBugsnagEvent).toBeDefined() + expect(lastBugsnagEvent!.groupingHash).toMatch(/^store:authentication:/) + }) + + test('groups GraphQL throttling failures as rate limits', async () => { + await metadata.addSensitiveMetadata(() => ({ + commandStartOptions: {startTime: Date.now(), startCommand: 'theme pull', startArgs: []}, + })) + const payload = { + response: { + status: 200, + errors: [{message: 'Too many requests', extensions: {code: 'THROTTLED'}}], + }, + request: {query: 'query ThemeFiles { theme { files { nodes { filename } } } }'}, + } + + await sendErrorToBugsnag(new Error(`GraphQL Error (Code: 200): ${JSON.stringify(payload)}`), 'unexpected_error') + + expect(lastBugsnagEvent).toBeDefined() + expect(lastBugsnagEvent!.groupingHash).toMatch(/^theme:rate_limit:/) + }) + + test('sets groupingHash with cli slice when startCommand is missing', async () => { + await metadata.addSensitiveMetadata(() => ({ + commandStartOptions: {startTime: Date.now(), startCommand: undefined as unknown as string, startArgs: []}, + })) + + await sendErrorToBugsnag(new Error('boom'), 'unexpected_error') + + expect(lastBugsnagEvent).toBeDefined() + expect(lastBugsnagEvent!.groupingHash).toBe('cli:unknown:boom') + }) }) diff --git a/packages/cli-kit/src/public/node/error-handler.ts b/packages/cli-kit/src/public/node/error-handler.ts index d639c9d3d33..a5827c95c90 100644 --- a/packages/cli-kit/src/public/node/error-handler.ts +++ b/packages/cli-kit/src/public/node/error-handler.ts @@ -12,6 +12,7 @@ import { } from './error.js' import {outputDebug, outputInfo} from './output.js' import {getEnvironmentData} from '../../private/node/analytics.js' +import {categorizeError, formatErrorMessage} from '../../private/node/analytics/error-categorizer.js' import {isLocalEnvironment} from '../../private/node/context/service.js' import {bugsnagApiKey, reportingRateLimit} from '../../private/node/constants.js' import {CLI_KIT_VERSION} from '../common/version.js' @@ -28,6 +29,72 @@ import {realpath} from 'fs/promises' // Hardcoded list per product slices to keep analytics consistent. const ALLOWED_SLICE_NAMES = new Set(['app', 'theme', 'hydrogen', 'store']) +function sliceNameFromStartCommand(startCommand: string | undefined): string { + if (!startCommand) return 'cli' + + const firstWord = startCommand.trim().split(/\s+/)[0] ?? 'cli' + return ALLOWED_SLICE_NAMES.has(firstWord) ? firstWord : 'cli' +} + +interface GraphQLErrorPayload { + response?: { + status?: number + errors?: { + message?: string + extensions?: { + code?: string + } + }[] + } +} + +function errorForGrouping(error: Error): Error { + const graphQLError = graphQLErrorForGrouping(error.message) + if (graphQLError) return graphQLError + + const messageWithoutJsonDump = error.message.split(': {')[0] ?? error.message + return new Error(messageWithoutJsonDump) +} + +function graphQLErrorForGrouping(message: string): Error | undefined { + const jsonStart = message.indexOf('{') + if (jsonStart === -1) return undefined + + let payload: GraphQLErrorPayload + try { + payload = JSON.parse(message.slice(jsonStart)) as GraphQLErrorPayload + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + return undefined + } + + const {response} = payload + if (!response) return undefined + + const firstError = response.errors?.[0] + const code = firstError?.extensions?.code + const firstErrorMessage = firstError?.message?.replace(/too many requests/gi, 'rate limit exceeded') + + const parts: string[] = [] + if (response.status === 401 && !firstErrorMessage?.match(/auth|credential|token|unauthorized/i)) { + parts.push('Unauthorized') + } + if (response.status === 403 && !firstErrorMessage?.match(/access|denied|forbidden|permission|unauthorized/i)) { + parts.push('Forbidden') + } + if (response.status) parts.push(`HTTP ${response.status}`) + if (code) parts.push(code) + if (firstErrorMessage) parts.push(firstErrorMessage) + + return new Error(parts.length > 0 ? parts.join(' ') : (message.split(': {')[0] ?? message)) +} + +function bugsnagGroupingHash(error: Error, sliceName: string): string { + const groupingError = errorForGrouping(error) + const category = categorizeError(groupingError) + return `${sliceName}:${category.toLowerCase()}:${formatErrorMessage(groupingError, category)}` +} + export async function errorHandler( error: Error & {exitCode?: number | undefined}, config?: Interfaces.Config, @@ -76,11 +143,6 @@ export async function sendErrorToBugsnag( return {reported: false, error, unhandled: undefined} } - if (exitMode === 'expected_error') { - outputDebug(`Skipping Bugsnag report for expected error`) - return {reported: false, error, unhandled: undefined} - } - // If the error was unexpected, we flag it as "unhandled" in Bugsnag. This is a helpful distinction. const unhandled = exitMode === 'unexpected_error' @@ -147,9 +209,9 @@ export async function sendErrorToBugsnag( // Attach command metadata so we know which CLI command triggered the error const {commandStartOptions} = metadata.getAllSensitiveMetadata() const {startCommand} = commandStartOptions ?? {} + const sliceName = sliceNameFromStartCommand(startCommand) + event.groupingHash = bugsnagGroupingHash(reportableError, sliceName) if (startCommand) { - const firstWord = startCommand.trim().split(/\s+/)[0] ?? 'cli' - const sliceName = ALLOWED_SLICE_NAMES.has(firstWord) ? firstWord : 'cli' event.addMetadata('custom', {slice_name: sliceName}) } } @@ -192,7 +254,7 @@ export function cleanStackFrameFilePath({ ? currentFilePath : path.joinPath(projectRoot, currentFilePath) - const matchingPluginPath = pluginLocations.find(({pluginPath}) => fullLocation.startsWith(pluginPath)) + const matchingPluginPath = pluginLocations.filter(({pluginPath}) => fullLocation.startsWith(pluginPath))[0] if (matchingPluginPath !== undefined) { // the plugin name (e.g. @shopify/cli-kit), plus the relative path of the error line from within the plugin's code (e.g. dist/something.js )