Skip to content

Commit 3a2d86b

Browse files
Add slice_name to error handler
1 parent 59bbf90 commit 3a2d86b

File tree

2 files changed

+135
-1
lines changed

2 files changed

+135
-1
lines changed

packages/cli-kit/src/public/node/error-handler.test.ts

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
1-
import {errorHandler, cleanStackFrameFilePath, addBugsnagMetadata, sendErrorToBugsnag} from './error-handler.js'
1+
import {
2+
errorHandler,
3+
cleanStackFrameFilePath,
4+
addBugsnagMetadata,
5+
sendErrorToBugsnag,
6+
computeAllowedSliceNames,
7+
registerCleanBugsnagErrorsFromWithinPlugins,
8+
} from './error-handler.js'
9+
import * as metadata from './metadata.js'
210
import {ciPlatform, cloudEnvironment, isUnitTest, macAddress} from './context/local.js'
311
import {mockAndCaptureOutput} from './testing/output.js'
412
import * as error from './error.js'
@@ -8,16 +16,23 @@ import {settings} from '@oclif/core'
816
import {beforeEach, describe, expect, test, vi} from 'vitest'
917

1018
const onNotify = vi.fn()
19+
let lastBugsnagEvent: {addMetadata: ReturnType<typeof vi.fn>} | undefined
1120

1221
vi.mock('process')
1322
vi.mock('@bugsnag/js', () => {
1423
return {
1524
default: {
1625
notify: (reportedError: any, args: any, callback: any) => {
1726
onNotify(reportedError)
27+
if (typeof args === 'function') {
28+
const event = {addMetadata: vi.fn()}
29+
lastBugsnagEvent = event as any
30+
args(event)
31+
}
1832
callback(null)
1933
},
2034
isStarted: () => true,
35+
addOnError: vi.fn(),
2136
},
2237
}
2338
})
@@ -39,6 +54,7 @@ beforeEach(() => {
3954
vi.mocked(hashString).mockReturnValue('hashed-macaddress')
4055
vi.mocked(isUnitTest).mockReturnValue(true)
4156
onNotify.mockClear()
57+
lastBugsnagEvent = undefined
4258
vi.mocked(settings).debug = false
4359
vi.mocked(isLocalEnvironment).mockReturnValue(false)
4460
})
@@ -207,4 +223,86 @@ describe('sends errors to Bugsnag', () => {
207223
expect(res.error).toEqual(toThrow)
208224
expect(mockOutput.debug()).toMatch('Error reporting to Bugsnag: Error: Bugsnag is down')
209225
})
226+
227+
test('logs and suppresses when allowed slice names not initialized', async () => {
228+
// Reset module state to ensure ALLOWED_SLICE_NAMES is undefined
229+
vi.resetModules()
230+
231+
await metadata.addSensitiveMetadata(() => ({
232+
commandStartOptions: {startTime: Date.now(), startCommand: 'app dev', startArgs: []},
233+
}))
234+
235+
const mockOutput = mockAndCaptureOutput()
236+
237+
const res = await sendErrorToBugsnag(new Error('boom'), 'unexpected_error')
238+
expect(res.reported).toEqual(false)
239+
expect(mockOutput.debug()).toMatch(
240+
'Error reporting to Bugsnag: Error: Internal error: allowed slice names not initialized.',
241+
)
242+
})
243+
244+
test('attaches custom metadata with allowed slice_name when startCommand is present', async () => {
245+
// Initialize allowed slice names to include 'app' using a mock config
246+
await registerCleanBugsnagErrorsFromWithinPlugins({
247+
// Minimal shape required by the function
248+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
249+
commands: [{id: 'app:dev'}] as any,
250+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
251+
plugins: new Map() as any,
252+
} as unknown as any)
253+
254+
await metadata.addSensitiveMetadata(() => ({
255+
commandStartOptions: {startTime: Date.now(), startCommand: 'app dev', startArgs: []},
256+
}))
257+
258+
await sendErrorToBugsnag(new Error('boom'), 'unexpected_error')
259+
260+
expect(lastBugsnagEvent).toBeDefined()
261+
expect(lastBugsnagEvent!.addMetadata).toHaveBeenCalledWith('custom', {slice_name: 'app'})
262+
})
263+
264+
test('does not attach custom slice_name when startCommand is missing', async () => {
265+
await metadata.addSensitiveMetadata(() => ({
266+
commandStartOptions: {startTime: Date.now(), startCommand: undefined as unknown as string, startArgs: []},
267+
}))
268+
269+
await sendErrorToBugsnag(new Error('boom'), 'unexpected_error')
270+
271+
expect(lastBugsnagEvent).toBeDefined()
272+
const calls = (lastBugsnagEvent!.addMetadata as any).mock.calls as any[]
273+
const customCall = calls.find(([section]: [string]) => section === 'custom')
274+
expect(customCall).toBeUndefined()
275+
})
276+
277+
test('defaults slice_name to cli when first word not allowed', async () => {
278+
// Initialize allowed slice names to known set that does not include 'version'
279+
await registerCleanBugsnagErrorsFromWithinPlugins({
280+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
281+
commands: [{id: 'app:dev'}] as any,
282+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
283+
plugins: new Map() as any,
284+
} as unknown as any)
285+
286+
await metadata.addSensitiveMetadata(() => ({
287+
commandStartOptions: {startTime: Date.now(), startCommand: 'version', startArgs: []},
288+
}))
289+
290+
await sendErrorToBugsnag(new Error('boom'), 'unexpected_error')
291+
292+
expect(lastBugsnagEvent).toBeDefined()
293+
expect(lastBugsnagEvent!.addMetadata).toHaveBeenCalledWith('custom', {slice_name: 'cli'})
294+
})
295+
})
296+
297+
describe('computeAllowedSliceNames', () => {
298+
test('derives first tokens from command IDs', () => {
299+
const names = computeAllowedSliceNames({
300+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
301+
commands: [{id: 'app:build'}, {id: 'theme:pull'}, {id: 'version'}, {id: undefined as any}] as any,
302+
} as unknown as any)
303+
expect(names.has('app')).toBe(true)
304+
expect(names.has('theme')).toBe(true)
305+
expect(names.has('version')).toBe(true)
306+
expect(names.has('store')).toBe(false)
307+
})
210308
})

packages/cli-kit/src/public/node/error-handler.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,27 @@ import StackTracey from 'stacktracey'
2121
import Bugsnag, {Event} from '@bugsnag/js'
2222
import {realpath} from 'fs/promises'
2323

24+
// Allowed slice names for error analytics grouping. This is populated from the
25+
// loaded OCLIF config at runtime.
26+
let ALLOWED_SLICE_NAMES: Set<string> | undefined
27+
28+
export function computeAllowedSliceNames(config: Interfaces.Config): Set<string> {
29+
const names = new Set<string>()
30+
const commands = (config.commands ?? []) as Array<{id?: string}>
31+
for (const cmd of commands) {
32+
// Command IDs are typically colon-separated (e.g. app:build). We take the first token.
33+
const id = (cmd as unknown as {id?: string}).id
34+
if (!id) continue
35+
const first = id.split(':')[0]?.trim()
36+
if (first) names.add(first)
37+
}
38+
return names
39+
}
40+
41+
export function initializeAllowedSliceNames(config: Interfaces.Config): void {
42+
ALLOWED_SLICE_NAMES = computeAllowedSliceNames(config)
43+
}
44+
2445
export async function errorHandler(
2546
error: Error & {exitCode?: number | undefined},
2647
config?: Interfaces.Config,
@@ -126,6 +147,19 @@ export async function sendErrorToBugsnag(
126147
const eventHandler = (event: Event) => {
127148
event.severity = 'error'
128149
event.unhandled = unhandled
150+
// Attach command metadata so we know which CLI command triggered the error
151+
const {commandStartOptions} = metadata.getAllSensitiveMetadata()
152+
const {startCommand} = commandStartOptions ?? {}
153+
if (startCommand) {
154+
if (!ALLOWED_SLICE_NAMES) {
155+
throw new Error(
156+
'Internal error: allowed slice names not initialized. Ensure registerCleanBugsnagErrorsFromWithinPlugins() ran.',
157+
)
158+
}
159+
const firstWord = startCommand.trim().split(/\s+/)[0] ?? 'cli'
160+
const sliceName = ALLOWED_SLICE_NAMES.has(firstWord) ? firstWord : 'cli'
161+
event.addMetadata('custom', {slice_name: sliceName})
162+
}
129163
}
130164
const errorHandler = (error: unknown) => {
131165
if (error) {
@@ -182,6 +216,8 @@ export function cleanStackFrameFilePath({
182216
*
183217
*/
184218
export async function registerCleanBugsnagErrorsFromWithinPlugins(config: Interfaces.Config): Promise<void> {
219+
// Update allowed slice names dynamically based on loaded commands
220+
initializeAllowedSliceNames(config)
185221
// Bugsnag have their own plug-ins that use this private field
186222

187223
// eslint-disable-next-line @typescript-eslint/no-explicit-any

0 commit comments

Comments
 (0)