fix(metrics): add validation/formatting before sending errors to BugS…

…nag (#5455)

* fix(metrics): add validation/formatting before sending errors to BugSnag

* chore: add tests

packages/build-info/src/metrics.test.ts

@@ -0,0 +1,60 @@
+import { Client } from '@bugsnag/js'
+import { describe, expect, test, vi } from 'vitest'
+
+import { report } from './metrics.js'
+
+describe('metrics', () => {
+  describe('normalizeError', () => {
+    const mockClient = { notify: (error) => console.error(error) } as Client
+
+    test('returns an error when passed a string', async () => {
+      const errorSpy = vi.spyOn(console, 'error')
+      report('error happened', { client: mockClient })
+      expect(errorSpy).toHaveBeenCalledOnce
+      expect(errorSpy.mock.calls[0][0]).toBeInstanceOf(Error)
+      expect(errorSpy.mock.calls[0][0].message).toBe('error happened')
+    })
+
+    test('returns an error when passed an error', async () => {
+      const errorSpy = vi.spyOn(console, 'error')
+      report(new Error('error happened'), { client: mockClient })
+      expect(errorSpy).toHaveBeenCalledOnce
+      expect(errorSpy.mock.calls[0][0]).toBeInstanceOf(Error)
+      expect(errorSpy.mock.calls[0][0].message).toBe('error happened')
+    })
+
+    test('returns an object when passed an object in an expected format (1)', async () => {
+      const errorSpy = vi.spyOn(console, 'error')
+      report({ name: 'Error', message: 'error happened' }, { client: mockClient })
+      expect(errorSpy).toHaveBeenCalledOnce
+      expect(errorSpy.mock.calls[0][0]).not.toBeInstanceOf(Error)
+      expect(errorSpy.mock.calls[0][0].name).toBe('Error')
+      expect(errorSpy.mock.calls[0][0].message).toBe('error happened')
+    })
+
+    test('returns an object when passed an object in an expected format (2)', async () => {
+      const errorSpy = vi.spyOn(console, 'error')
+      report({ errorClass: 'Error', errorMessage: 'error happened' }, { client: mockClient })
+      expect(errorSpy).toHaveBeenCalledOnce
+      expect(errorSpy.mock.calls[0][0]).not.toBeInstanceOf(Error)
+      expect(errorSpy.mock.calls[0][0].errorClass).toBe('Error')
+      expect(errorSpy.mock.calls[0][0].errorMessage).toBe('error happened')
+    })
+
+    test('returns an error when passed an object in an unexpected format but includes a message', async () => {
+      const errorSpy = vi.spyOn(console, 'error')
+      report({ message: 'error happened', documentation_url: 'bar' }, { client: mockClient })
+      expect(errorSpy).toHaveBeenCalledOnce
+      expect(errorSpy.mock.calls[0][0]).toBeInstanceOf(Error)
+      expect(errorSpy.mock.calls[0][0].message).toBe('error happened')
+    })
+
+    test('returns an error when passed an object in an unexpected format', async () => {
+      const errorSpy = vi.spyOn(console, 'error')
+      report({ foo: 'bar' }, { client: mockClient })
+      expect(errorSpy).toHaveBeenCalledOnce
+      expect(errorSpy.mock.calls[0][0]).toBeInstanceOf(Error)
+      expect(errorSpy.mock.calls[0][0].message).toBe('Unexpected error format: {"foo":"bar"}')
+    })
+  })
+})

packages/build-info/src/metrics.ts

@@ -4,20 +4,51 @@ const { default: Bugsnag } = bugsnag
 
 export type Severity = 'info' | 'warning' | 'error'
 
+/** Normalize an error to the NotifiableError type */
+const normalizeError = (error: any): NotifiableError => {
+  // Already in an acceptable NotifiableError format
+  if (error instanceof Error) {
+    return error
+  }
+  if (typeof error === 'object' && (error.errorClass || error.name)) {
+    return error
+  }
+  if (typeof error === 'string') {
+    // BugSnag suggest sending an Error rather than string to get better stack traces
+    return new Error(error)
+  }
+
+  // If the error is an object with a message, create a generic Error
+  if (typeof error === 'object' && error.message) {
+    return new Error(error.message)
+  }
+
+  // If the error format is unexpected, create a generic Error
+  return new Error(`Unexpected error format: ${JSON.stringify(error)}`)
+}
+
 /** Report an error to bugsnag */
 export function report(
-  error: NotifiableError,
+  error: NotifiableError | Record<string, any>,
   options: {
     context?: string
     severity?: Severity
     metadata?: Record<string, Record<string, any>>
     client?: Client
   } = {},
 ) {
-  ;(options.client || Bugsnag).notify(error, (event) => {
+  const normalizedError = normalizeError(error)
+  const client = options.client || Bugsnag
+
+  client.notify(normalizedError, (event) => {
     for (const [section, values] of Object.entries(options.metadata || {})) {
       event.addMetadata(section, values)
     }
+    // If the error is an object with a documentation_url property, it's probably a GitHub API error
+    if (typeof error === 'object' && 'documentation_url' in error) {
+      event.addMetadata('Documentation URL', error.documentation_url)
+    }
+    event.context = options.context
     event.severity = options.severity || 'error'
     event.context = options.context
   })