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.

Sign up for GitHub

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

fix(errors): Better define schema, align with python #1460

Merged
merged 10 commits into from
Oct 15, 2024
Merged

fix(errors): Better define schema, align with python #1460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e1ab25d
fix(errors): Better define schema, align with python
neilkakkar Oct 10, 2024
57cf509
sus edit
neilkakkar Oct 10, 2024
7b5aca3
fix tests
neilkakkar Oct 10, 2024
89d2f46
Apply suggestions from code review
neilkakkar Oct 11, 2024
7223c4d
add
neilkakkar Oct 14, 2024
925886c
Merge branch 'schema-define' of github.com:PostHog/posthog-js into sc…
neilkakkar Oct 14, 2024
53359d3
rework exception schema
neilkakkar Oct 14, 2024
ae1e74e
cypress
neilkakkar Oct 14, 2024
27cf484
fix type
neilkakkar Oct 14, 2024
ab4bd72
make exception_list required
neilkakkar Oct 14, 2024
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
17 changes: 7 additions & 10 deletions cypress/e2e/error-tracking.cy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,11 @@ describe('Exception capture', () => {
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('wat even am I')
expect(captures[2].properties.$exception_type).to.be.eql('Error')
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
expect(captures[2].properties.$exception_list[0].value).to.be.eql('wat even am I')
expect(captures[2].properties.$exception_list[0].type).to.be.eql('Error')
})
})

Expand Down Expand Up @@ -51,11 +50,9 @@ describe('Exception capture', () => {
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_list[0].value).to.be.eql('This is an error')
expect(captures[2].properties.$exception_list[0].type).to.be.eql('Error')

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}/
)
Expand All @@ -69,8 +66,8 @@ describe('Exception capture', () => {
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
expect(captures[2].properties.$exception_list).to.exist
expect(captures[2].properties.$exception_list[0].value).to.be.eql('wat even am I')
})
})
})
Expand Down
115 changes: 72 additions & 43 deletions src/__tests__/extensions/exception-autocapture/error-conversion.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
/* eslint-disable compat/compat */

import {
ErrorProperties,
errorToProperties,
unhandledRejectionToProperties,
} from '../../../extensions/exception-autocapture/error-conversion'

import { isNull } from '../../../utils/type-utils'
import { expect } from '@jest/globals'
import { ErrorProperties } from '../../../types'

// ugh, jest
// can't reference PromiseRejectionEvent to construct it 🤷
Expand All @@ -34,30 +34,42 @@ export class PromiseRejectionEvent extends Event {
describe('Error conversion', () => {
it('should convert a string to an error', () => {
const expected: ErrorProperties = {
$exception_type: 'InternalError',
$exception_message: 'but somehow still a string',
$exception_is_synthetic: true,
$exception_level: 'error',
$exception_list: [
{
type: 'InternalError',
value: 'but somehow still a string',
mechanism: { synthetic: true, handled: true },
},
],
}
expect(errorToProperties(['Uncaught exception: InternalError: but somehow still a string'])).toEqual(expected)
})

it('should convert a plain object to an error', () => {
const expected: ErrorProperties = {
$exception_type: 'Error',
$exception_message: 'Non-Error exception captured with keys: foo, string',
$exception_is_synthetic: true,
$exception_level: 'error',
$exception_list: [
{
type: 'Error',
value: 'Non-Error exception captured with keys: foo, string',
mechanism: { synthetic: true, handled: true },
},
],
}
expect(errorToProperties([{ string: 'candidate', foo: 'bar' } as unknown as Event])).toEqual(expected)
})

it('should convert a plain Event to an error', () => {
const expected: ErrorProperties = {
$exception_type: 'MouseEvent',
$exception_message: 'Non-Error exception captured with keys: isTrusted',
$exception_is_synthetic: true,
$exception_level: 'error',
$exception_list: [
{
type: 'MouseEvent',
value: 'Non-Error exception captured with keys: isTrusted',
mechanism: { synthetic: true, handled: true },
},
],
}
const event = new MouseEvent('click', { bubbles: true, cancelable: true, composed: true })
expect(errorToProperties([event])).toEqual(expected)
Expand All @@ -71,13 +83,16 @@ describe('Error conversion', () => {
throw new Error("this mustn't be null")
}

expect(Object.keys(errorProperties)).toHaveLength(4)
expect(errorProperties.$exception_type).toEqual('Error')
expect(errorProperties.$exception_message).toEqual('oh no an error has happened')
expect(Object.keys(errorProperties)).toHaveLength(2)
expect(errorProperties.$exception_level).toEqual('error')
// the stack trace changes between runs, so we just check that it's there
expect(errorProperties.$exception_stack_trace_raw).toBeDefined()
expect(errorProperties.$exception_stack_trace_raw).toContain('{"filename')
expect(errorProperties.$exception_list).toBeDefined()
expect(errorProperties.$exception_list[0].type).toEqual('Error')
expect(errorProperties.$exception_list[0].value).toEqual('oh no an error has happened')
expect(errorProperties.$exception_list[0].stacktrace.frames[0].in_app).toEqual(true)
expect(errorProperties.$exception_list[0].stacktrace.frames[0].filename).toBeDefined()
expect(errorProperties.$exception_list[0].mechanism.synthetic).toEqual(false)
expect(errorProperties.$exception_list[0].mechanism.handled).toEqual(true)
})

class FakeDomError {
Expand All @@ -87,9 +102,14 @@ describe('Error conversion', () => {

it('should convert a DOM Error to an error', () => {
const expected: ErrorProperties = {
$exception_type: 'DOMError',
$exception_message: 'click: foo',
$exception_level: 'error',
$exception_list: [
{
type: 'DOMError',
value: 'click: foo',
mechanism: { synthetic: true, handled: true },
},
],
}
const event = new FakeDomError('click', 'foo')
expect(errorToProperties([event as unknown as Event])).toEqual(expected)
Expand All @@ -103,13 +123,15 @@ describe('Error conversion', () => {
throw new Error("this mustn't be null")
}

expect(Object.keys(errorProperties)).toHaveLength(5)
expect(errorProperties.$exception_type).toEqual('dom-exception')
expect(errorProperties.$exception_message).toEqual('oh no disaster')
expect(Object.keys(errorProperties)).toHaveLength(3)
expect(errorProperties.$exception_list[0].type).toEqual('dom-exception')
expect(errorProperties.$exception_list[0].value).toEqual('oh no disaster')
expect(errorProperties.$exception_DOMException_code).toEqual('0')
expect(errorProperties.$exception_level).toEqual('error')
// the stack trace changes between runs, so we just check that it's there
expect(errorProperties.$exception_stack_trace_raw).toBeDefined()
expect(errorProperties.$exception_stack_trace_raw).toContain('{"filename')
expect(errorProperties.$exception_list).toBeDefined()
expect(errorProperties.$exception_list[0].stacktrace.frames[0].in_app).toEqual(true)
expect(errorProperties.$exception_list[0].stacktrace.frames[0].filename).toBeDefined()
})

it('should convert an error event to an error', () => {
Expand All @@ -120,24 +142,28 @@ describe('Error conversion', () => {
throw new Error("this mustn't be null")
}

expect(Object.keys(errorProperties)).toHaveLength(4)
expect(errorProperties.$exception_type).toEqual('Error')
expect(errorProperties.$exception_message).toEqual('the real error is hidden inside')
expect(Object.keys(errorProperties)).toHaveLength(2)
expect(errorProperties.$exception_list[0].type).toEqual('Error')
expect(errorProperties.$exception_list[0].value).toEqual('the real error is hidden inside')
expect(errorProperties.$exception_level).toEqual('error')
// the stack trace changes between runs, so we just check that it's there
expect(errorProperties.$exception_stack_trace_raw).toBeDefined()
expect(errorProperties.$exception_stack_trace_raw).toContain('{"filename')
expect(errorProperties.$exception_list).toBeDefined()
expect(errorProperties.$exception_list[0].stacktrace.frames[0].in_app).toEqual(true)
expect(errorProperties.$exception_list[0].stacktrace.frames[0].filename).toBeDefined()
expect(errorProperties.$exception_list[0].mechanism.synthetic).toEqual(false)
expect(errorProperties.$exception_list[0].mechanism.handled).toEqual(true)
})

it('can convert source, lineno, colno', () => {
const expected: ErrorProperties = {
$exception_colno: 200,
$exception_is_synthetic: true,
$exception_lineno: 12,
$exception_message: 'string candidate',
$exception_source: 'a source',
$exception_type: 'Error',
$exception_level: 'error',
$exception_list: [
{
type: 'Error',
value: 'string candidate',
mechanism: { synthetic: true, handled: true },
},
],
}
expect(errorToProperties(['string candidate', 'a source', 12, 200])).toEqual(expected)
})
Expand All @@ -152,14 +178,16 @@ describe('Error conversion', () => {
const errorProperties: ErrorProperties = unhandledRejectionToProperties([
ce as unknown as PromiseRejectionEvent,
])
expect(Object.keys(errorProperties)).toHaveLength(5)
expect(errorProperties.$exception_type).toEqual('UnhandledRejection')
expect(errorProperties.$exception_message).toEqual('a wrapped rejection event')
expect(errorProperties.$exception_handled).toEqual(false)
expect(Object.keys(errorProperties)).toHaveLength(2)
expect(errorProperties.$exception_list[0].type).toEqual('UnhandledRejection')
expect(errorProperties.$exception_list[0].value).toEqual('a wrapped rejection event')
expect(errorProperties.$exception_level).toEqual('error')
// the stack trace changes between runs, so we just check that it's there
expect(errorProperties.$exception_stack_trace_raw).toBeDefined()
expect(errorProperties.$exception_stack_trace_raw).toContain('{"filename')
expect(errorProperties.$exception_list).toBeDefined()
expect(errorProperties.$exception_list[0].stacktrace.frames[0].in_app).toEqual(true)
expect(errorProperties.$exception_list[0].stacktrace.frames[0].filename).toBeDefined()
expect(errorProperties.$exception_list[0].mechanism.synthetic).toEqual(false)
expect(errorProperties.$exception_list[0].mechanism.handled).toEqual(false)
})

it('should convert unhandled promise rejection', () => {
Expand All @@ -170,12 +198,13 @@ describe('Error conversion', () => {
const errorProperties: ErrorProperties = unhandledRejectionToProperties([
pre as unknown as PromiseRejectionEvent,
])
expect(Object.keys(errorProperties)).toHaveLength(4)
expect(errorProperties.$exception_type).toEqual('UnhandledRejection')
expect(errorProperties.$exception_message).toEqual(
expect(Object.keys(errorProperties)).toHaveLength(2)
expect(errorProperties.$exception_list[0].type).toEqual('UnhandledRejection')
expect(errorProperties.$exception_list[0].value).toEqual(
'Non-Error promise rejection captured with value: My house is on fire'
)
expect(errorProperties.$exception_handled).toEqual(false)
expect(errorProperties.$exception_level).toEqual('error')
expect(errorProperties.$exception_list[0].mechanism.synthetic).toEqual(false)
expect(errorProperties.$exception_list[0].mechanism.handled).toEqual(false)
})
})
31 changes: 24 additions & 7 deletions src/__tests__/extensions/exception-autocapture/exception-observer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,15 @@ describe('Exception Observer', () => {
expect(singleCall[0]).toBe('$exception')
expect(singleCall[1]).toMatchObject({
properties: {
$exception_message: 'test error',
$exception_type: 'Error',
$exception_personURL: expect.any(String),
$exception_list: [
{
type: 'Error',
value: 'test error',
stacktrace: { frames: expect.any(Array) },
mechanism: { synthetic: false, handled: true },
},
],
},
})
})
Expand All @@ -120,9 +126,15 @@ describe('Exception Observer', () => {
expect(singleCall[0]).toBe('$exception')
expect(singleCall[1]).toMatchObject({
properties: {
$exception_message: 'test error',
$exception_type: 'UnhandledRejection',
$exception_personURL: expect.any(String),
$exception_list: [
{
type: 'UnhandledRejection',
value: 'test error',
stacktrace: { frames: expect.any(Array) },
mechanism: { synthetic: false, handled: false },
},
],
},
})
})
Expand All @@ -137,10 +149,15 @@ describe('Exception Observer', () => {
expect(request.data).toMatchObject({
event: '$exception',
properties: {
$exception_message: 'test error',
$exception_type: 'Error',
$exception_personURL: expect.any(String),
$exception_stack_trace_raw: expect.any(String),
$exception_list: [
{
type: 'Error',
value: 'test error',
stacktrace: { frames: expect.any(Array) },
mechanism: { synthetic: false, handled: true },
},
],
},
})
expect(request.batchKey).toBe('exceptionEvent')
Expand Down
Loading
Loading