-
Notifications
You must be signed in to change notification settings - Fork 157
✨ [RUM-10145] collect errors on module evaluation #3622
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
Changes from all commits
4bad010
e3b52bc
a024b45
d14a29f
f6d52ef
e05fedd
7b19999
3068ee7
a4f4d32
555d660
b1cd3b6
cafe16b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import { registerCleanupTask } from '../../test' | ||
import { Observable } from '../tools/observable' | ||
import { clocksNow } from '../tools/utils/timeUtils' | ||
import { BufferedDataType, startBufferingData } from './bufferedData' | ||
import { ErrorHandling, ErrorSource, type RawError } from './error/error.types' | ||
|
||
describe('startBufferingData', () => { | ||
it('collects runtime errors', (done) => { | ||
const runtimeErrorObservable = new Observable<RawError>() | ||
const { observable, stop } = startBufferingData(() => runtimeErrorObservable) | ||
registerCleanupTask(stop) | ||
|
||
const rawError = { | ||
startClocks: clocksNow(), | ||
source: ErrorSource.SOURCE, | ||
type: 'Error', | ||
stack: 'Error: error!', | ||
handling: ErrorHandling.UNHANDLED, | ||
causes: undefined, | ||
fingerprint: undefined, | ||
message: 'error!', | ||
} | ||
|
||
runtimeErrorObservable.notify(rawError) | ||
|
||
observable.subscribe((data) => { | ||
expect(data).toEqual({ | ||
type: BufferedDataType.RUNTIME_ERROR, | ||
error: rawError, | ||
}) | ||
done() | ||
}) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
import { BufferedObservable } from '../tools/observable' | ||
import type { RawError } from './error/error.types' | ||
import { trackRuntimeError } from './error/trackRuntimeError' | ||
|
||
const BUFFER_LIMIT = 500 | ||
|
||
export const enum BufferedDataType { | ||
RUNTIME_ERROR, | ||
} | ||
|
||
export interface BufferedData { | ||
type: BufferedDataType.RUNTIME_ERROR | ||
error: RawError | ||
} | ||
|
||
export function startBufferingData(trackRuntimeErrorImpl = trackRuntimeError) { | ||
const observable = new BufferedObservable<BufferedData>(BUFFER_LIMIT) | ||
|
||
const runtimeErrorSubscription = trackRuntimeErrorImpl().subscribe((error) => { | ||
observable.notify({ | ||
type: BufferedDataType.RUNTIME_ERROR, | ||
error, | ||
}) | ||
}) | ||
|
||
return { | ||
observable, | ||
stop: () => { | ||
runtimeErrorSubscription.unsubscribe() | ||
}, | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,39 +1,40 @@ | ||
import { instrumentMethod } from '../../tools/instrumentMethod' | ||
import type { Observable } from '../../tools/observable' | ||
import { Observable } from '../../tools/observable' | ||
import { clocksNow } from '../../tools/utils/timeUtils' | ||
import type { StackTrace } from '../../tools/stackTrace/computeStackTrace' | ||
import { computeStackTraceFromOnErrorMessage } from '../../tools/stackTrace/computeStackTrace' | ||
import { getGlobalObject } from '../../tools/getGlobalObject' | ||
import { computeRawError, isError } from './error' | ||
import type { RawError } from './error.types' | ||
import { ErrorHandling, ErrorSource, NonErrorPrefix } from './error.types' | ||
|
||
export type UnhandledErrorCallback = (originalError: unknown, stackTrace?: StackTrace) => any | ||
|
||
export function trackRuntimeError(errorObservable: Observable<RawError>) { | ||
const handleRuntimeError = (originalError: unknown, stackTrace?: StackTrace) => { | ||
const rawError = computeRawError({ | ||
stackTrace, | ||
originalError, | ||
startClocks: clocksNow(), | ||
nonErrorPrefix: NonErrorPrefix.UNCAUGHT, | ||
source: ErrorSource.SOURCE, | ||
handling: ErrorHandling.UNHANDLED, | ||
}) | ||
errorObservable.notify(rawError) | ||
} | ||
const { stop: stopInstrumentingOnError } = instrumentOnError(handleRuntimeError) | ||
const { stop: stopInstrumentingOnUnhandledRejection } = instrumentUnhandledRejection(handleRuntimeError) | ||
export function trackRuntimeError() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💬 suggestion: Why not using BufferedObservable directly in here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried, but:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hum, fine by me! Let’s move forward and maybe refine the design later as this pattern is used more widely :) |
||
return new Observable<RawError>((observer) => { | ||
const handleRuntimeError = (originalError: unknown, stackTrace?: StackTrace) => { | ||
const rawError = computeRawError({ | ||
stackTrace, | ||
originalError, | ||
startClocks: clocksNow(), | ||
nonErrorPrefix: NonErrorPrefix.UNCAUGHT, | ||
source: ErrorSource.SOURCE, | ||
handling: ErrorHandling.UNHANDLED, | ||
}) | ||
observer.notify(rawError) | ||
} | ||
const { stop: stopInstrumentingOnError } = instrumentOnError(handleRuntimeError) | ||
const { stop: stopInstrumentingOnUnhandledRejection } = instrumentUnhandledRejection(handleRuntimeError) | ||
|
||
return { | ||
stop: () => { | ||
return () => { | ||
stopInstrumentingOnError() | ||
stopInstrumentingOnUnhandledRejection() | ||
}, | ||
} | ||
} | ||
}) | ||
} | ||
|
||
export function instrumentOnError(callback: UnhandledErrorCallback) { | ||
return instrumentMethod(window, 'onerror', ({ parameters: [messageObj, url, line, column, errorObj] }) => { | ||
return instrumentMethod(getGlobalObject(), 'onerror', ({ parameters: [messageObj, url, line, column, errorObj] }) => { | ||
let stackTrace | ||
if (!isError(errorObj)) { | ||
stackTrace = computeStackTraceFromOnErrorMessage(messageObj, url, line, column) | ||
|
@@ -43,7 +44,7 @@ export function instrumentOnError(callback: UnhandledErrorCallback) { | |
} | ||
|
||
export function instrumentUnhandledRejection(callback: UnhandledErrorCallback) { | ||
return instrumentMethod(window, 'onunhandledrejection', ({ parameters: [e] }) => { | ||
return instrumentMethod(getGlobalObject(), 'onunhandledrejection', ({ parameters: [e] }) => { | ||
callback(e.reason || 'Empty reason') | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 suggestion : Could we put a single const for the buffer limit or is it expected since the other is deprecated?
(see boundedBuffer.ts)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to keep
BufferedObservable
as generic as possible to encourage using it in more places. But I could add a default value in theBufferedObservable
constructor, to keep it configurable while not having to define a limit every single time when it doesn't matter much. WDYT?