From dc66bcf3e16a373cffede7e383a88fcfa6b6e459 Mon Sep 17 00:00:00 2001 From: David Newell Date: Thu, 29 Aug 2024 10:47:07 +0100 Subject: [PATCH] feat: manually capture errors (#1374) --- cypress/e2e/error-tracking.cy.ts | 75 ++++++++++++++----- playground/cypress-full/index.html | 2 +- playground/cypress/index.html | 6 +- src/constants.ts | 2 +- src/entrypoints/exception-autocapture.ts | 1 + src/extensions/exception-autocapture/index.ts | 40 +--------- src/extensions/sentry-integration.ts | 9 +-- src/posthog-core.ts | 18 +++++ src/posthog-exceptions.ts | 50 +++++++++++++ 9 files changed, 139 insertions(+), 64 deletions(-) create mode 100644 src/posthog-exceptions.ts diff --git a/cypress/e2e/error-tracking.cy.ts b/cypress/e2e/error-tracking.cy.ts index 36ed5f092..2b58ede97 100644 --- a/cypress/e2e/error-tracking.cy.ts +++ b/cypress/e2e/error-tracking.cy.ts @@ -1,23 +1,15 @@ import { start } from '../support/setup' -describe('Exception autocapture', () => { - beforeEach(() => { - cy.on('uncaught:exception', () => { - // otherwise the exception we throw on purpose causes the test to fail - return false - }) - +describe('Exception capture', () => { + it('manual exception capture', () => { start({ decideResponseOverrides: { - autocaptureExceptions: true, + autocaptureExceptions: false, }, url: './playground/cypress', }) - cy.wait('@exception-autocapture-script') - }) - it('captures exceptions', () => { - cy.get('[data-cy-button-throws-error]').click() + cy.get('[data-cy-exception-button]').click() // ugh cy.wait(1500) @@ -25,12 +17,61 @@ describe('Exception autocapture', () => { 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_message).to.be.eql('This is an error') + expect(captures[2].properties.$exception_message).to.be.eql('wat even am I') expect(captures[2].properties.$exception_type).to.be.eql('Error') - expect(captures[2].properties.$exception_source).to.match(/http:\/\/localhost:\d+\/playground\/cypress\//) - expect(captures[2].properties.$exception_personURL).to.match( - /http:\/\/localhost:\d+\/project\/test_token\/person\/[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}/ - ) + expect(captures[2].properties.extra_prop).to.be.eql(2) + expect(captures[2].properties.$exception_source).to.eql(undefined) + expect(captures[2].properties.$exception_personURL).to.eql(undefined) + expect(captures[2].properties.$exception_stack_trace_raw).not.to.exist + }) + }) + + describe('Exception autocapture enabled', () => { + beforeEach(() => { + cy.on('uncaught:exception', () => { + // otherwise the exception we throw on purpose causes the test to fail + return false + }) + + start({ + decideResponseOverrides: { + autocaptureExceptions: true, + }, + url: './playground/cypress', + }) + cy.wait('@exception-autocapture-script') + }) + + it('autocaptures exceptions', () => { + cy.get('[data-cy-button-throws-error]').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_message).to.be.eql('This is an error') + expect(captures[2].properties.$exception_type).to.be.eql('Error') + expect(captures[2].properties.$exception_source).to.match( + /http:\/\/localhost:\d+\/playground\/cypress\// + ) + expect(captures[2].properties.$exception_personURL).to.match( + /http:\/\/localhost:\d+\/project\/test_token\/person\/[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}/ + ) + }) + }) + + it('sets stacktrace on manual captures if autocapture enabled', () => { + cy.get('[data-cy-exception-button]').click() + + // ugh + cy.wait(1500) + + cy.phCaptures({ full: true }).then((captures) => { + expect(captures[2].properties.$exception_message).to.be.eql('wat even am I') + expect(captures[2].properties.$exception_stack_trace_raw).to.exist + }) }) }) }) diff --git a/playground/cypress-full/index.html b/playground/cypress-full/index.html index 4b53160bc..a6807cee8 100644 --- a/playground/cypress-full/index.html +++ b/playground/cypress-full/index.html @@ -43,7 +43,7 @@
diff --git a/playground/cypress/index.html b/playground/cypress/index.html index f7b111e50..81ab457d6 100644 --- a/playground/cypress/index.html +++ b/playground/cypress/index.html @@ -48,8 +48,12 @@ Throw error + + diff --git a/src/constants.ts b/src/constants.ts index c6e17149d..8913847fa 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -13,7 +13,7 @@ export const EVENT_TIMERS_KEY = '__timers' export const AUTOCAPTURE_DISABLED_SERVER_SIDE = '$autocapture_disabled_server_side' export const HEATMAPS_ENABLED_SERVER_SIDE = '$heatmaps_enabled_server_side' export const EXCEPTION_CAPTURE_ENABLED_SERVER_SIDE = '$exception_capture_enabled_server_side' -export const EXCEPTION_CAPTURE_ENDPOINT = '$exception_capture_endpoint' +export const EXCEPTION_CAPTURE_ENDPOINT_SUFFIX = '$exception_capture_endpoint_suffix' export const WEB_VITALS_ENABLED_SERVER_SIDE = '$web_vitals_enabled_server_side' export const SESSION_RECORDING_ENABLED_SERVER_SIDE = '$session_recording_enabled_server_side' export const CONSOLE_LOG_RECORDING_ENABLED_SERVER_SIDE = '$console_log_recording_enabled_server_side' diff --git a/src/entrypoints/exception-autocapture.ts b/src/entrypoints/exception-autocapture.ts index 3cf09c41c..f1411b62d 100644 --- a/src/entrypoints/exception-autocapture.ts +++ b/src/entrypoints/exception-autocapture.ts @@ -51,6 +51,7 @@ const posthogErrorWrappingFunctions = { if (window) { ;(window as any).posthogErrorWrappingFunctions = posthogErrorWrappingFunctions + ;(window as any).parseErrorAsProperties = errorToProperties } export default posthogErrorWrappingFunctions diff --git a/src/extensions/exception-autocapture/index.ts b/src/extensions/exception-autocapture/index.ts index 605974e81..d820812b5 100644 --- a/src/extensions/exception-autocapture/index.ts +++ b/src/extensions/exception-autocapture/index.ts @@ -2,17 +2,13 @@ import { window } from '../../utils/globals' import { PostHog } from '../../posthog-core' import { DecideResponse, Properties } from '../../types' -import { isObject } from '../../utils/type-utils' import { logger } from '../../utils/logger' -import { EXCEPTION_CAPTURE_ENABLED_SERVER_SIDE, EXCEPTION_CAPTURE_ENDPOINT } from '../../constants' +import { EXCEPTION_CAPTURE_ENABLED_SERVER_SIDE } from '../../constants' import Config from '../../config' -// TODO: move this to /x/ as default -export const BASE_ERROR_ENDPOINT = '/e/' -const LOGGER_PREFIX = '[Exception Capture]' +const LOGGER_PREFIX = '[Exception Autocapture]' export class ExceptionObserver { - private _endpointSuffix: string instance: PostHog remoteEnabled: boolean | undefined private originalOnUnhandledRejectionHandler: Window['onunhandledrejection'] | null | undefined = undefined @@ -23,17 +19,9 @@ export class ExceptionObserver { this.instance = instance this.remoteEnabled = !!this.instance.persistence?.props[EXCEPTION_CAPTURE_ENABLED_SERVER_SIDE] - // TODO: once BASE_ERROR_ENDPOINT is no longer /e/ this can be removed - this._endpointSuffix = this.instance.persistence?.props[EXCEPTION_CAPTURE_ENDPOINT] || BASE_ERROR_ENDPOINT - this.startIfEnabled() } - get endpoint() { - // Always respect any api_host set by the client config - return this.instance.requestRouter.endpointFor('api', this._endpointSuffix) - } - get isEnabled() { return this.remoteEnabled ?? false } @@ -68,7 +56,7 @@ export class ExceptionObserver { } private startCapturing = () => { - if (!window || !this.isEnabled || this.hasHandlers || (window.onerror as any)?.__POSTHOG_INSTRUMENTED__) { + if (!window || !this.isEnabled || this.hasHandlers || this.isCapturing) { return } @@ -99,20 +87,11 @@ export class ExceptionObserver { // store this in-memory in case persistence is disabled this.remoteEnabled = !!autocaptureExceptionsResponse || false - this._endpointSuffix = isObject(autocaptureExceptionsResponse) - ? autocaptureExceptionsResponse.endpoint || BASE_ERROR_ENDPOINT - : BASE_ERROR_ENDPOINT if (this.instance.persistence) { this.instance.persistence.register({ [EXCEPTION_CAPTURE_ENABLED_SERVER_SIDE]: this.remoteEnabled, }) - // when we come to moving the endpoint to not /e/ - // we'll want that to persist between startup and decide response - // TODO: once BASE_ENDPOINT is no longer /e/ this can be removed - this.instance.persistence.register({ - [EXCEPTION_CAPTURE_ENDPOINT]: this._endpointSuffix, - }) } this.startIfEnabled() @@ -125,17 +104,6 @@ export class ExceptionObserver { this.instance.config.token }/person/${this.instance.get_distinct_id()}` - this.sendExceptionEvent(errorProperties) - } - - /** - * :TRICKY: Make sure we batch these requests - */ - sendExceptionEvent(properties: { [key: string]: any }) { - this.instance.capture('$exception', properties, { - _noTruncate: true, - _batchKey: 'exceptionEvent', - _url: this.endpoint, - }) + this.instance.exceptions.sendExceptionEvent(errorProperties) } } diff --git a/src/extensions/sentry-integration.ts b/src/extensions/sentry-integration.ts index 4b6bf9bb0..4930fdfd0 100644 --- a/src/extensions/sentry-integration.ts +++ b/src/extensions/sentry-integration.ts @@ -18,7 +18,6 @@ import { PostHog } from '../posthog-core' import { SeverityLevel } from '../types' -import { BASE_ERROR_ENDPOINT } from './exception-autocapture' // NOTE - we can't import from @sentry/types because it changes frequently and causes clashes // We only use a small subset of the types, so we can just define the integration overall and use any for the rest @@ -126,13 +125,7 @@ export function createEventProcessor( event.event_id } - // we take the URL from the exception observer - // so that when we add error specific URL for ingestion - // these errors are sent there too - _posthog.capture('$exception', data, { - _url: - _posthog.exceptionObserver?.endpoint || _posthog.requestRouter.endpointFor('api', BASE_ERROR_ENDPOINT), - }) + _posthog.exceptions.sendExceptionEvent(data) return event } diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 32b71f971..d8cf8c3d9 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -77,6 +77,7 @@ import { TracingHeaders } from './extensions/tracing-headers' import { ConsentManager } from './consent' import { ExceptionObserver } from './extensions/exception-autocapture' import { WebVitalsAutocapture } from './extensions/web-vitals' +import { PostHogExceptions } from './posthog-exceptions' /* SIMPLE STYLE GUIDE: @@ -242,6 +243,7 @@ export class PostHog { featureFlags: PostHogFeatureFlags surveys: PostHogSurveys toolbar: Toolbar + exceptions: PostHogExceptions consent: ConsentManager // These are instance-specific state created after initialisation @@ -295,6 +297,7 @@ export class PostHog { this.scrollManager = new ScrollManager(this) this.pageViewManager = new PageViewManager(this) this.surveys = new PostHogSurveys(this) + this.exceptions = new PostHogExceptions(this) this.rateLimiter = new RateLimiter(this) this.requestRouter = new RequestRouter(this) this.consent = new ConsentManager(this) @@ -536,6 +539,7 @@ export class PostHog { this.heatmaps?.afterDecideResponse(response) this.surveys?.afterDecideResponse(response) this.webVitalsAutocapture?.afterDecideResponse(response) + this.exceptions?.afterDecideResponse(response) this.exceptionObserver?.afterDecideResponse(response) } @@ -1811,6 +1815,20 @@ export class PostHog { return !!this.sessionRecording?.started } + /** Capture a caught exception manually */ + captureException(error: Error, additionalProperties?: Properties): void { + const properties: Properties = isFunction(assignableWindow.parseErrorAsProperties) + ? assignableWindow.parseErrorAsProperties([error.message, undefined, undefined, undefined, error]) + : { + $exception_type: error.name, + $exception_message: error.message, + $exception_level: 'error', + ...additionalProperties, + } + + this.exceptions.sendExceptionEvent(properties) + } + /** * returns a boolean indicating whether the toolbar loaded * @param toolbarParams diff --git a/src/posthog-exceptions.ts b/src/posthog-exceptions.ts new file mode 100644 index 000000000..f8cc687a2 --- /dev/null +++ b/src/posthog-exceptions.ts @@ -0,0 +1,50 @@ +import { EXCEPTION_CAPTURE_ENDPOINT_SUFFIX } from './constants' +import { PostHog } from './posthog-core' +import { DecideResponse, Properties } from './types' +import { isObject } from './utils/type-utils' + +// TODO: move this to /x/ as default +export const BASE_ERROR_ENDPOINT_SUFFIX = '/e/' + +export class PostHogExceptions { + private _endpointSuffix: string + + constructor(private readonly instance: PostHog) { + // TODO: once BASE_ERROR_ENDPOINT_SUFFIX is no longer /e/ this can be removed + this._endpointSuffix = + this.instance.persistence?.props[EXCEPTION_CAPTURE_ENDPOINT_SUFFIX] || BASE_ERROR_ENDPOINT_SUFFIX + } + + get endpoint() { + // Always respect any api_host set by the client config + return this.instance.requestRouter.endpointFor('api', this._endpointSuffix) + } + + afterDecideResponse(response: DecideResponse) { + const autocaptureExceptionsResponse = response.autocaptureExceptions + + this._endpointSuffix = isObject(autocaptureExceptionsResponse) + ? autocaptureExceptionsResponse.endpoint || BASE_ERROR_ENDPOINT_SUFFIX + : BASE_ERROR_ENDPOINT_SUFFIX + + if (this.instance.persistence) { + // when we come to moving the endpoint to not /e/ + // we'll want that to persist between startup and decide response + // TODO: once BASE_ENDPOINT is no longer /e/ this can be removed + this.instance.persistence.register({ + [EXCEPTION_CAPTURE_ENDPOINT_SUFFIX]: this._endpointSuffix, + }) + } + } + + /** + * :TRICKY: Make sure we batch these requests + */ + sendExceptionEvent(properties: Properties) { + this.instance.capture('$exception', properties, { + _noTruncate: true, + _batchKey: 'exceptionEvent', + _url: this.endpoint, + }) + } +}