From c22873e8148a4701473237e6e884bdfc602122ea Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Wed, 7 Jul 2021 15:17:31 +0200 Subject: [PATCH 1/4] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Put=20each=20error=20t?= =?UTF-8?q?ype=20in=20its=20own=20tracker=20file?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../domain/automaticErrorCollection.spec.ts | 292 ------------------ .../src/domain/automaticErrorCollection.ts | 151 --------- .../domain/error/automaticErrorCollection.ts | 19 ++ .../domain/error/trackConsoleError.spec.ts | 79 +++++ .../src/domain/error/trackConsoleError.ts | 58 ++++ .../domain/error/trackNetworkError.spec.ts | 152 +++++++++ .../src/domain/error/trackNetworkError.ts | 60 ++++ .../domain/error/trackRuntimeError.spec.ts | 62 ++++ .../src/domain/error/trackRuntimeError.ts | 27 ++ packages/core/src/index.ts | 4 +- packages/core/src/tools/observable.ts | 4 + 11 files changed, 463 insertions(+), 445 deletions(-) delete mode 100644 packages/core/src/domain/automaticErrorCollection.spec.ts delete mode 100644 packages/core/src/domain/automaticErrorCollection.ts create mode 100644 packages/core/src/domain/error/automaticErrorCollection.ts create mode 100644 packages/core/src/domain/error/trackConsoleError.spec.ts create mode 100644 packages/core/src/domain/error/trackConsoleError.ts create mode 100644 packages/core/src/domain/error/trackNetworkError.spec.ts create mode 100644 packages/core/src/domain/error/trackNetworkError.ts create mode 100644 packages/core/src/domain/error/trackRuntimeError.spec.ts create mode 100644 packages/core/src/domain/error/trackRuntimeError.ts diff --git a/packages/core/src/domain/automaticErrorCollection.spec.ts b/packages/core/src/domain/automaticErrorCollection.spec.ts deleted file mode 100644 index f627662182..0000000000 --- a/packages/core/src/domain/automaticErrorCollection.spec.ts +++ /dev/null @@ -1,292 +0,0 @@ -import { ErrorHandling, ErrorSource, RawError } from '../tools/error' -import { Observable } from '../tools/observable' -import { FetchStub, FetchStubManager, isIE, SPEC_ENDPOINTS, stubFetch } from '../../test/specHelper' -import { includes } from '../tools/utils' -import { - startConsoleTracking, - startRuntimeErrorTracking, - stopConsoleTracking, - stopRuntimeErrorTracking, - trackNetworkError, -} from './automaticErrorCollection' -import { Configuration } from './configuration' - -/* eslint-disable no-console */ -describe('console tracker', () => { - let consoleErrorStub: jasmine.Spy - let notifyError: jasmine.Spy - const CONSOLE_CONTEXT = { - source: ErrorSource.CONSOLE, - } - - beforeEach(() => { - consoleErrorStub = spyOn(console, 'error') - notifyError = jasmine.createSpy('notifyError') - const errorObservable = new Observable() - errorObservable.subscribe(notifyError) - startConsoleTracking(errorObservable) - }) - - afterEach(() => { - stopConsoleTracking() - }) - - it('should keep original behavior', () => { - console.error('foo', 'bar') - expect(consoleErrorStub).toHaveBeenCalledWith('foo', 'bar') - }) - - it('should notify error', () => { - console.error('foo', 'bar') - expect(notifyError).toHaveBeenCalledWith({ - ...CONSOLE_CONTEXT, - message: 'console error: foo bar', - stack: undefined, - handlingStack: jasmine.any(String), - startClocks: jasmine.any(Object), - handling: ErrorHandling.HANDLED, - }) - }) - - it('should generate a handling stack', () => { - function triggerError() { - console.error('foo', 'bar') - } - triggerError() - const rawError = notifyError.calls.mostRecent().args[0] as RawError - expect(rawError.handlingStack).toMatch(/^Error:\s+at triggerError (.|\n)*$/) - }) - - it('should stringify object parameters', () => { - console.error('Hello', { foo: 'bar' }) - expect(notifyError).toHaveBeenCalledWith({ - ...CONSOLE_CONTEXT, - message: 'console error: Hello {\n "foo": "bar"\n}', - stack: undefined, - handlingStack: jasmine.any(String), - startClocks: jasmine.any(Object), - handling: ErrorHandling.HANDLED, - }) - }) - - it('should format error instance', () => { - console.error(new TypeError('hello')) - expect((notifyError.calls.mostRecent().args[0] as RawError).message).toBe('console error: TypeError: hello') - }) - - it('should extract stack from first error', () => { - console.error(new TypeError('foo'), new TypeError('bar')) - const stack = (notifyError.calls.mostRecent().args[0] as RawError).stack - if (!isIE()) { - expect(stack).toMatch(/^TypeError: foo\s+at/) - } else { - expect(stack).toContain('TypeError: foo') - } - }) -}) -/* eslint-enable no-console */ - -describe('runtime error tracker', () => { - const ERROR_MESSAGE = 'foo' - let originalHandler: OnErrorEventHandler - let notifyError: jasmine.Spy - let onerrorSpy: jasmine.Spy - - beforeEach(() => { - originalHandler = window.onerror - onerrorSpy = jasmine.createSpy() - window.onerror = onerrorSpy - - notifyError = jasmine.createSpy() - const errorObservable = new Observable() - errorObservable.subscribe((e: RawError) => notifyError(e) as void) - - startRuntimeErrorTracking(errorObservable) - }) - - afterEach(() => { - stopRuntimeErrorTracking() - window.onerror = originalHandler - }) - - it('should call original error handler', (done) => { - setTimeout(() => { - throw new Error(ERROR_MESSAGE) - }, 10) - - setTimeout(() => { - expect(onerrorSpy.calls.mostRecent().args[0]).toMatch(ERROR_MESSAGE) - done() - }, 100) - }) - - it('should notify error', (done) => { - setTimeout(() => { - throw new Error(ERROR_MESSAGE) - }, 10) - - setTimeout(() => { - expect((notifyError.calls.mostRecent().args[0] as RawError).message).toEqual(ERROR_MESSAGE) - done() - }, 100) - }) - - it('should handle direct onerror calls with objects', (done) => { - setTimeout(() => { - window.onerror!({ foo: 'bar' } as any) - }, 10) - - setTimeout(() => { - const collectedError = notifyError.calls.mostRecent().args[0] as RawError - expect(collectedError.message).toEqual('Uncaught {"foo":"bar"}') - expect(collectedError.stack).toEqual('No stack, consider using an instance of Error') - done() - }, 100) - }) -}) - -describe('network error tracker', () => { - let errorObservableSpy: jasmine.Spy - let fetchStub: FetchStub - let fetchStubManager: FetchStubManager - let stopNetworkErrorTracking: () => void - let enabledExperimentalFeatures: string[] - const FAKE_URL = 'http://fake.com/' - const DEFAULT_REQUEST = { - duration: 10, - method: 'GET', - responseText: 'Server error', - startTime: 0, - status: 503, - url: FAKE_URL, - } - - beforeEach(() => { - if (isIE()) { - pending('no fetch support') - } - enabledExperimentalFeatures = [] - - const errorObservable = new Observable() - errorObservableSpy = spyOn(errorObservable, 'notify') - const configuration = { - requestErrorResponseLengthLimit: 32, - isEnabled(featureFlag: string) { - return includes(enabledExperimentalFeatures, featureFlag) - }, - ...SPEC_ENDPOINTS, - } - - fetchStubManager = stubFetch() - ;({ stop: stopNetworkErrorTracking } = trackNetworkError(configuration as Configuration, errorObservable)) - fetchStub = window.fetch as FetchStub - }) - - afterEach(() => { - fetchStubManager.reset() - stopNetworkErrorTracking() - }) - - it('should track server error', (done) => { - fetchStub(FAKE_URL).resolveWith(DEFAULT_REQUEST) - - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).toHaveBeenCalledWith({ - message: 'Fetch error GET http://fake.com/', - resource: { - method: 'GET', - statusCode: 503, - url: 'http://fake.com/', - }, - source: 'network', - stack: 'Server error', - startClocks: jasmine.any(Object), - }) - done() - }) - }) - - it('should not track intake error', (done) => { - fetchStub('https://logs-intake.com/v1/input/send?foo=bar').resolveWith(DEFAULT_REQUEST) - - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).not.toHaveBeenCalled() - done() - }) - }) - - it('should track aborted requests ', (done) => { - fetchStub(FAKE_URL).abort() - - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).toHaveBeenCalled() - done() - }) - }) - - it('should track refused request', (done) => { - fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 0 }) - - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).toHaveBeenCalled() - done() - }) - }) - - it('should not track client error', (done) => { - fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 400 }) - - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).not.toHaveBeenCalled() - done() - }) - }) - - it('should not track successful request', (done) => { - fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 200 }) - - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).not.toHaveBeenCalled() - done() - }) - }) - - it('should add a default error response text', (done) => { - fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, responseText: undefined }) - - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).toHaveBeenCalled() - const stack = (errorObservableSpy.calls.mostRecent().args[0] as RawError).stack - expect(stack).toEqual('Failed to load') - done() - }) - }) - - it('should truncate error response text', (done) => { - fetchStub(FAKE_URL).resolveWith({ - ...DEFAULT_REQUEST, - responseText: 'Lorem ipsum dolor sit amet orci aliquam.', - }) - - fetchStubManager.whenAllComplete(() => { - const stack = (errorObservableSpy.calls.mostRecent().args[0] as RawError).stack - expect(stack).toEqual('Lorem ipsum dolor sit amet orci ...') - done() - }) - }) - - describe('feature "remove-network-errors" enabled', () => { - beforeEach(() => { - enabledExperimentalFeatures.push('remove-network-errors') - }) - - it('should not track aborted requests ', (done) => { - fetchStub(FAKE_URL).abort() - - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).not.toHaveBeenCalled() - done() - }) - }) - }) -}) diff --git a/packages/core/src/domain/automaticErrorCollection.ts b/packages/core/src/domain/automaticErrorCollection.ts deleted file mode 100644 index ea2b2f1008..0000000000 --- a/packages/core/src/domain/automaticErrorCollection.ts +++ /dev/null @@ -1,151 +0,0 @@ -import { FetchCompleteContext, resetFetchProxy, startFetchProxy } from '../browser/fetchProxy' -import { resetXhrProxy, startXhrProxy, XhrCompleteContext } from '../browser/xhrProxy' -import { - ErrorSource, - formatUnknownError, - RawError, - toStackTraceString, - formatErrorMessage, - ErrorHandling, - createHandlingStack, -} from '../tools/error' -import { Observable } from '../tools/observable' -import { clocksNow } from '../tools/timeUtils' -import { jsonStringify, RequestType, find } from '../tools/utils' -import { Configuration } from './configuration' -import { callMonitored } from './internalMonitoring' -import { computeStackTrace, subscribe, unsubscribe, StackTrace } from './tracekit' - -export type ErrorObservable = Observable -let errorObservable: ErrorObservable - -export function startAutomaticErrorCollection(configuration: Configuration) { - if (!errorObservable) { - errorObservable = new Observable() - trackNetworkError(configuration, errorObservable) - startConsoleTracking(errorObservable) - startRuntimeErrorTracking(errorObservable) - } - return errorObservable -} - -let originalConsoleError: (...params: unknown[]) => void - -/* eslint-disable no-console */ -export function startConsoleTracking(errorObservable: ErrorObservable) { - originalConsoleError = console.error - - console.error = (...params: unknown[]) => { - const handlingStack = createHandlingStack() - callMonitored(() => { - originalConsoleError.apply(console, params) - errorObservable.notify({ - ...buildErrorFromParams(params, handlingStack), - source: ErrorSource.CONSOLE, - startClocks: clocksNow(), - handling: ErrorHandling.HANDLED, - }) - }) - } -} - -export function stopConsoleTracking() { - console.error = originalConsoleError -} -/* eslint-enable no-console */ - -function buildErrorFromParams(params: unknown[], handlingStack: string) { - const firstErrorParam = find(params, (param: unknown): param is Error => param instanceof Error) - - return { - message: ['console error:', ...params].map((param) => formatConsoleParameters(param)).join(' '), - stack: firstErrorParam ? toStackTraceString(computeStackTrace(firstErrorParam)) : undefined, - handlingStack, - } -} - -function formatConsoleParameters(param: unknown) { - if (typeof param === 'string') { - return param - } - if (param instanceof Error) { - return formatErrorMessage(computeStackTrace(param)) - } - return jsonStringify(param, undefined, 2) -} - -let traceKitReportHandler: (stack: StackTrace, isWindowError: boolean, errorObject?: any) => void - -export function startRuntimeErrorTracking(errorObservable: ErrorObservable) { - traceKitReportHandler = (stackTrace: StackTrace, _: boolean, errorObject?: any) => { - const { stack, message, type } = formatUnknownError(stackTrace, errorObject, 'Uncaught') - errorObservable.notify({ - message, - stack, - type, - source: ErrorSource.SOURCE, - startClocks: clocksNow(), - originalError: errorObject, - handling: ErrorHandling.UNHANDLED, - }) - } - subscribe(traceKitReportHandler) -} - -export function stopRuntimeErrorTracking() { - unsubscribe(traceKitReportHandler) -} - -export function trackNetworkError(configuration: Configuration, errorObservable: ErrorObservable) { - startXhrProxy().onRequestComplete((context) => handleCompleteRequest(RequestType.XHR, context)) - startFetchProxy().onRequestComplete((context) => handleCompleteRequest(RequestType.FETCH, context)) - - function handleCompleteRequest(type: RequestType, request: XhrCompleteContext | FetchCompleteContext) { - if ( - !configuration.isIntakeUrl(request.url) && - (!configuration.isEnabled('remove-network-errors') || !request.isAborted) && - (isRejected(request) || isServerError(request)) - ) { - errorObservable.notify({ - message: `${format(type)} error ${request.method} ${request.url}`, - resource: { - method: request.method, - statusCode: request.status, - url: request.url, - }, - source: ErrorSource.NETWORK, - stack: truncateResponseText(request.responseText, configuration) || 'Failed to load', - startClocks: request.startClocks, - }) - } - } - - return { - stop: () => { - resetXhrProxy() - resetFetchProxy() - }, - } -} - -function isRejected(request: { status: number; responseType?: string }) { - return request.status === 0 && request.responseType !== 'opaque' -} - -function isServerError(request: { status: number }) { - return request.status >= 500 -} - -function truncateResponseText(responseText: string | undefined, configuration: Configuration) { - if (responseText && responseText.length > configuration.requestErrorResponseLengthLimit) { - return `${responseText.substring(0, configuration.requestErrorResponseLengthLimit)}...` - } - return responseText -} - -function format(type: RequestType) { - if (RequestType.XHR === type) { - return 'XHR' - } - return 'Fetch' -} diff --git a/packages/core/src/domain/error/automaticErrorCollection.ts b/packages/core/src/domain/error/automaticErrorCollection.ts new file mode 100644 index 0000000000..42dd60f272 --- /dev/null +++ b/packages/core/src/domain/error/automaticErrorCollection.ts @@ -0,0 +1,19 @@ +import { RawError } from '../../tools/error' +import { Observable } from '../../tools/observable' +import { Configuration } from '../configuration' +import { trackConsoleError } from './trackConsoleError' +import { trackRuntimeError } from './trackRuntimeError' +import { trackNetworkError } from './trackNetworkError' + +export type ErrorObservable = Observable +let errorObservable: ErrorObservable + +export function startAutomaticErrorCollection(configuration: Configuration) { + if (!errorObservable) { + errorObservable = new Observable() + trackNetworkError(configuration, errorObservable) + trackConsoleError(errorObservable) + trackRuntimeError(errorObservable) + } + return errorObservable +} diff --git a/packages/core/src/domain/error/trackConsoleError.spec.ts b/packages/core/src/domain/error/trackConsoleError.spec.ts new file mode 100644 index 0000000000..f9537b3bb8 --- /dev/null +++ b/packages/core/src/domain/error/trackConsoleError.spec.ts @@ -0,0 +1,79 @@ +import { isIE } from '../../../test/specHelper' +import { ErrorHandling, ErrorSource, RawError } from '../../tools/error' +import { Observable } from '../../tools/observable' +import { trackConsoleError } from './trackConsoleError' + +/* eslint-disable no-console */ +describe('console tracker', () => { + let consoleErrorStub: jasmine.Spy + let notifyError: jasmine.Spy + let stopConsoleErrorTracking: () => void + const CONSOLE_CONTEXT = { + source: ErrorSource.CONSOLE, + } + + beforeEach(() => { + consoleErrorStub = spyOn(console, 'error') + notifyError = jasmine.createSpy('notifyError') + const errorObservable = new Observable() + errorObservable.subscribe(notifyError) + ;({ stop: stopConsoleErrorTracking } = trackConsoleError(errorObservable)) + }) + + afterEach(() => { + stopConsoleErrorTracking() + }) + + it('should keep original behavior', () => { + console.error('foo', 'bar') + expect(consoleErrorStub).toHaveBeenCalledWith('foo', 'bar') + }) + + it('should notify error', () => { + console.error('foo', 'bar') + expect(notifyError).toHaveBeenCalledWith({ + ...CONSOLE_CONTEXT, + message: 'console error: foo bar', + stack: undefined, + handlingStack: jasmine.any(String), + startClocks: jasmine.any(Object), + handling: ErrorHandling.HANDLED, + }) + }) + + it('should generate a handling stack', () => { + function triggerError() { + console.error('foo', 'bar') + } + triggerError() + const rawError = notifyError.calls.mostRecent().args[0] as RawError + expect(rawError.handlingStack).toMatch(/^Error:\s+at triggerError (.|\n)*$/) + }) + + it('should stringify object parameters', () => { + console.error('Hello', { foo: 'bar' }) + expect(notifyError).toHaveBeenCalledWith({ + ...CONSOLE_CONTEXT, + message: 'console error: Hello {\n "foo": "bar"\n}', + stack: undefined, + handlingStack: jasmine.any(String), + startClocks: jasmine.any(Object), + handling: ErrorHandling.HANDLED, + }) + }) + + it('should format error instance', () => { + console.error(new TypeError('hello')) + expect((notifyError.calls.mostRecent().args[0] as RawError).message).toBe('console error: TypeError: hello') + }) + + it('should extract stack from first error', () => { + console.error(new TypeError('foo'), new TypeError('bar')) + const stack = (notifyError.calls.mostRecent().args[0] as RawError).stack + if (!isIE()) { + expect(stack).toMatch(/^TypeError: foo\s+at/) + } else { + expect(stack).toContain('TypeError: foo') + } + }) +}) diff --git a/packages/core/src/domain/error/trackConsoleError.ts b/packages/core/src/domain/error/trackConsoleError.ts new file mode 100644 index 0000000000..e0140bdd15 --- /dev/null +++ b/packages/core/src/domain/error/trackConsoleError.ts @@ -0,0 +1,58 @@ +import { + ErrorSource, + toStackTraceString, + ErrorHandling, + createHandlingStack, + formatErrorMessage, +} from '../../tools/error' +import { clocksNow } from '../../tools/timeUtils' +import { find, jsonStringify } from '../../tools/utils' +import { callMonitored } from '../internalMonitoring' +import { computeStackTrace } from '../tracekit' +import { ErrorObservable } from '../../tools/observable' + +let originalConsoleError: (...params: unknown[]) => void + +/* eslint-disable no-console */ +export function trackConsoleError(errorObservable: ErrorObservable) { + originalConsoleError = console.error + + console.error = (...params: unknown[]) => { + const handlingStack = createHandlingStack() + callMonitored(() => { + originalConsoleError.apply(console, params) + errorObservable.notify({ + ...buildErrorFromParams(params, handlingStack), + source: ErrorSource.CONSOLE, + startClocks: clocksNow(), + handling: ErrorHandling.HANDLED, + }) + }) + } + + return { + stop: () => { + console.error = originalConsoleError + }, + } +} + +function buildErrorFromParams(params: unknown[], handlingStack: string) { + const firstErrorParam = find(params, (param: unknown): param is Error => param instanceof Error) + + return { + message: ['console error:', ...params].map((param) => formatConsoleParameters(param)).join(' '), + stack: firstErrorParam ? toStackTraceString(computeStackTrace(firstErrorParam)) : undefined, + handlingStack, + } +} + +function formatConsoleParameters(param: unknown) { + if (typeof param === 'string') { + return param + } + if (param instanceof Error) { + return formatErrorMessage(computeStackTrace(param)) + } + return jsonStringify(param, undefined, 2) +} diff --git a/packages/core/src/domain/error/trackNetworkError.spec.ts b/packages/core/src/domain/error/trackNetworkError.spec.ts new file mode 100644 index 0000000000..0e35d4d6c9 --- /dev/null +++ b/packages/core/src/domain/error/trackNetworkError.spec.ts @@ -0,0 +1,152 @@ +import { FetchStub, FetchStubManager, isIE, SPEC_ENDPOINTS, stubFetch } from '../../../test/specHelper' +import { RawError } from '../../tools/error' +import { Observable } from '../../tools/observable' +import { includes } from '../../tools/utils' +import { Configuration } from '../configuration' +import { trackNetworkError } from './trackNetworkError' + +describe('network error tracker', () => { + let errorObservableSpy: jasmine.Spy + let fetchStub: FetchStub + let fetchStubManager: FetchStubManager + let stopNetworkErrorTracking: () => void + let enabledExperimentalFeatures: string[] + const FAKE_URL = 'http://fake.com/' + const DEFAULT_REQUEST = { + duration: 10, + method: 'GET', + responseText: 'Server error', + startTime: 0, + status: 503, + url: FAKE_URL, + } + + beforeEach(() => { + if (isIE()) { + pending('no fetch support') + } + enabledExperimentalFeatures = [] + + const errorObservable = new Observable() + errorObservableSpy = spyOn(errorObservable, 'notify') + const configuration = { + requestErrorResponseLengthLimit: 32, + isEnabled(featureFlag: string) { + return includes(enabledExperimentalFeatures, featureFlag) + }, + ...SPEC_ENDPOINTS, + } + + fetchStubManager = stubFetch() + ;({ stop: stopNetworkErrorTracking } = trackNetworkError(configuration as Configuration, errorObservable)) + fetchStub = window.fetch as FetchStub + }) + + afterEach(() => { + fetchStubManager.reset() + stopNetworkErrorTracking() + }) + + it('should track server error', (done) => { + fetchStub(FAKE_URL).resolveWith(DEFAULT_REQUEST) + + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).toHaveBeenCalledWith({ + message: 'Fetch error GET http://fake.com/', + resource: { + method: 'GET', + statusCode: 503, + url: 'http://fake.com/', + }, + source: 'network', + stack: 'Server error', + startClocks: jasmine.any(Object), + }) + done() + }) + }) + + it('should not track intake error', (done) => { + fetchStub('https://logs-intake.com/v1/input/send?foo=bar').resolveWith(DEFAULT_REQUEST) + + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).not.toHaveBeenCalled() + done() + }) + }) + + it('should track aborted requests ', (done) => { + fetchStub(FAKE_URL).abort() + + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).toHaveBeenCalled() + done() + }) + }) + + it('should track refused request', (done) => { + fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 0 }) + + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).toHaveBeenCalled() + done() + }) + }) + + it('should not track client error', (done) => { + fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 400 }) + + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).not.toHaveBeenCalled() + done() + }) + }) + + it('should not track successful request', (done) => { + fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 200 }) + + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).not.toHaveBeenCalled() + done() + }) + }) + + it('should add a default error response text', (done) => { + fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, responseText: undefined }) + + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).toHaveBeenCalled() + const stack = (errorObservableSpy.calls.mostRecent().args[0] as RawError).stack + expect(stack).toEqual('Failed to load') + done() + }) + }) + + it('should truncate error response text', (done) => { + fetchStub(FAKE_URL).resolveWith({ + ...DEFAULT_REQUEST, + responseText: 'Lorem ipsum dolor sit amet orci aliquam.', + }) + + fetchStubManager.whenAllComplete(() => { + const stack = (errorObservableSpy.calls.mostRecent().args[0] as RawError).stack + expect(stack).toEqual('Lorem ipsum dolor sit amet orci ...') + done() + }) + }) + + describe('feature "remove-network-errors" enabled', () => { + beforeEach(() => { + enabledExperimentalFeatures.push('remove-network-errors') + }) + + it('should not track aborted requests ', (done) => { + fetchStub(FAKE_URL).abort() + + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).not.toHaveBeenCalled() + done() + }) + }) + }) +}) diff --git a/packages/core/src/domain/error/trackNetworkError.ts b/packages/core/src/domain/error/trackNetworkError.ts new file mode 100644 index 0000000000..6c32ba5c74 --- /dev/null +++ b/packages/core/src/domain/error/trackNetworkError.ts @@ -0,0 +1,60 @@ +import { FetchCompleteContext, resetFetchProxy, startFetchProxy } from '../../browser/fetchProxy' +import { resetXhrProxy, startXhrProxy, XhrCompleteContext } from '../../browser/xhrProxy' +import { ErrorSource } from '../../tools/error' +import { ErrorObservable } from '../../tools/observable' +import { RequestType } from '../../tools/utils' +import { Configuration } from '../configuration' + +export function trackNetworkError(configuration: Configuration, errorObservable: ErrorObservable) { + startXhrProxy().onRequestComplete((context) => handleCompleteRequest(RequestType.XHR, context)) + startFetchProxy().onRequestComplete((context) => handleCompleteRequest(RequestType.FETCH, context)) + + function handleCompleteRequest(type: RequestType, request: XhrCompleteContext | FetchCompleteContext) { + if ( + !configuration.isIntakeUrl(request.url) && + (!configuration.isEnabled('remove-network-errors') || !request.isAborted) && + (isRejected(request) || isServerError(request)) + ) { + errorObservable.notify({ + message: `${format(type)} error ${request.method} ${request.url}`, + resource: { + method: request.method, + statusCode: request.status, + url: request.url, + }, + source: ErrorSource.NETWORK, + stack: truncateResponseText(request.responseText, configuration) || 'Failed to load', + startClocks: request.startClocks, + }) + } + } + + return { + stop: () => { + resetXhrProxy() + resetFetchProxy() + }, + } +} + +function isRejected(request: { status: number; responseType?: string }) { + return request.status === 0 && request.responseType !== 'opaque' +} + +function isServerError(request: { status: number }) { + return request.status >= 500 +} + +function truncateResponseText(responseText: string | undefined, configuration: Configuration) { + if (responseText && responseText.length > configuration.requestErrorResponseLengthLimit) { + return `${responseText.substring(0, configuration.requestErrorResponseLengthLimit)}...` + } + return responseText +} + +function format(type: RequestType) { + if (RequestType.XHR === type) { + return 'XHR' + } + return 'Fetch' +} diff --git a/packages/core/src/domain/error/trackRuntimeError.spec.ts b/packages/core/src/domain/error/trackRuntimeError.spec.ts new file mode 100644 index 0000000000..d6043170f1 --- /dev/null +++ b/packages/core/src/domain/error/trackRuntimeError.spec.ts @@ -0,0 +1,62 @@ +import { RawError } from '../../tools/error' +import { Observable } from '../../tools/observable' +import { trackRuntimeError } from './trackRuntimeError' + +describe('runtime error tracker', () => { + const ERROR_MESSAGE = 'foo' + let originalHandler: OnErrorEventHandler + let notifyError: jasmine.Spy + let onerrorSpy: jasmine.Spy + let stopRuntimeErrorTracking: () => void + + beforeEach(() => { + originalHandler = window.onerror + onerrorSpy = jasmine.createSpy() + window.onerror = onerrorSpy + + notifyError = jasmine.createSpy() + const errorObservable = new Observable() + errorObservable.subscribe((e: RawError) => notifyError(e) as void) + ;({ stop: stopRuntimeErrorTracking } = trackRuntimeError(errorObservable)) + }) + + afterEach(() => { + stopRuntimeErrorTracking() + window.onerror = originalHandler + }) + + it('should call original error handler', (done) => { + setTimeout(() => { + throw new Error(ERROR_MESSAGE) + }, 10) + + setTimeout(() => { + expect(onerrorSpy.calls.mostRecent().args[0]).toMatch(ERROR_MESSAGE) + done() + }, 100) + }) + + it('should notify error', (done) => { + setTimeout(() => { + throw new Error(ERROR_MESSAGE) + }, 10) + + setTimeout(() => { + expect((notifyError.calls.mostRecent().args[0] as RawError).message).toEqual(ERROR_MESSAGE) + done() + }, 100) + }) + + it('should handle direct onerror calls with objects', (done) => { + setTimeout(() => { + window.onerror!({ foo: 'bar' } as any) + }, 10) + + setTimeout(() => { + const collectedError = notifyError.calls.mostRecent().args[0] as RawError + expect(collectedError.message).toEqual('Uncaught {"foo":"bar"}') + expect(collectedError.stack).toEqual('No stack, consider using an instance of Error') + done() + }, 100) + }) +}) diff --git a/packages/core/src/domain/error/trackRuntimeError.ts b/packages/core/src/domain/error/trackRuntimeError.ts new file mode 100644 index 0000000000..7a51c310c0 --- /dev/null +++ b/packages/core/src/domain/error/trackRuntimeError.ts @@ -0,0 +1,27 @@ +import { ErrorSource, ErrorHandling, formatUnknownError } from '../../tools/error' +import { clocksNow } from '../../tools/timeUtils' +import { StackTrace, subscribe, unsubscribe } from '../tracekit' +import { ErrorObservable } from '../../tools/observable' + +let traceKitReportHandler: (stack: StackTrace, isWindowError: boolean, errorObject?: any) => void + +export function trackRuntimeError(errorObservable: ErrorObservable) { + traceKitReportHandler = (stackTrace: StackTrace, _: boolean, errorObject?: any) => { + const { stack, message, type } = formatUnknownError(stackTrace, errorObject, 'Uncaught') + errorObservable.notify({ + message, + stack, + type, + source: ErrorSource.SOURCE, + startClocks: clocksNow(), + originalError: errorObject, + handling: ErrorHandling.UNHANDLED, + }) + } + subscribe(traceKitReportHandler) + return { + stop: () => { + unsubscribe(traceKitReportHandler) + }, + } +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 1b0ea5e703..cf486191f5 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -6,7 +6,7 @@ export { buildCookieOptions, BeforeSendCallback, } from './domain/configuration' -export { startAutomaticErrorCollection, ErrorObservable } from './domain/automaticErrorCollection' +export { startAutomaticErrorCollection } from './domain/error/automaticErrorCollection' export { computeStackTrace, StackTrace } from './domain/tracekit' export { BuildEnv, @@ -27,7 +27,7 @@ export { addErrorToMonitoringBatch, setDebugMode, } from './domain/internalMonitoring' -export { Observable, Subscription } from './tools/observable' +export { Observable, ErrorObservable, Subscription } from './tools/observable' export { startSessionManagement, SESSION_TIME_OUT_DELAY, diff --git a/packages/core/src/tools/observable.ts b/packages/core/src/tools/observable.ts index 1609620204..fa924e9947 100644 --- a/packages/core/src/tools/observable.ts +++ b/packages/core/src/tools/observable.ts @@ -1,3 +1,5 @@ +import { RawError } from './error' + export interface Subscription { unsubscribe: () => void } @@ -18,3 +20,5 @@ export class Observable { this.observers.forEach((observer) => observer(data)) } } + +export type ErrorObservable = Observable From 8dddc3e94cdd57faad19466f2b23f50b4579f92e Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Wed, 7 Jul 2021 16:10:16 +0200 Subject: [PATCH 2/4] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Use=20core=20error=20t?= =?UTF-8?q?rackers=20in=20RUM=20and=20Logs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../domain/error/automaticErrorCollection.ts | 19 ------------------- packages/core/src/index.ts | 4 +++- packages/logs/src/boot/startLogs.ts | 16 +++++++++++----- .../error/errorCollection.ts | 14 ++++++++++---- 4 files changed, 24 insertions(+), 29 deletions(-) delete mode 100644 packages/core/src/domain/error/automaticErrorCollection.ts diff --git a/packages/core/src/domain/error/automaticErrorCollection.ts b/packages/core/src/domain/error/automaticErrorCollection.ts deleted file mode 100644 index 42dd60f272..0000000000 --- a/packages/core/src/domain/error/automaticErrorCollection.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { RawError } from '../../tools/error' -import { Observable } from '../../tools/observable' -import { Configuration } from '../configuration' -import { trackConsoleError } from './trackConsoleError' -import { trackRuntimeError } from './trackRuntimeError' -import { trackNetworkError } from './trackNetworkError' - -export type ErrorObservable = Observable -let errorObservable: ErrorObservable - -export function startAutomaticErrorCollection(configuration: Configuration) { - if (!errorObservable) { - errorObservable = new Observable() - trackNetworkError(configuration, errorObservable) - trackConsoleError(errorObservable) - trackRuntimeError(errorObservable) - } - return errorObservable -} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index cf486191f5..023e3b592b 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -6,7 +6,9 @@ export { buildCookieOptions, BeforeSendCallback, } from './domain/configuration' -export { startAutomaticErrorCollection } from './domain/error/automaticErrorCollection' +export { trackConsoleError } from './domain/error/trackConsoleError' +export { trackNetworkError } from './domain/error/trackNetworkError' +export { trackRuntimeError } from './domain/error/trackRuntimeError' export { computeStackTrace, StackTrace } from './domain/tracekit' export { BuildEnv, diff --git a/packages/logs/src/boot/startLogs.ts b/packages/logs/src/boot/startLogs.ts index 90391293e9..22e96721e4 100644 --- a/packages/logs/src/boot/startLogs.ts +++ b/packages/logs/src/boot/startLogs.ts @@ -12,8 +12,10 @@ import { Observable, RawError, RelativeTime, - startAutomaticErrorCollection, InitConfiguration, + trackNetworkError, + trackRuntimeError, + trackConsoleError, } from '@datadog/browser-core' import { Logger, LogsMessage, StatusType } from '../domain/logger' import { LoggerSession, startLoggerSession } from '../domain/loggerSession' @@ -37,10 +39,14 @@ export function startLogs( getGlobalContext: () => Context ) { const { configuration, internalMonitoring } = commonInit(initConfiguration, buildEnv) - const errorObservable = - initConfiguration.forwardErrorsToLogs !== false - ? startAutomaticErrorCollection(configuration) - : new Observable() + const errorObservable = new Observable() + + if (initConfiguration.forwardErrorsToLogs !== false) { + trackConsoleError(errorObservable) + trackRuntimeError(errorObservable) + trackNetworkError(configuration, errorObservable) + } + const session = startLoggerSession(configuration, areCookiesAuthorized(configuration.cookieOptions)) return doStartLogs(configuration, errorObservable, internalMonitoring, session, errorLogger, getGlobalContext) } diff --git a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts index 92eb1318b9..60ec62c204 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts @@ -4,10 +4,13 @@ import { Context, formatUnknownError, RawError, - startAutomaticErrorCollection, ClocksState, generateUUID, ErrorHandling, + Observable, + trackConsoleError, + trackRuntimeError, + trackNetworkError, } from '@datadog/browser-core' import { CommonContext, RawRumErrorEvent, RumEventType } from '../../../rawRumEvent.types' import { LifeCycle, LifeCycleEventType, RawRumEventCollectedData } from '../../lifeCycle' @@ -28,9 +31,12 @@ export function startErrorCollection( configuration: Configuration, foregroundContexts: ForegroundContexts ) { - startAutomaticErrorCollection(configuration).subscribe((error) => - lifeCycle.notify(LifeCycleEventType.RAW_ERROR_COLLECTED, { error }) - ) + const errorObservable = new Observable() + trackConsoleError(errorObservable) + trackRuntimeError(errorObservable) + trackNetworkError(configuration, errorObservable) // deprecated: to remove with version 3 + + errorObservable.subscribe((error) => lifeCycle.notify(LifeCycleEventType.RAW_ERROR_COLLECTED, { error })) return doStartErrorCollection(lifeCycle, foregroundContexts) } From f4b5430646142a5b607d01d34698f5c7906349d9 Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Thu, 8 Jul 2021 17:56:05 +0200 Subject: [PATCH 3/4] =?UTF-8?q?=E2=9C=A8=20Remove=20RUM=20network=20errors?= =?UTF-8?q?=20under=20feature=20flag?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../domain/error/trackNetworkError.spec.ts | 154 +++++++++--------- .../src/domain/error/trackNetworkError.ts | 8 +- packages/logs/src/boot/startLogs.ts | 2 +- .../error/errorCollection.ts | 4 +- 4 files changed, 87 insertions(+), 81 deletions(-) diff --git a/packages/core/src/domain/error/trackNetworkError.spec.ts b/packages/core/src/domain/error/trackNetworkError.spec.ts index 0e35d4d6c9..76790aeecc 100644 --- a/packages/core/src/domain/error/trackNetworkError.spec.ts +++ b/packages/core/src/domain/error/trackNetworkError.spec.ts @@ -1,7 +1,6 @@ import { FetchStub, FetchStubManager, isIE, SPEC_ENDPOINTS, stubFetch } from '../../../test/specHelper' import { RawError } from '../../tools/error' import { Observable } from '../../tools/observable' -import { includes } from '../../tools/utils' import { Configuration } from '../configuration' import { trackNetworkError } from './trackNetworkError' @@ -10,7 +9,8 @@ describe('network error tracker', () => { let fetchStub: FetchStub let fetchStubManager: FetchStubManager let stopNetworkErrorTracking: () => void - let enabledExperimentalFeatures: string[] + let configuration: Configuration + let errorObservable: Observable const FAKE_URL = 'http://fake.com/' const DEFAULT_REQUEST = { duration: 10, @@ -25,21 +25,14 @@ describe('network error tracker', () => { if (isIE()) { pending('no fetch support') } - enabledExperimentalFeatures = [] - - const errorObservable = new Observable() + errorObservable = new Observable() errorObservableSpy = spyOn(errorObservable, 'notify') - const configuration = { + configuration = { requestErrorResponseLengthLimit: 32, - isEnabled(featureFlag: string) { - return includes(enabledExperimentalFeatures, featureFlag) - }, ...SPEC_ENDPOINTS, - } + } as Configuration fetchStubManager = stubFetch() - ;({ stop: stopNetworkErrorTracking } = trackNetworkError(configuration as Configuration, errorObservable)) - fetchStub = window.fetch as FetchStub }) afterEach(() => { @@ -47,97 +40,104 @@ describe('network error tracker', () => { stopNetworkErrorTracking() }) - it('should track server error', (done) => { - fetchStub(FAKE_URL).resolveWith(DEFAULT_REQUEST) - - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).toHaveBeenCalledWith({ - message: 'Fetch error GET http://fake.com/', - resource: { - method: 'GET', - statusCode: 503, - url: 'http://fake.com/', - }, - source: 'network', - stack: 'Server error', - startClocks: jasmine.any(Object), + describe('when default configuration', () => { + beforeEach(() => { + ;({ stop: stopNetworkErrorTracking } = trackNetworkError(configuration, errorObservable)) + fetchStub = window.fetch as FetchStub + }) + it('should track server error', (done) => { + fetchStub(FAKE_URL).resolveWith(DEFAULT_REQUEST) + + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).toHaveBeenCalledWith({ + message: 'Fetch error GET http://fake.com/', + resource: { + method: 'GET', + statusCode: 503, + url: 'http://fake.com/', + }, + source: 'network', + stack: 'Server error', + startClocks: jasmine.any(Object), + }) + done() }) - done() }) - }) - it('should not track intake error', (done) => { - fetchStub('https://logs-intake.com/v1/input/send?foo=bar').resolveWith(DEFAULT_REQUEST) + it('should not track intake error', (done) => { + fetchStub('https://logs-intake.com/v1/input/send?foo=bar').resolveWith(DEFAULT_REQUEST) - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).not.toHaveBeenCalled() - done() + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).not.toHaveBeenCalled() + done() + }) }) - }) - it('should track aborted requests ', (done) => { - fetchStub(FAKE_URL).abort() + it('should track aborted requests ', (done) => { + fetchStub(FAKE_URL).abort() - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).toHaveBeenCalled() - done() + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).toHaveBeenCalled() + done() + }) }) - }) - it('should track refused request', (done) => { - fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 0 }) + it('should track refused request', (done) => { + fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 0 }) - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).toHaveBeenCalled() - done() + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).toHaveBeenCalled() + done() + }) }) - }) - it('should not track client error', (done) => { - fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 400 }) + it('should not track client error', (done) => { + fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 400 }) - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).not.toHaveBeenCalled() - done() + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).not.toHaveBeenCalled() + done() + }) }) - }) - it('should not track successful request', (done) => { - fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 200 }) + it('should not track successful request', (done) => { + fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, status: 200 }) - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).not.toHaveBeenCalled() - done() + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).not.toHaveBeenCalled() + done() + }) }) - }) - it('should add a default error response text', (done) => { - fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, responseText: undefined }) + it('should add a default error response text', (done) => { + fetchStub(FAKE_URL).resolveWith({ ...DEFAULT_REQUEST, responseText: undefined }) - fetchStubManager.whenAllComplete(() => { - expect(errorObservableSpy).toHaveBeenCalled() - const stack = (errorObservableSpy.calls.mostRecent().args[0] as RawError).stack - expect(stack).toEqual('Failed to load') - done() + fetchStubManager.whenAllComplete(() => { + expect(errorObservableSpy).toHaveBeenCalled() + const stack = (errorObservableSpy.calls.mostRecent().args[0] as RawError).stack + expect(stack).toEqual('Failed to load') + done() + }) }) - }) - it('should truncate error response text', (done) => { - fetchStub(FAKE_URL).resolveWith({ - ...DEFAULT_REQUEST, - responseText: 'Lorem ipsum dolor sit amet orci aliquam.', - }) + it('should truncate error response text', (done) => { + fetchStub(FAKE_URL).resolveWith({ + ...DEFAULT_REQUEST, + responseText: 'Lorem ipsum dolor sit amet orci aliquam.', + }) - fetchStubManager.whenAllComplete(() => { - const stack = (errorObservableSpy.calls.mostRecent().args[0] as RawError).stack - expect(stack).toEqual('Lorem ipsum dolor sit amet orci ...') - done() + fetchStubManager.whenAllComplete(() => { + const stack = (errorObservableSpy.calls.mostRecent().args[0] as RawError).stack + expect(stack).toEqual('Lorem ipsum dolor sit amet orci ...') + done() + }) }) }) - describe('feature "remove-network-errors" enabled', () => { + describe('when trackAbortedRequests param is true', () => { beforeEach(() => { - enabledExperimentalFeatures.push('remove-network-errors') + ;({ stop: stopNetworkErrorTracking } = trackNetworkError(configuration, errorObservable, false)) + fetchStub = window.fetch as FetchStub }) it('should not track aborted requests ', (done) => { diff --git a/packages/core/src/domain/error/trackNetworkError.ts b/packages/core/src/domain/error/trackNetworkError.ts index 6c32ba5c74..7e06d7ba85 100644 --- a/packages/core/src/domain/error/trackNetworkError.ts +++ b/packages/core/src/domain/error/trackNetworkError.ts @@ -5,14 +5,18 @@ import { ErrorObservable } from '../../tools/observable' import { RequestType } from '../../tools/utils' import { Configuration } from '../configuration' -export function trackNetworkError(configuration: Configuration, errorObservable: ErrorObservable) { +export function trackNetworkError( + configuration: Configuration, + errorObservable: ErrorObservable, + trackAbortedRequests = true +) { startXhrProxy().onRequestComplete((context) => handleCompleteRequest(RequestType.XHR, context)) startFetchProxy().onRequestComplete((context) => handleCompleteRequest(RequestType.FETCH, context)) function handleCompleteRequest(type: RequestType, request: XhrCompleteContext | FetchCompleteContext) { if ( !configuration.isIntakeUrl(request.url) && - (!configuration.isEnabled('remove-network-errors') || !request.isAborted) && + (trackAbortedRequests || !request.isAborted) && (isRejected(request) || isServerError(request)) ) { errorObservable.notify({ diff --git a/packages/logs/src/boot/startLogs.ts b/packages/logs/src/boot/startLogs.ts index 22e96721e4..41c05de986 100644 --- a/packages/logs/src/boot/startLogs.ts +++ b/packages/logs/src/boot/startLogs.ts @@ -44,7 +44,7 @@ export function startLogs( if (initConfiguration.forwardErrorsToLogs !== false) { trackConsoleError(errorObservable) trackRuntimeError(errorObservable) - trackNetworkError(configuration, errorObservable) + trackNetworkError(configuration, errorObservable, configuration.isEnabled('remove-network-errors')) } const session = startLoggerSession(configuration, areCookiesAuthorized(configuration.cookieOptions)) diff --git a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts index 60ec62c204..4253302cc2 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts @@ -34,7 +34,9 @@ export function startErrorCollection( const errorObservable = new Observable() trackConsoleError(errorObservable) trackRuntimeError(errorObservable) - trackNetworkError(configuration, errorObservable) // deprecated: to remove with version 3 + if (!configuration.isEnabled('remove-network-errors')) { + trackNetworkError(configuration, errorObservable) // deprecated: to remove with version 3 + } errorObservable.subscribe((error) => lifeCycle.notify(LifeCycleEventType.RAW_ERROR_COLLECTED, { error })) From 1255f34f028cb68468b8e8eb604583e2f67a688f Mon Sep 17 00:00:00 2001 From: Aymeric Mortemousque Date: Fri, 9 Jul 2021 14:59:01 +0200 Subject: [PATCH 4/4] =?UTF-8?q?=F0=9F=91=8C=20Replace=20ErrorObservable=20?= =?UTF-8?q?by=20Observable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/domain/error/trackConsoleError.ts | 5 +++-- packages/core/src/domain/error/trackNetworkError.spec.ts | 2 +- packages/core/src/domain/error/trackNetworkError.ts | 6 +++--- packages/core/src/domain/error/trackRuntimeError.ts | 6 +++--- packages/core/src/index.ts | 2 +- packages/core/src/tools/observable.ts | 4 ---- packages/logs/src/boot/startLogs.spec.ts | 3 +-- packages/logs/src/boot/startLogs.ts | 3 +-- 8 files changed, 13 insertions(+), 18 deletions(-) diff --git a/packages/core/src/domain/error/trackConsoleError.ts b/packages/core/src/domain/error/trackConsoleError.ts index e0140bdd15..40a3fd17f0 100644 --- a/packages/core/src/domain/error/trackConsoleError.ts +++ b/packages/core/src/domain/error/trackConsoleError.ts @@ -4,17 +4,18 @@ import { ErrorHandling, createHandlingStack, formatErrorMessage, + RawError, } from '../../tools/error' +import { Observable } from '../../tools/observable' import { clocksNow } from '../../tools/timeUtils' import { find, jsonStringify } from '../../tools/utils' import { callMonitored } from '../internalMonitoring' import { computeStackTrace } from '../tracekit' -import { ErrorObservable } from '../../tools/observable' let originalConsoleError: (...params: unknown[]) => void /* eslint-disable no-console */ -export function trackConsoleError(errorObservable: ErrorObservable) { +export function trackConsoleError(errorObservable: Observable) { originalConsoleError = console.error console.error = (...params: unknown[]) => { diff --git a/packages/core/src/domain/error/trackNetworkError.spec.ts b/packages/core/src/domain/error/trackNetworkError.spec.ts index 76790aeecc..a1142fa560 100644 --- a/packages/core/src/domain/error/trackNetworkError.spec.ts +++ b/packages/core/src/domain/error/trackNetworkError.spec.ts @@ -134,7 +134,7 @@ describe('network error tracker', () => { }) }) - describe('when trackAbortedRequests param is true', () => { + describe('when trackAbortedRequests param is false', () => { beforeEach(() => { ;({ stop: stopNetworkErrorTracking } = trackNetworkError(configuration, errorObservable, false)) fetchStub = window.fetch as FetchStub diff --git a/packages/core/src/domain/error/trackNetworkError.ts b/packages/core/src/domain/error/trackNetworkError.ts index 7e06d7ba85..149f684e5f 100644 --- a/packages/core/src/domain/error/trackNetworkError.ts +++ b/packages/core/src/domain/error/trackNetworkError.ts @@ -1,13 +1,13 @@ import { FetchCompleteContext, resetFetchProxy, startFetchProxy } from '../../browser/fetchProxy' import { resetXhrProxy, startXhrProxy, XhrCompleteContext } from '../../browser/xhrProxy' -import { ErrorSource } from '../../tools/error' -import { ErrorObservable } from '../../tools/observable' +import { ErrorSource, RawError } from '../../tools/error' +import { Observable } from '../../tools/observable' import { RequestType } from '../../tools/utils' import { Configuration } from '../configuration' export function trackNetworkError( configuration: Configuration, - errorObservable: ErrorObservable, + errorObservable: Observable, trackAbortedRequests = true ) { startXhrProxy().onRequestComplete((context) => handleCompleteRequest(RequestType.XHR, context)) diff --git a/packages/core/src/domain/error/trackRuntimeError.ts b/packages/core/src/domain/error/trackRuntimeError.ts index 7a51c310c0..6d134eb5d5 100644 --- a/packages/core/src/domain/error/trackRuntimeError.ts +++ b/packages/core/src/domain/error/trackRuntimeError.ts @@ -1,11 +1,11 @@ -import { ErrorSource, ErrorHandling, formatUnknownError } from '../../tools/error' +import { ErrorSource, ErrorHandling, formatUnknownError, RawError } from '../../tools/error' +import { Observable } from '../../tools/observable' import { clocksNow } from '../../tools/timeUtils' import { StackTrace, subscribe, unsubscribe } from '../tracekit' -import { ErrorObservable } from '../../tools/observable' let traceKitReportHandler: (stack: StackTrace, isWindowError: boolean, errorObject?: any) => void -export function trackRuntimeError(errorObservable: ErrorObservable) { +export function trackRuntimeError(errorObservable: Observable) { traceKitReportHandler = (stackTrace: StackTrace, _: boolean, errorObject?: any) => { const { stack, message, type } = formatUnknownError(stackTrace, errorObject, 'Uncaught') errorObservable.notify({ diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 023e3b592b..493bd66816 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -29,7 +29,7 @@ export { addErrorToMonitoringBatch, setDebugMode, } from './domain/internalMonitoring' -export { Observable, ErrorObservable, Subscription } from './tools/observable' +export { Observable, Subscription } from './tools/observable' export { startSessionManagement, SESSION_TIME_OUT_DELAY, diff --git a/packages/core/src/tools/observable.ts b/packages/core/src/tools/observable.ts index fa924e9947..1609620204 100644 --- a/packages/core/src/tools/observable.ts +++ b/packages/core/src/tools/observable.ts @@ -1,5 +1,3 @@ -import { RawError } from './error' - export interface Subscription { unsubscribe: () => void } @@ -20,5 +18,3 @@ export class Observable { this.observers.forEach((observer) => observer(data)) } } - -export type ErrorObservable = Observable diff --git a/packages/logs/src/boot/startLogs.spec.ts b/packages/logs/src/boot/startLogs.spec.ts index afcc5ab638..fe8bf18236 100644 --- a/packages/logs/src/boot/startLogs.spec.ts +++ b/packages/logs/src/boot/startLogs.spec.ts @@ -2,7 +2,6 @@ import { Configuration, Context, DEFAULT_CONFIGURATION, - ErrorObservable, ErrorSource, noop, Observable, @@ -55,7 +54,7 @@ const DEFAULT_MESSAGE = { status: StatusType.info, message: 'message' } describe('logs', () => { let sessionIsTracked: boolean let server: sinon.SinonFakeServer - let errorObservable: ErrorObservable + let errorObservable: Observable const session = { getId: () => (sessionIsTracked ? SESSION_ID : undefined), isTracked: () => sessionIsTracked, diff --git a/packages/logs/src/boot/startLogs.ts b/packages/logs/src/boot/startLogs.ts index 41c05de986..989c42ce2b 100644 --- a/packages/logs/src/boot/startLogs.ts +++ b/packages/logs/src/boot/startLogs.ts @@ -6,7 +6,6 @@ import { Configuration, Context, createErrorFilter, - ErrorObservable, HttpRequest, InternalMonitoring, Observable, @@ -53,7 +52,7 @@ export function startLogs( export function doStartLogs( configuration: Configuration, - errorObservable: ErrorObservable, + errorObservable: Observable, internalMonitoring: InternalMonitoring, session: LoggerSession, errorLogger: Logger,