Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: lazily load exception autocapture #856

Merged
merged 4 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ module.exports = {
node: true,
},
},
{
files: 'cypress/**/*',
globals: {
cy: true,
},
},
],
root: true,
}
10 changes: 1 addition & 9 deletions cypress/e2e/capture.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('Event capture', () => {
sessionRecording: given.sessionRecording,
supportedCompression: given.supportedCompression,
excludedDomains: [],
autocaptureExceptions: true,
autocaptureExceptions: false,
},
}).as('decide')

Expand Down Expand Up @@ -53,14 +53,6 @@ describe('Event capture', () => {
cy.phCaptures().should('include', 'custom-event')
})

it('captures exceptions', () => {
start()

cy.get('[data-cy-exception-button]').click()
cy.phCaptures().should('have.length', 3)
cy.phCaptures().should('include', '$exception')
})

describe('autocapture config', () => {
it('dont capture click when configured not to', () => {
given('options', () => ({
Expand Down
12 changes: 12 additions & 0 deletions rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,18 @@ export default [
],
plugins: [...plugins],
},
{
input: 'src/loader-exception-autocapture.ts',
output: [
{
file: 'dist/exception-autocapture.js',
sourcemap: true,
format: 'iife',
name: 'posthog',
},
],
plugins: [...plugins],
},
{
input: 'src/loader-globals.ts',
output: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
errorToProperties,
ErrorProperties,
unhandledRejectionToProperties,
} from '../../../extensions/exceptions/error-conversion'
} from '../../../extensions/exception-autocapture/error-conversion'
import { _isNull } from '../../../utils'

// ugh, jest
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
import { PostHog } from '../../../posthog-core'
import { DecideResponse, PostHogConfig } from '../../../types'
import { ExceptionObserver } from '../../../extensions/exceptions/exception-autocapture'
import { ExceptionObserver } from '../../../extensions/exception-autocapture'
import { window } from '../../../utils'

describe('Exception Observer', () => {
let exceptionObserver: ExceptionObserver
Expand Down
23 changes: 20 additions & 3 deletions src/decide.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { autocapture } from './autocapture'
import { _base64Encode, loadScript, logger } from './utils'
import { _base64Encode, _isUndefined, loadScript, logger } from './utils'
import { PostHog } from './posthog-core'
import { Compression, DecideResponse } from './types'
import { STORED_GROUP_PROPERTIES_KEY, STORED_PERSON_PROPERTIES_KEY } from './constants'
Expand Down Expand Up @@ -59,7 +59,6 @@ export class Decide {
this.instance.sessionRecording?.afterDecideResponse(response)
autocapture.afterDecideResponse(response, this.instance)
this.instance.webPerformance?.afterDecideResponse(response)
this.instance.exceptionAutocapture?.afterDecideResponse(response)

if (!this.instance.config.advanced_disable_feature_flags_on_first_load) {
this.instance.featureFlags.receivedFeatureFlags(response)
Expand All @@ -74,7 +73,6 @@ export class Decide {
this.instance['compression'] = compression
}

// Check if recorder.js is already loaded
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const surveysGenerator = window?.extendPostHogWithSurveys
Expand All @@ -91,6 +89,25 @@ export class Decide {
})
}

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const exceptionAutoCaptureAddedToWindow = window?.extendPostHogWithExceptionAutoCapture
if (
response['autocaptureExceptions'] &&
!!response['autocaptureExceptions'] &&
_isUndefined(exceptionAutoCaptureAddedToWindow)
) {
loadScript(this.instance.config.api_host + `/static/exception-autocapture.js`, (err) => {
if (err) {
return logger.error(`Could not load exception autocapture script`, err)
}

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
window.extendPostHogWithExceptionAutocapture(this.instance)
})
}

if (response['siteApps']) {
if (this.instance.config.opt_in_site_apps) {
const apiHost = this.instance.config.api_host
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
import { _isArray, _isUndefined, logger, window } from '../../utils'
import { _isArray, _isObject, _isUndefined, logger, window } from '../../utils'
import { PostHog } from '../../posthog-core'
import { DecideResponse, Properties } from '../../types'
import { ErrorEventArgs, ErrorProperties, errorToProperties, unhandledRejectionToProperties } from './error-conversion'
import { isPrimitive } from './type-checking'

const EXCEPTION_INGESTION_ENDPOINT = '/e/'

export const extendPostHog = (instance: PostHog, response: DecideResponse) => {
const exceptionObserver = new ExceptionObserver(instance)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The place I could see this being called, the decide response isn't passed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good eyes!

exceptionObserver.afterDecideResponse(response)
return exceptionObserver
}

export class ExceptionObserver {
instance: PostHog
remoteEnabled: boolean | undefined
Expand All @@ -18,10 +24,6 @@ export class ExceptionObserver {
this.instance = instance
}

private debugLog(...args: any[]) {
logger.info('[ExceptionObserver]', ...args)
}

startCapturing() {
if (!this.isEnabled() || (window.onerror as any)?.__POSTHOG_INSTRUMENTED__) {
return
Expand Down Expand Up @@ -104,15 +106,18 @@ export class ExceptionObserver {

if (this.isEnabled()) {
this.startCapturing()
this.debugLog('Remote config for exception autocapture is enabled, starting', autocaptureExceptionsResponse)
logger.info(
'[Exception Capture] Remote config for exception autocapture is enabled, starting with config: ',
_isObject(autocaptureExceptionsResponse) ? autocaptureExceptionsResponse : {}
)
}
}

captureException(args: ErrorEventArgs, properties?: Properties) {
const errorProperties = errorToProperties(args)

if (this.errorsToIgnore.some((regex) => regex.test(errorProperties.$exception_message || ''))) {
this.debugLog('Ignoring exception based on remote config', errorProperties)
logger.info('[Exception Capture] Ignoring exception based on remote config', errorProperties)
return
}

Expand Down
9 changes: 7 additions & 2 deletions src/extensions/sentry-integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
*/

import { PostHog } from '../posthog-core'
import { ErrorProperties } from './exceptions/error-conversion'

// 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
Expand Down Expand Up @@ -73,7 +72,13 @@ export class SentryIntegration implements _SentryIntegration {

const exceptions = event.exception?.values || []

const data: SentryExceptionProperties & ErrorProperties = {
const data: SentryExceptionProperties & {
// two properties added to match any exception auto-capture
// added manually to avoid any dependency on the lazily loaded content
$exception_message: any
$exception_type: any
$exception_personURL: string
} = {
// PostHog Exception Properties,
$exception_message: exceptions[0]?.value,
$exception_type: exceptions[0]?.type,
Expand Down
8 changes: 8 additions & 0 deletions src/loader-exception-autocapture.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { extendPostHog } from './extensions/exception-autocapture'
import { _isUndefined } from './utils'

const win: Window & typeof globalThis = _isUndefined(window) ? ({} as typeof window) : window

;(win as any).extendPostHogWithExceptionAutoCapture = extendPostHog

export default extendPostHog
21 changes: 0 additions & 21 deletions src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ import {
import { SentryIntegration } from './extensions/sentry-integration'
import { createSegmentIntegration } from './extensions/segment-integration'
import { PageViewManager } from './page-view'
import { ExceptionObserver } from './extensions/exceptions/exception-autocapture'
import { PostHogSurveys } from './posthog-surveys'
import { RateLimiter } from './rate-limiter'
import { uuidv7 } from './uuidv7'
Expand Down Expand Up @@ -224,8 +223,6 @@ const create_phlib = function (
instance.pageViewManager.startMeasuringScrollPosition()
}

instance.exceptionAutocapture = new ExceptionObserver(instance)

instance.__autocapture = instance.config.autocapture
autocapture._setIsAutocaptureEnabled(instance)
if (autocapture._isAutocaptureEnabled) {
Expand Down Expand Up @@ -284,7 +281,6 @@ export class PostHog {
_retryQueue?: RetryQueue
sessionRecording?: SessionRecording
webPerformance?: WebPerformanceObserver
exceptionAutocapture?: ExceptionObserver

_triggered_notifs: any
compression: Partial<Record<Compression, boolean>>
Expand Down Expand Up @@ -795,23 +791,6 @@ export class PostHog {
this._execute_array([item])
}

/*
* PostHog supports exception autocapture, however, this function
* is used to manually capture an exception
* and can be used to add more context to that exception
*
* Properties passed as the second option will be merged with the properties
* of the exception event.
* Where there is a key in both generated exception and passed properties,
* the generated exception property takes precedence.
*/
captureException(exception: Error, properties?: Properties): void {
this.exceptionAutocapture?.captureException(
[exception.name, undefined, undefined, undefined, exception],
properties
)
}

/**
* Capture an event. This is the most important and
* frequently used PostHog function.
Expand Down
Loading