diff --git a/package-lock.json b/package-lock.json index 713fef3f6..bc6476da6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9687,9 +9687,9 @@ } }, "node_modules/caniuse-lite": { - "version": "1.0.30001514", - "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001514.tgz", - "integrity": "sha512-ENcIpYBmwAAOm/V2cXgM7rZUrKKaqisZl4ZAI520FIkqGXUxJjmaIssbRW5HVVR5tyV6ygTLIm15aU8LUmQSaQ==", + "version": "1.0.30001515", + "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001515.tgz", + "integrity": "sha512-eEFDwUOZbE24sb+Ecsx3+OvNETqjWIdabMy52oOkIgcUtAsQifjUG9q4U9dgTHJM2mfk4uEPxc0+xuFdJ629QA==", "dev": true, "funding": [ { @@ -34042,9 +34042,9 @@ } }, "caniuse-lite": { - "version": "1.0.30001514", - "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001514.tgz", - "integrity": "sha512-ENcIpYBmwAAOm/V2cXgM7rZUrKKaqisZl4ZAI520FIkqGXUxJjmaIssbRW5HVVR5tyV6ygTLIm15aU8LUmQSaQ==", + "version": "1.0.30001515", + "resolved": "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001515.tgz", + "integrity": "sha512-eEFDwUOZbE24sb+Ecsx3+OvNETqjWIdabMy52oOkIgcUtAsQifjUG9q4U9dgTHJM2mfk4uEPxc0+xuFdJ629QA==", "dev": true }, "capital-case": { diff --git a/src/features/jserrors/aggregate/compute-stack-trace.js b/src/features/jserrors/aggregate/compute-stack-trace.js index add0d4994..581b2565a 100644 --- a/src/features/jserrors/aggregate/compute-stack-trace.js +++ b/src/features/jserrors/aggregate/compute-stack-trace.js @@ -233,7 +233,7 @@ function computeStackTraceBySourceAndLine (ex) { mode: 'sourceline', name: className, message: ex.message, - stackString: getClassName(ex) + ': ' + ex.message + '\n in evaluated code', + stackString: className + ': ' + ex.message + '\n in evaluated code', frames: [{ func: 'evaluated code' }] diff --git a/src/features/jserrors/instrument/debug.js b/src/features/jserrors/instrument/debug.js deleted file mode 100644 index c3f57f520..000000000 --- a/src/features/jserrors/instrument/debug.js +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright 2020 New Relic Corporation. All rights reserved. - * SPDX-License-Identifier: Apache-2.0 - */ - -import { ee } from '../../../common/event-emitter/contextual-ee' -import { mapOwn } from '../../../common/util/map-own' - -var flags = {} -var flagArr - -try { - flagArr = localStorage.getItem('__nr_flags').split(',') - if (console && typeof console.log === 'function') { - flags.console = true - if (flagArr.indexOf('dev') !== -1) flags.dev = true - if (flagArr.indexOf('nr_dev') !== -1) flags.nrDev = true - } -} catch (err) { - // no op -} - -if (flags.nrDev) ee.on('internal-error', function (err) { log(err.stack) }) -if (flags.dev) ee.on('fn-err', function (args, origThis, err) { log(err.stack) }) -if (flags.dev) { - log('NR AGENT IN DEVELOPMENT MODE') - log('flags: ' + mapOwn(flags, function (key, val) { return key }).join(', ')) -} - -function log (message) { - try { - if (flags.console) log(message) - } catch (err) { - // no op - } -} diff --git a/src/features/jserrors/instrument/index.js b/src/features/jserrors/instrument/index.js index 05a104209..900f97be7 100644 --- a/src/features/jserrors/instrument/index.js +++ b/src/features/jserrors/instrument/index.js @@ -5,64 +5,55 @@ import { handle } from '../../../common/event-emitter/handle' import { now } from '../../../common/timing/now' -import { getOrSet } from '../../../common/util/get-or-set' -import { wrapRaf, wrapTimer, wrapEvents, wrapXhr } from '../../../common/wrap' -import './debug' import { InstrumentBase } from '../../utils/instrument-base' -import { FEATURE_NAME, NR_ERR_PROP } from '../constants' +import { FEATURE_NAME } from '../constants' import { FEATURE_NAMES } from '../../../loaders/features/features' import { globalScope } from '../../../common/constants/runtime' import { eventListenerOpts } from '../../../common/event-listener/event-listener-opts' -import { getRuntime } from '../../../common/config/config' import { stringify } from '../../../common/util/stringify' +import { UncaughtError } from './uncaught-error' export class Instrument extends InstrumentBase { static featureName = FEATURE_NAME + + #seenErrors = new Set() + constructor (agentIdentifier, aggregator, auto = true) { super(agentIdentifier, aggregator, FEATURE_NAME, auto) - // skipNext counter to keep track of uncaught - // errors that will be the same as caught errors. - this.skipNext = 0 + try { // this try-catch can be removed when IE11 is completely unsupported & gone this.removeOnAbort = new AbortController() } catch (e) {} - const thisInstrument = this - thisInstrument.ee.on('fn-start', function (args, obj, methodName) { - if (thisInstrument.abortHandler) thisInstrument.skipNext += 1 - }) - thisInstrument.ee.on('fn-err', function (args, obj, err) { - if (thisInstrument.abortHandler && !err[NR_ERR_PROP]) { - getOrSet(err, NR_ERR_PROP, function getVal () { - return true - }) - this.thrown = true - handle('err', [err, now()], undefined, FEATURE_NAMES.jserrors, thisInstrument.ee) - } - }) - thisInstrument.ee.on('fn-end', function () { - if (!thisInstrument.abortHandler) return - if (!this.thrown && thisInstrument.skipNext > 0) thisInstrument.skipNext -= 1 + // Capture function errors early in case the spa feature is loaded + this.ee.on('fn-err', (args, obj, error) => { + if (!this.abortHandler || this.#seenErrors.has(error)) return + this.#seenErrors.add(error) + + handle('err', [this.#castError(error), now()], undefined, FEATURE_NAMES.jserrors, this.ee) }) - thisInstrument.ee.on('internal-error', function (e) { - handle('ierr', [e, now(), true], undefined, FEATURE_NAMES.jserrors, thisInstrument.ee) + + this.ee.on('internal-error', (error) => { + if (!this.abortHandler) return + handle('ierr', [this.#castError(error), now(), true], undefined, FEATURE_NAMES.jserrors, this.ee) }) - // Replace global error handler with our own. - this.origOnerror = globalScope.onerror - globalScope.onerror = this.onerrorHandler.bind(this) + globalScope.addEventListener('unhandledrejection', (promiseRejectionEvent) => { + if (!this.abortHandler) return - globalScope.addEventListener('unhandledrejection', (e) => { - /** rejections can contain data of any type -- this is an effort to keep the message human readable */ - const err = castReasonToError(e.reason) - handle('err', [err, now(), false, { unhandledPromiseRejection: 1 }], undefined, FEATURE_NAMES.jserrors, this.ee) + handle('err', [this.#castPromiseRejectionEvent(promiseRejectionEvent), now(), false, { unhandledPromiseRejection: 1 }], undefined, FEATURE_NAMES.jserrors, this.ee) }, eventListenerOpts(false, this.removeOnAbort?.signal)) - wrapRaf(this.ee) - wrapTimer(this.ee) - wrapEvents(this.ee) - if (getRuntime(agentIdentifier).xhrWrappable) wrapXhr(this.ee) + globalScope.addEventListener('error', (errorEvent) => { + if (!this.abortHandler) return + if (this.#seenErrors.has(errorEvent.error)) { + this.#seenErrors.delete(errorEvent.error) + return + } + + handle('err', [this.#castErrorEvent(errorEvent), now()], undefined, FEATURE_NAMES.jserrors, this.ee) + }, eventListenerOpts(false, this.removeOnAbort?.signal)) this.abortHandler = this.#abort // we also use this as a flag to denote that the feature is active or on and handling errors this.importAggregator() @@ -71,67 +62,55 @@ export class Instrument extends InstrumentBase { /** Restoration and resource release tasks to be done if JS error loader is being aborted. Unwind changes to globals. */ #abort () { this.removeOnAbort?.abort() + this.#seenErrors.clear() this.abortHandler = undefined // weakly allow this abort op to run only once } + #castError (error) { + if (error instanceof Error) { + return error + } + + if (typeof error.message !== 'undefined') { + return new UncaughtError(error.message, error.filename, error.lineno, error.colno) + } + + return new UncaughtError(error) + } + /** - * FF and Android browsers do not provide error info to the 'error' event callback, - * so we must use window.onerror - * @param {string} message - * @param {string} filename - * @param {number} lineno - * @param {number} column - * @param {Error | *} errorObj - * @returns + * Attempts to convert a PromiseRejectionEvent object to an Error object + * @param {PromiseRejectionEvent} unhandledRejectionEvent The unhandled promise rejection event + * @returns {Error} An Error object with the message as the casted reason */ - onerrorHandler (message, filename, lineno, column, errorObj) { - if (typeof this.origOnerror === 'function') this.origOnerror(...arguments) - - try { - if (this.skipNext) this.skipNext -= 1 - else handle('err', [errorObj || new UncaughtException(message, filename, lineno), now()], undefined, FEATURE_NAMES.jserrors, this.ee) - } catch (e) { + #castPromiseRejectionEvent (promiseRejectionEvent) { + let prefix = 'Unhandled Promise Rejection: ' + if (promiseRejectionEvent?.reason instanceof Error) { try { - handle('ierr', [e, now(), true], undefined, FEATURE_NAMES.jserrors, this.ee) - } catch (err) { - // do nothing + promiseRejectionEvent.reason.message = prefix + promiseRejectionEvent.reason.message + return promiseRejectionEvent.reason + } catch (e) { + return promiseRejectionEvent.reason } } - return false // maintain default behavior of the error event of Window - } -} - -/** - * - * @param {string} message - * @param {string} filename - * @param {number} lineno - */ -function UncaughtException (message, filename, lineno) { - this.message = message || 'Uncaught error with no additional information' - this.sourceURL = filename - this.line = lineno -} - -/** - * Attempts to cast an unhandledPromiseRejection reason (reject(...)) to an Error object - * @param {*} reason - The reason property from an unhandled promise rejection - * @returns {Error} - An Error object with the message as the casted reason - */ -function castReasonToError (reason) { - let prefix = 'Unhandled Promise Rejection: ' - if (reason instanceof Error) { + if (typeof promiseRejectionEvent.reason === 'undefined') return new Error(prefix) try { - reason.message = prefix + reason.message - return reason + return new Error(prefix + stringify(promiseRejectionEvent.reason)) } catch (e) { - return reason + return new Error(promiseRejectionEvent.reason) } } - if (typeof reason === 'undefined') return new Error(prefix) - try { - return new Error(prefix + stringify(reason)) - } catch (err) { - return new Error(prefix) + + /** + * Attempts to convert an ErrorEvent object to an Error object + * @param {ErrorEvent} errorEvent The error event + * @returns {Error|UncaughtError} The error event converted to an Error object + */ + #castErrorEvent (errorEvent) { + if (errorEvent.error instanceof Error) { + return errorEvent.error + } + + return new UncaughtError(errorEvent.message, errorEvent.filename, errorEvent.lineno, errorEvent.colno) } } diff --git a/src/features/jserrors/instrument/uncaught-error.js b/src/features/jserrors/instrument/uncaught-error.js new file mode 100644 index 000000000..8d3c79714 --- /dev/null +++ b/src/features/jserrors/instrument/uncaught-error.js @@ -0,0 +1,15 @@ +/** + * Represents an uncaught non Error type error. This class does + * not extend the Error class to prevent an invalid stack trace + * from being created. Use this class to cast thrown errors that + * do not use the Error class (strings, etc) to an object. + */ +export class UncaughtError { + constructor (message, filename, lineno, colno) { + this.name = 'UncaughtError' + this.message = message + this.sourceURL = filename + this.line = lineno + this.column = colno + } +} diff --git a/src/loaders/api/api.js b/src/loaders/api/api.js index b62744410..a4772386f 100644 --- a/src/loaders/api/api.js +++ b/src/loaders/api/api.js @@ -127,7 +127,7 @@ export function setAPI (agentIdentifier, forceDrain) { try { return cb.apply(this, arguments) } catch (err) { - tracerEE.emit('fn-err', [arguments, this, typeof err == 'string' ? new Error(err) : err], contextStore) + tracerEE.emit('fn-err', [arguments, this, err], contextStore) // the error came from outside the agent, so don't swallow throw err } finally { diff --git a/tests/functional/err/uncaught.test.js b/tests/functional/err/uncaught.test.js index 2e845cf0f..7021b4589 100644 --- a/tests/functional/err/uncaught.test.js +++ b/tests/functional/err/uncaught.test.js @@ -30,8 +30,7 @@ testDriver.test('reporting uncaught errors', supported, function (t, browser, ro const expectedErrorMessages = [ { message: 'original onerror', tested: false }, { message: 'uncaught error', tested: false }, - { message: 'fake', tested: false }, - { message: 'original return false', tested: false } + { message: 'original return abc', tested: false } ] actualErrors.forEach(err => { const targetError = expectedErrorMessages.find(x => x.message === err.params.message) @@ -43,7 +42,7 @@ testDriver.test('reporting uncaught errors', supported, function (t, browser, ro if (err.params.message === 'fake') t.ok(err.params.exceptionClass !== 'Error', `fake error has correct exceptionClass (${err.params.exceptionClass})`) else t.ok(err.params.exceptionClass === 'Error', `error has correct exceptionClass (${err.params.exceptionClass})`) }) - t.ok(expectedErrorMessages.every(x => x.tested), 'All expected error messages were found') + t.ok(expectedErrorMessages.every(x => x.tested), `All expected error messages were found ${JSON.stringify(expectedErrorMessages.filter(x => !x.tested))}`) t.end() }).catch(fail) diff --git a/tests/functional/err/unhandled-rejection-readable.test.js b/tests/functional/err/unhandled-rejection-readable.test.js index b643de43d..ae64b92a3 100644 --- a/tests/functional/err/unhandled-rejection-readable.test.js +++ b/tests/functional/err/unhandled-rejection-readable.test.js @@ -49,7 +49,7 @@ testDriver.test('unhandledPromiseRejections are caught and are readable', suppor t.ok(!!err.params.stack_trace, 'stack_trace exists') t.ok(!!err.params.stackHash, 'stackHash exists') }) - t.ok(expectedErrorMessages.every(x => x.tested), `All expected error messages were found ${expectedErrorMessages.filter(x => !x.tested)}`) + t.ok(expectedErrorMessages.every(x => x.tested), `All expected error messages were found ${JSON.stringify(expectedErrorMessages.filter(x => !x.tested))}`) t.end() }).catch(fail) diff --git a/tests/functional/spa/errors.test.js b/tests/functional/spa/errors.test.js index c259b1701..5e3166605 100644 --- a/tests/functional/spa/errors.test.js +++ b/tests/functional/spa/errors.test.js @@ -160,9 +160,6 @@ testDriver.test('error in custom tracer', function (t, browser, router) { }) testDriver.test('string error in custom tracer', function (t, browser, router) { - // This tests throwing a string inside a custom tracer. It shows that in a specific case, the - // agent will double count the error because the error is first caught in the custom node, re-thrown, and caught again in the click event listener. - // This behavior only happens in ie11, other browsers ignore the string error and only generate 1 error. waitForPageLoadAnInitialCalls(browser, router, 'spa/errors/captured-custom-string.html') .then(() => { return clickPageAndWaitForEventsAndErrors(t, browser, router) @@ -171,8 +168,7 @@ testDriver.test('string error in custom tracer', function (t, browser, router) { // check that errors payload did not include the error const errors = getErrorsFromResponse(errorData, browser) - if (browser.match('ie@>=11')) t.equal(errors.length, 2, 'should have 2 errors (1 String Class, 1 Error Class)') - else t.equal(errors.length, 1, 'should have 1 errors') + t.equal(errors.length, 1, 'should have 1 errors') let { body, query } = eventData let interactionTree = (body && body.length ? body : querypack.decode(query.e))[0] @@ -183,6 +179,7 @@ testDriver.test('string error in custom tracer', function (t, browser, router) { var nodeId = interactionTree.children[0].nodeId var error = errors[0] + console.log(JSON.stringify(error)) t.equal(error.params.message, 'some error') t.equal(error.params.browserInteractionId, interactionId, 'should have the correct interaction id') t.equal(error.params.parentNodeId, nodeId, 'has the correct node id') diff --git a/tools/test-builds/browser-agent-wrapper/package.json b/tools/test-builds/browser-agent-wrapper/package.json index 84aca5d04..842ff2588 100644 --- a/tools/test-builds/browser-agent-wrapper/package.json +++ b/tools/test-builds/browser-agent-wrapper/package.json @@ -9,6 +9,6 @@ "author": "", "license": "ISC", "dependencies": { - "@newrelic/browser-agent": "file:../../../temp/newrelic-browser-agent-1.235.0.tgz" + "@newrelic/browser-agent": "file:../../../temp/newrelic-browser-agent-1.236.0.tgz" } } diff --git a/tools/test-builds/browser-tests/package.json b/tools/test-builds/browser-tests/package.json index 4792b15c0..fe5dcf72c 100644 --- a/tools/test-builds/browser-tests/package.json +++ b/tools/test-builds/browser-tests/package.json @@ -9,6 +9,6 @@ "author": "", "license": "ISC", "dependencies": { - "@newrelic/browser-agent": "file:../../../temp/newrelic-browser-agent-1.235.0.tgz" + "@newrelic/browser-agent": "file:../../../temp/newrelic-browser-agent-1.236.0.tgz" } } diff --git a/tools/test-builds/raw-src-wrapper/package.json b/tools/test-builds/raw-src-wrapper/package.json index 04a8e86db..e25013237 100644 --- a/tools/test-builds/raw-src-wrapper/package.json +++ b/tools/test-builds/raw-src-wrapper/package.json @@ -9,6 +9,6 @@ "author": "", "license": "ISC", "dependencies": { - "@newrelic/browser-agent": "file:../../../temp/newrelic-browser-agent-1.235.0.tgz" + "@newrelic/browser-agent": "file:../../../temp/newrelic-browser-agent-1.236.0.tgz" } } diff --git a/tools/test-builds/vite-react-wrapper/package.json b/tools/test-builds/vite-react-wrapper/package.json index 1c8b20bd2..a9ceaf88d 100644 --- a/tools/test-builds/vite-react-wrapper/package.json +++ b/tools/test-builds/vite-react-wrapper/package.json @@ -4,7 +4,7 @@ "main": "index.js", "license": "MIT", "dependencies": { - "@newrelic/browser-agent": "file:../../../temp/newrelic-browser-agent-1.235.0.tgz", + "@newrelic/browser-agent": "file:../../../temp/newrelic-browser-agent-1.236.0.tgz", "react": "18.2.0", "react-dom": "18.2.0" },