From 84f98b969438113d9b4c5bf3faea22ace0a1bdc1 Mon Sep 17 00:00:00 2001 From: Terry Moore Date: Fri, 12 Jun 2026 09:43:47 -0400 Subject: [PATCH] fix(logger): Updates to uncaught error handlers Better support for logging uncaught errors and its details Semver: patch Summary: Test plan: Ref: Semver: --- src/capture.ts | 71 +++++++++++++++++++++++++--- src/logdna.d.ts | 12 +++++ src/plugins/global-handler.ts | 18 ++++--- tests/capture.spec.ts | 66 ++++++++++++++++++++++++++ tests/plugins/global-handler.spec.ts | 44 +++++++++++++++++ 5 files changed, 198 insertions(+), 13 deletions(-) diff --git a/src/capture.ts b/src/capture.ts index d16426b..a263a4d 100644 --- a/src/capture.ts +++ b/src/capture.ts @@ -4,7 +4,7 @@ import { process } from './buffer-manager'; import utils from './utils'; import { getStaticContext, getContext, getDynamicContext } from './context-manager'; import { getSessionId } from './session-manager'; -import { LogMessage } from './logdna'; +import { LogMessage, ErrorEventContext } from './logdna'; const captureMessage = async ({ level = 'log', message, lineContext = {} }: LogMessage) => { if (isSendingDisabled()) return; @@ -17,10 +17,62 @@ const captureMessage = async ({ level = 'log', message, lineContext = {} }: LogM await generateLogLine({ level, message, lineContext }); }; -const captureError = async (error: any, isUnhandledRejection = false) => { +// Classify whatever was thrown/rejected (could be an Error, string, number, +// plain object, null, etc.) into a consistent shape so we never drop the value. +const normalizeReason = (value: any) => { + if (value instanceof Error) { + return { + type: value.name, + message: value.message, + isError: true, + reason: undefined, + }; + } + + if (typeof value === 'string') { + return { type: 'string', message: value, isError: false, reason: value }; + } + + if (value === null || value === undefined || typeof value !== 'object') { + return { type: typeof value, message: String(value), isError: false, reason: value }; + } + + // Plain object (or Error-like object) — keep both a human message and the raw value. + const stringified = utils.stringify(value); + return { + type: value.name || value.constructor?.name || 'object', + message: value.message ?? stringified, + isError: false, + reason: stringified, + }; +}; + +// Capture error.cause (ES2022) one level deep without risking deep/circular recursion. +const normalizeCause = (cause: any) => { + if (cause === null || cause === undefined) return undefined; + if (cause instanceof Error) { + return { + type: cause.name, + message: cause.message, + rawStack: typeof cause.stack === 'string' ? cause.stack : undefined, + }; + } + const normalized = normalizeReason(cause); + return { type: normalized.type, message: normalized.message }; +}; + +const captureError = async (error: any, isUnhandledRejection = false, eventContext: ErrorEventContext = {}) => { if (isSendingDisabled()) return; - let message = error.name ? `${error.name}: ${error.message}` : error.message; + const normalized = normalizeReason(error); + + // Prefix with the type only when one is meaningfully present (matches prior behavior + // where a bare `{ message }` produced just the message, while `TypeError` was prefixed). + let message = normalized.isError || (error && error.name) ? `${normalized.type}: ${normalized.message}` : normalized.message; + // Fall back to the event message (uncaught errors) so we never send an empty line. + if (message === undefined || message === null || message === '') { + message = eventContext.message || normalized.message; + } if (isUnhandledRejection) { message = `Uncaught (in promise) ${message}`; @@ -30,12 +82,17 @@ const captureError = async (error: any, isUnhandledRejection = false) => { level: 'error', message, errorContext: { - colno: error.columnNumber || error.colno || error.colNo, - lineno: error.lineNumber || error.lineno || error.lineNo, + colno: error?.columnNumber || error?.colno || error?.colNo || eventContext.colno, + lineno: error?.lineNumber || error?.lineno || error?.lineNo || eventContext.lineno, stacktrace: await utils.getStackTraceFromError(error), - source: error.fileName || error.source, + source: error?.fileName || error?.source || eventContext.filename, + type: normalized.type, + rawStack: typeof error?.stack === 'string' ? error.stack : undefined, + cause: normalizeCause(error?.cause), + reason: isUnhandledRejection ? normalized.reason : undefined, + isUnhandledRejection: isUnhandledRejection || undefined, }, - disableStacktrace: !!(error.stack || error.stacktrace), // Don't generate a second stacktrace for errors since they already have it + disableStacktrace: !!(error?.stack || error?.stacktrace), // Don't generate a second stacktrace for errors since they already have it }); }; diff --git a/src/logdna.d.ts b/src/logdna.d.ts index 76f2894..9e179ff 100644 --- a/src/logdna.d.ts +++ b/src/logdna.d.ts @@ -65,6 +65,18 @@ export type ErrorContext = { lineno?: number; stacktrace?: string; source?: string; + type?: string; + rawStack?: string; + cause?: any; + reason?: any; + isUnhandledRejection?: boolean; +}; + +export type ErrorEventContext = { + message?: string; + filename?: string; + lineno?: number; + colno?: number; }; export type LogMessage = { diff --git a/src/plugins/global-handler.ts b/src/plugins/global-handler.ts index a84180f..208aaac 100644 --- a/src/plugins/global-handler.ts +++ b/src/plugins/global-handler.ts @@ -24,15 +24,21 @@ const addUnhandledrejection = () => { }; /* istanbul ignore next */ -const onUnhandledRejection = (e: any) => { - let error: any = e.reason; - captureError(error, true); +const onUnhandledRejection = (event: PromiseRejectionEvent) => { + captureError(event?.reason, true); }; /* istanbul ignore next */ -const onError = (error: ErrorEvent) => { - const e = error?.error ?? error ?? {}; - captureError(e); +const onError = (event: ErrorEvent) => { + // Prefer the underlying Error (carries name/message/stack), but keep the event's + // own position fields — Chromium Errors don't expose filename/lineno/colno. + const error = event?.error ?? event ?? {}; + captureError(error, false, { + message: event?.message, + filename: event?.filename, + lineno: event?.lineno, + colno: event?.colno, + }); }; const GlobalErrorHandler = (opts: GlobalErrorHandlerPlugin = DEFAULT_OPTIONS): Plugin => ({ diff --git a/tests/capture.spec.ts b/tests/capture.spec.ts index 9092611..3e0d407 100644 --- a/tests/capture.spec.ts +++ b/tests/capture.spec.ts @@ -123,6 +123,72 @@ describe('capture.ts', () => { meta: expect.any(Object), }); }); + + it('should keep filename/lineno/colno from the error event when the Error lacks them (Chromium)', async () => { + jest.spyOn(init, 'isSendingDisabled').mockImplementationOnce(() => false); + + // Chromium Errors do not carry fileName/lineNumber/columnNumber. + const error = new Error('Boom'); + await captureError(error, false, { + message: 'Boom', + filename: 'https://app.example.com/main.js', + lineno: 42, + colno: 13, + }); + + const { meta } = process.mock.calls[0][0] as any; + expect(meta.errorContext.source).toEqual('https://app.example.com/main.js'); + expect(meta.errorContext.lineno).toEqual(42); + expect(meta.errorContext.colno).toEqual(13); + expect(meta.errorContext.type).toEqual('Error'); + }); + + it('should capture a string promise rejection reason instead of undefined', async () => { + jest.spyOn(init, 'isSendingDisabled').mockImplementationOnce(() => false); + + await captureError('Something failed', true); + + const { line, meta } = process.mock.calls[0][0] as any; + expect(line).toEqual('Uncaught (in promise) Something failed'); + expect(meta.errorContext.reason).toEqual('Something failed'); + expect(meta.errorContext.type).toEqual('string'); + expect(meta.errorContext.isUnhandledRejection).toEqual(true); + }); + + it('should capture a non-Error object promise rejection reason', async () => { + jest.spyOn(init, 'isSendingDisabled').mockImplementationOnce(() => false); + + await captureError({ code: 500, detail: 'Internal' }, true); + + const { line, meta } = process.mock.calls[0][0] as any; + expect(line).toContain('Uncaught (in promise)'); + expect(line).toContain('500'); + expect(meta.errorContext.reason).toEqual(JSON.stringify({ code: 500, detail: 'Internal' })); + }); + + it('should capture error.cause when present', async () => { + jest.spyOn(init, 'isSendingDisabled').mockImplementationOnce(() => false); + + const cause = new TypeError('Root cause'); + const error = new Error('Wrapper'); + // @ts-ignore - cause may not be in lib target + error.cause = cause; + await captureError(error); + + const { meta } = process.mock.calls[0][0] as any; + expect(meta.errorContext.cause).toEqual(expect.objectContaining({ type: 'TypeError', message: 'Root cause' })); + }); + + it('should include the raw stack as a fallback', async () => { + jest.spyOn(init, 'isSendingDisabled').mockImplementationOnce(() => false); + + const error = new Error('With stack'); + await captureError(error); + + const { meta } = process.mock.calls[0][0] as any; + expect(typeof meta.errorContext.rawStack).toEqual('string'); + expect(meta.errorContext.rawStack).toContain('With stack'); + }); }); describe('internalErrorLogger', () => { diff --git a/tests/plugins/global-handler.spec.ts b/tests/plugins/global-handler.spec.ts index 0a2e4ee..d6c739d 100644 --- a/tests/plugins/global-handler.spec.ts +++ b/tests/plugins/global-handler.spec.ts @@ -25,4 +25,48 @@ describe('global-handler.ts', () => { globalErrorHandler.init({ enableErrorHandler: true }); expect(window.addEventListener).toHaveBeenCalledWith('error', expect.any(Function)); }); + + describe('handlers', () => { + const getHandler = (eventName: string) => { + const add = window.addEventListener as jest.Mock; + const call = add.mock.calls.find((c) => c[0] === eventName); + return call?.[1]; + }; + + it('should forward the error event position fields to captureError', () => { + captureError.mockImplementation(async () => {}); + window.addEventListener = jest.fn(); + // @ts-ignore + globalErrorHandler.init({ enableErrorHandler: true }); + + const error = new Error('Boom'); + const onError = getHandler('error'); + onError({ + error, + message: 'Boom', + filename: 'https://app.example.com/main.js', + lineno: 42, + colno: 13, + } as ErrorEvent); + + expect(captureError).toHaveBeenCalledWith(error, false, { + message: 'Boom', + filename: 'https://app.example.com/main.js', + lineno: 42, + colno: 13, + }); + }); + + it('should forward the raw rejection reason to captureError', () => { + captureError.mockImplementation(async () => {}); + window.addEventListener = jest.fn(); + // @ts-ignore + globalErrorHandler.init({ enableUnhandledPromiseRejection: true }); + + const onUnhandledRejection = getHandler('unhandledrejection'); + onUnhandledRejection({ reason: { code: 500 } } as PromiseRejectionEvent); + + expect(captureError).toHaveBeenCalledWith({ code: 500 }, true); + }); + }); });