feat: delay loading error-callsites module (elastic#2906)

* feat: Delay loading error-callsites module to avoid side effects.

elastic#2833

Author: astorm

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
 

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

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

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()

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()
 })