From ffad724b9337e966573f9636854562933d793180 Mon Sep 17 00:00:00 2001 From: David Newell Date: Wed, 16 Oct 2024 12:42:15 +0100 Subject: [PATCH] feat: add stack to stacktraceless "exceptions" (#1472) --- cypress/e2e/error-tracking.cy.ts | 16 +++++ playground/cypress-full/index.html | 4 ++ playground/cypress/index.html | 4 ++ .../exception-autocapture/error-conversion.ts | 66 ++++++++++++------- .../exception-autocapture/stack-trace.ts | 9 --- .../exception-autocapture/type-checking.ts | 1 + src/posthog-core.ts | 15 +++-- src/types.ts | 1 + 8 files changed, 77 insertions(+), 39 deletions(-) diff --git a/cypress/e2e/error-tracking.cy.ts b/cypress/e2e/error-tracking.cy.ts index 94c685b11..4a84ed9ee 100644 --- a/cypress/e2e/error-tracking.cy.ts +++ b/cypress/e2e/error-tracking.cy.ts @@ -41,6 +41,22 @@ describe('Exception capture', () => { cy.wait('@exception-autocapture-script') }) + it('adds stacktrace to captured strings', () => { + cy.get('[data-cy-exception-string-button]').click() + + // ugh + cy.wait(1500) + + cy.phCaptures({ full: true }).then((captures) => { + expect(captures.map((c) => c.event)).to.deep.equal(['$pageview', '$autocapture', '$exception']) + expect(captures[2].event).to.be.eql('$exception') + expect(captures[2].properties.$exception_list[0].stacktrace.frames.length).to.be.eq(1) + expect(captures[2].properties.$exception_list[0].stacktrace.frames[0].function).to.be.eq( + 'HTMLButtonElement.onclick' + ) + }) + }) + it('autocaptures exceptions', () => { cy.get('[data-cy-button-throws-error]').click() diff --git a/playground/cypress-full/index.html b/playground/cypress-full/index.html index a6807cee8..a55faa495 100644 --- a/playground/cypress-full/index.html +++ b/playground/cypress-full/index.html @@ -40,6 +40,10 @@ Send an exception + +
diff --git a/src/extensions/exception-autocapture/error-conversion.ts b/src/extensions/exception-autocapture/error-conversion.ts index f4997690d..4c92a6d93 100644 --- a/src/extensions/exception-autocapture/error-conversion.ts +++ b/src/extensions/exception-autocapture/error-conversion.ts @@ -56,7 +56,7 @@ export interface ErrorConversions { const ERROR_TYPES_PATTERN = /^(?:[Uu]ncaught (?:exception: )?)?(?:((?:Eval|Internal|Range|Reference|Syntax|Type|URI|)Error): )?(.*)$/i -export function parseStackFrames(ex: Error & { framesToPop?: number; stacktrace?: string }): StackFrame[] { +export function parseStackFrames(ex: Error & { stacktrace?: string }, framesToPop: number = 0): StackFrame[] { // Access and store the stacktrace property before doing ANYTHING // else to it because Opera is not very good at providing it // reliably in other circumstances. @@ -65,7 +65,9 @@ export function parseStackFrames(ex: Error & { framesToPop?: number; stacktrace? const skipLines = getSkipFirstStackStringLines(ex) try { - return defaultStackParser(stacktrace, skipLines) + const frames = defaultStackParser(stacktrace, skipLines) + // frames are reversed so we remove the from the back of the array + return frames.slice(0, frames.length - framesToPop) } catch { // no-empty } @@ -146,17 +148,26 @@ function errorPropertiesFromString(candidate: string, metadata?: ErrorMetadata): ? candidate : metadata?.defaultExceptionMessage + const exception: Exception = { + type: exceptionType, + value: exceptionMessage, + mechanism: { + handled, + synthetic, + }, + } + + if (metadata?.syntheticException) { + // Kludge: strip the last frame from a synthetically created error + // so that it does not appear in a users stack trace + const frames = parseStackFrames(metadata.syntheticException, 1) + if (frames.length) { + exception.stacktrace = { frames } + } + } + return { - $exception_list: [ - { - type: exceptionType, - value: exceptionMessage, - mechanism: { - handled, - synthetic, - }, - }, - ], + $exception_list: [exception], $exception_level: 'error', } } @@ -206,17 +217,26 @@ function errorPropertiesFromObject(candidate: Record, metadata? ? metadata.overrideExceptionMessage : `Non-Error ${'exception'} captured with keys: ${extractExceptionKeysForMessage(candidate)}` + const exception: Exception = { + type: exceptionType, + value: exceptionMessage, + mechanism: { + handled, + synthetic, + }, + } + + if (metadata?.syntheticException) { + // Kludge: strip the last frame from a synthetically created error + // so that it does not appear in a users stack trace + const frames = parseStackFrames(metadata?.syntheticException, 1) + if (frames.length) { + exception.stacktrace = { frames } + } + } + return { - $exception_list: [ - { - type: exceptionType, - value: exceptionMessage, - mechanism: { - handled, - synthetic, - }, - }, - ], + $exception_list: [exception], $exception_level: isSeverityLevel(candidate.level) ? candidate.level : 'error', } } @@ -259,7 +279,7 @@ export function errorToProperties( } else if (isPlainObject(candidate) || isEvent(candidate)) { // group these by using the keys available on the object const objectException = candidate as Record - return errorPropertiesFromObject(objectException) + return errorPropertiesFromObject(objectException, metadata) } else if (isUndefined(error) && isString(event)) { let name = 'Error' let message = event diff --git a/src/extensions/exception-autocapture/stack-trace.ts b/src/extensions/exception-autocapture/stack-trace.ts index f09127713..a3e6d041b 100644 --- a/src/extensions/exception-autocapture/stack-trace.ts +++ b/src/extensions/exception-autocapture/stack-trace.ts @@ -51,7 +51,6 @@ export interface StackFrame { } const WEBPACK_ERROR_REGEXP = /\(error: (.*)\)/ -const STRIP_FRAME_REGEXP = /captureException/ const STACKTRACE_FRAME_LIMIT = 50 const UNKNOWN_FUNCTION = '?' @@ -210,14 +209,6 @@ export function reverseAndStripFrames(stack: ReadonlyArray): StackFr localStack.reverse() - if (STRIP_FRAME_REGEXP.test(getLastStackFrame(localStack).function || '')) { - localStack.pop() - - if (STRIP_FRAME_REGEXP.test(getLastStackFrame(localStack).function || '')) { - localStack.pop() - } - } - return localStack.slice(0, STACKTRACE_FRAME_LIMIT).map((frame) => ({ ...frame, filename: frame.filename || getLastStackFrame(localStack).filename, diff --git a/src/extensions/exception-autocapture/type-checking.ts b/src/extensions/exception-autocapture/type-checking.ts index 01016ee6f..4e561e806 100644 --- a/src/extensions/exception-autocapture/type-checking.ts +++ b/src/extensions/exception-autocapture/type-checking.ts @@ -27,6 +27,7 @@ export function isError(candidate: unknown): candidate is Error { case '[object Error]': case '[object Exception]': case '[object DOMException]': + case '[object DOMError]': return true default: return isInstanceOf(candidate, Error) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 6f9845ff7..c5f4a9bec 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -1824,14 +1824,15 @@ export class PostHog { /** Capture a caught exception manually */ captureException(error: Error, additionalProperties?: Properties): void { + const syntheticException = new Error('PostHog syntheticException') const properties: Properties = isFunction(assignableWindow.__PosthogExtensions__?.parseErrorAsProperties) - ? assignableWindow.__PosthogExtensions__.parseErrorAsProperties([ - error.message, - undefined, - undefined, - undefined, - error, - ]) + ? assignableWindow.__PosthogExtensions__.parseErrorAsProperties( + [error.message, undefined, undefined, undefined, error], + // create synthetic error to get stack in cases where user input does not contain one + // creating the exceptionas soon into our code as possible means we should only have to + // remove a single frame (this 'captureException' method) from the resultant stack + { syntheticException } + ) : { $exception_level: 'error', $exception_list: [ diff --git a/src/types.ts b/src/types.ts index 21f49efa3..bd28662cf 100644 --- a/src/types.ts +++ b/src/types.ts @@ -579,6 +579,7 @@ export type ErrorEventArgs = [ export type ErrorMetadata = { handled?: boolean synthetic?: boolean + syntheticException?: Error overrideExceptionType?: string overrideExceptionMessage?: string defaultExceptionType?: string