Add tests for structured logging: BUILD_SHA, error serialization, toError() (#60538)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: heiskr

src/observability/lib/handle-exceptions.ts

@@ -1,19 +1,9 @@
 import FailBot from './failbot'
+import { toError } from '@/observability/lib/to-error'
 import { createLogger } from '@/observability/logger'
 
 const logger = createLogger(import.meta.url)
 
-// Safely convert an unknown thrown value to an Error, avoiding JSON.stringify
-// which can throw on circular references.
-function toError(value: Error | unknown): Error {
-  if (value instanceof Error) return value
-  try {
-    return new Error(JSON.stringify(value))
-  } catch {
-    return new Error(String(value))
-  }
-}
-
 process.on('uncaughtException', async (err: Error | unknown) => {
   const error = toError(err)
   logger.error('uncaughtException', { error })

src/observability/lib/to-error.ts

@@ -0,0 +1,10 @@
+// Safely convert an unknown thrown value to an Error, avoiding JSON.stringify
+// which can throw on circular references.
+export function toError(value: Error | unknown): Error {
+  if (value instanceof Error) return value
+  try {
+    return new Error(JSON.stringify(value))
+  } catch {
+    return new Error(String(value))
+  }
+}

src/observability/tests/logger.ts

@@ -431,4 +431,97 @@ describe('createLogger', () => {
       expect(logOutput).not.toContain('nodeHostname=')
     })
   })
+
+  describe('BUILD_SHA in production logs', () => {
+    it('should include build_sha in logfmt output when BUILD_SHA env var is set', async () => {
+      vi.stubEnv('BUILD_SHA', 'abc123def456')
+      vi.stubEnv('LOG_LIKE_PRODUCTION', 'true')
+
+      vi.resetModules()
+      const { createLogger: freshCreateLogger } = await import('@/observability/logger')
+
+      const logger = freshCreateLogger('file:///path/to/test.js')
+      logger.info('Build SHA test')
+
+      expect(consoleLogs).toHaveLength(1)
+      const logOutput = consoleLogs[0]
+      expect(logOutput).toContain('build_sha=abc123def456')
+    })
+
+    it('should not include build_sha in logfmt output when BUILD_SHA env var is absent', async () => {
+      vi.stubEnv('LOG_LIKE_PRODUCTION', 'true')
+      delete process.env.BUILD_SHA
+
+      vi.resetModules()
+      const { createLogger: freshCreateLogger } = await import('@/observability/logger')
+
+      const logger = freshCreateLogger('file:///path/to/test.js')
+      logger.info('No build SHA test')
+
+      expect(consoleLogs).toHaveLength(1)
+      const logOutput = consoleLogs[0]
+      expect(logOutput).not.toContain('build_sha=')
+    })
+  })
+
+  describe('error serialization in production logs', () => {
+    it('should include error_code and error_name when Error has a .code property', async () => {
+      vi.stubEnv('LOG_LIKE_PRODUCTION', 'true')
+
+      vi.resetModules()
+      const { createLogger: freshCreateLogger } = await import('@/observability/logger')
+
+      const logger = freshCreateLogger('file:///path/to/test.js')
+      const error = new Error('Connection reset') as NodeJS.ErrnoException
+      error.code = 'ECONNRESET'
+      logger.error('Network failure', error)
+
+      expect(consoleLogs).toHaveLength(1)
+      const logOutput = consoleLogs[0]
+      expect(logOutput).toContain('included.error="Connection reset"')
+      expect(logOutput).toContain('included.error_code=ECONNRESET')
+      expect(logOutput).toContain('included.error_name=Error')
+      expect(logOutput).toContain('included.error_stack=')
+    })
+
+    it('should include error_name even when .code is undefined', async () => {
+      vi.stubEnv('LOG_LIKE_PRODUCTION', 'true')
+
+      vi.resetModules()
+      const { createLogger: freshCreateLogger } = await import('@/observability/logger')
+
+      const logger = freshCreateLogger('file:///path/to/test.js')
+      const error = new TypeError('Cannot read property')
+      logger.error('Type error occurred', error)
+
+      expect(consoleLogs).toHaveLength(1)
+      const logOutput = consoleLogs[0]
+      expect(logOutput).toContain('included.error="Cannot read property"')
+      expect(logOutput).toContain('included.error_name=TypeError')
+      // When .code is undefined, error_code is present but empty
+      expect(logOutput).toMatch(/included\.error_code= /)
+      expect(logOutput).toContain('included.error_stack=')
+    })
+
+    it('should serialize multiple errors with indexed keys', async () => {
+      vi.stubEnv('LOG_LIKE_PRODUCTION', 'true')
+
+      vi.resetModules()
+      const { createLogger: freshCreateLogger } = await import('@/observability/logger')
+
+      const logger = freshCreateLogger('file:///path/to/test.js')
+      const error1 = new Error('First') as NodeJS.ErrnoException
+      error1.code = 'ERR_FIRST'
+      const error2 = new Error('Second')
+      logger.error('Multiple errors', error1, error2)
+
+      expect(consoleLogs).toHaveLength(1)
+      const logOutput = consoleLogs[0]
+      expect(logOutput).toContain('included.error_1=First')
+      expect(logOutput).toContain('included.error_1_code=ERR_FIRST')
+      expect(logOutput).toContain('included.error_1_name=Error')
+      expect(logOutput).toContain('included.error_2=Second')
+      expect(logOutput).toContain('included.error_2_name=Error')
+    })
+  })
 })

src/observability/tests/to-error.ts

@@ -0,0 +1,60 @@
+import { describe, expect, it } from 'vitest'
+
+import { toError } from '@/observability/lib/to-error'
+
+describe('toError', () => {
+  it('should return Error instances as-is', () => {
+    const error = new Error('test error')
+    const result = toError(error)
+    expect(result).toBe(error)
+    expect(result.message).toBe('test error')
+  })
+
+  it('should return subclassed Error instances as-is', () => {
+    const error = new TypeError('type error')
+    const result = toError(error)
+    expect(result).toBe(error)
+    expect(result).toBeInstanceOf(TypeError)
+  })
+
+  it('should convert a plain string to an Error via JSON.stringify', () => {
+    const result = toError('something went wrong')
+    expect(result).toBeInstanceOf(Error)
+    expect(result.message).toBe('"something went wrong"')
+  })
+
+  it('should convert a number to an Error', () => {
+    const result = toError(42)
+    expect(result).toBeInstanceOf(Error)
+    expect(result.message).toBe('42')
+  })
+
+  it('should convert null to an Error', () => {
+    const result = toError(null)
+    expect(result).toBeInstanceOf(Error)
+    expect(result.message).toBe('null')
+  })
+
+  it('should convert undefined to an Error via JSON.stringify', () => {
+    const result = toError(undefined)
+    expect(result).toBeInstanceOf(Error)
+    // JSON.stringify(undefined) returns undefined (not a string),
+    // so new Error(undefined) has an empty message
+    expect(result.message).toBe('')
+  })
+
+  it('should convert a plain object to an Error via JSON.stringify', () => {
+    const result = toError({ code: 'ERR_TIMEOUT', detail: 'took too long' })
+    expect(result).toBeInstanceOf(Error)
+    expect(result.message).toBe('{"code":"ERR_TIMEOUT","detail":"took too long"}')
+  })
+
+  it('should fall back to String() for circular references', () => {
+    const circular: Record<string, unknown> = { name: 'loop' }
+    circular.self = circular
+    const result = toError(circular)
+    expect(result).toBeInstanceOf(Error)
+    // String() on an object returns '[object Object]'
+    expect(result.message).toBe('[object Object]')
+  })
+})