diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 72553cc49c..75975efd80 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -63,6 +63,11 @@ paths are considered for body capture. ({pull}2873[#2873]) For example `require('node:http')` will now be instrumented. ({issues}2816[#2816]) +- Agent will delay loading of the `error-callsites` module until agent start time, + and will not load the module if the agent is disabled/inactive. This prevents the + setting of an `Error.prepareStackTrace` handler until necessary for stacktrace + collection. ({issues}2833[#2833] {pull}2906[#2906]) + [float] ===== Bug fixes diff --git a/lib/agent.js b/lib/agent.js index f3ae3e834c..c9df001974 100644 --- a/lib/agent.js +++ b/lib/agent.js @@ -22,7 +22,7 @@ var { elasticApmAwsLambda } = require('./lambda') var Metrics = require('./metrics') var parsers = require('./parsers') var symbols = require('./symbols') -const { frameCacheStats } = require('./stacktraces') +const { frameCacheStats, initStackTraceCollection } = require('./stacktraces') const Span = require('./instrumentation/span') const Transaction = require('./instrumentation/transaction') @@ -261,6 +261,7 @@ Agent.prototype.start = function (opts) { }, 'agent configured correctly') } + initStackTraceCollection() this._transport = this._conf.transport(this._conf, this) let runContextClass diff --git a/lib/stacktraces.js b/lib/stacktraces.js index 5d8fede884..c42cee4af6 100644 --- a/lib/stacktraces.js +++ b/lib/stacktraces.js @@ -18,7 +18,14 @@ var path = require('path') const asyncCache = require('async-cache') const afterAllResults = require('after-all-results') -const errorCallsites = require('error-callsites') + +// avoid loading error-callsites until needed to avoid +// Error.prepareStackTrace side-effects +// https://github.com/elastic/apm-agent-nodejs/issues/2833 +let errorCallsites +function initStackTraceCollection () { + errorCallsites = require('error-callsites') +} const errorStackParser = require('error-stack-parser') const loadSourceMap = require('./load-source-map') const LRU = require('lru-cache') @@ -408,7 +415,7 @@ function frameFromCallSite (log, callsite, cwd, sourceLinesAppFrames, sourceLine function gatherStackTrace (log, err, sourceLinesAppFrames, sourceLinesLibraryFrames, filterCallSite, cb) { // errorCallsites returns an array of v8 CallSite objects. // https://v8.dev/docs/stack-trace-api#customizing-stack-traces - let callsites = errorCallsites(err) + let callsites = errorCallsites ? errorCallsites(err) : null const next = afterAllResults(function finish (_err, stacktrace) { // _err is always null from frameFromCallSite. @@ -424,7 +431,7 @@ function gatherStackTrace (log, err, sourceLinesAppFrames, sourceLinesLibraryFra if (!isValidCallsites(callsites)) { // When can this happen? Another competing Error.prepareStackTrace breaking - // error-callsites? + // error-callsites? Also initStackTraceCollection not having been called. log.debug('could not get valid callsites from error "%s"', err) } else if (callsites) { if (filterCallSite) { @@ -447,6 +454,7 @@ function gatherStackTrace (log, err, sourceLinesAppFrames, sourceLinesLibraryFra module.exports = { gatherStackTrace, frameCacheStats, + initStackTraceCollection, // Exported for testing only. stackTraceFromErrStackString diff --git a/test/stacktraces/fixtures/get-prepare-stacktrace.js b/test/stacktraces/fixtures/get-prepare-stacktrace.js new file mode 100644 index 0000000000..02a19bc63f --- /dev/null +++ b/test/stacktraces/fixtures/get-prepare-stacktrace.js @@ -0,0 +1,20 @@ +/* + * Copyright Elasticsearch B.V. and other contributors where applicable. + * Licensed under the BSD 2-Clause License; you may not use this file except in + * compliance with the BSD 2-Clause License. + */ + +// print name of error.prepareStackTrace function to STDOUT +require('../../../').start({ + serviceName: 'test-get-prepare-stacktrace', + logUncaughtExceptions: true, + metricsInterval: 0, + centralConfig: false, + logLevel: 'off' +}) +function main () { + const name = Error.prepareStackTrace ? Error.prepareStackTrace.name : undefined + console.log(name) +} + +main() diff --git a/test/stacktraces/stacktraces.test.js b/test/stacktraces/stacktraces.test.js index 65503fa474..12759d526a 100644 --- a/test/stacktraces/stacktraces.test.js +++ b/test/stacktraces/stacktraces.test.js @@ -16,7 +16,7 @@ const tape = require('tape') const logging = require('../../lib/logging') const { MockAPMServer } = require('../_mock_apm_server') -const { gatherStackTrace, stackTraceFromErrStackString } = require('../../lib/stacktraces') +const { gatherStackTrace, initStackTraceCollection, stackTraceFromErrStackString } = require('../../lib/stacktraces') const log = logging.createLogger('off') @@ -304,6 +304,7 @@ tape.test('stackTraceFromErrStackString()', function (t) { }) tape.test('gatherStackTrace()', function (suite) { + initStackTraceCollection() function thisIsMyFunction () { // before 2 // before 1 @@ -398,5 +399,51 @@ tape.test('gatherStackTrace()', function (suite) { }) }) + tape.test('Error.prepareStackTrace is set', function (t) { + const server = new MockAPMServer() + server.start(function (serverUrl) { + execFile( + process.execPath, + ['fixtures/get-prepare-stacktrace.js'], + { + cwd: __dirname, + timeout: 3000, + env: Object.assign({}, process.env, { + ELASTIC_APM_ACTIVE: true + }) + }, + function done (err, _stdout, _stderr) { + t.ok(!err) + t.equals(_stdout.trim(), 'csPrepareStackTrace', 'Error.prepareStackTrace is set') + server.close() + t.end() + } + ) + }) + }) + + tape.test('Error.prepareStackTrace is not set', function (t) { + const server = new MockAPMServer() + server.start(function (serverUrl) { + execFile( + process.execPath, + ['fixtures/get-prepare-stacktrace.js'], + { + cwd: __dirname, + timeout: 3000, + env: Object.assign({}, process.env, { + ELASTIC_APM_ACTIVE: false + }) + }, + function done (err, _stdout, _stderr) { + t.ok(!err) + t.equals(_stdout.trim(), 'undefined', 'Error.prepareStackTrace is set') + server.close() + t.end() + } + ) + }) + }) + suite.end() })