feat(node): Collect Local Variables via a worker (#11586)

Our Local Variables integration is limited by the fact that it executes
debugger commands from the main app thread. This means that no async
work can be completed while the debugger is paused and local variables
are being collected. This in turn means that all the synchronous work
needs to be queued which risks stack overflows. For this reason we've
limited local variable collection to the 5 frames closest to the top of
the stack.

When debugging from a worker thread, there are no such limitations
around performing async work during debugger pause.

This PR adds a worker thread which is tasked with collecting local
variables when the debugger pauses. It passes these back to the main
thread via IPC which are then later added to event stack frames.

## Possible Improvements

It's possible to combine both ANR and Local Variables into a single
worker thread.

Author: timfish

dev-packages/rollup-utils/utils.mjs

@@ -21,7 +21,7 @@ export function mergePlugins(pluginsA, pluginsB) {
     // here.
     // Additionally, the excludeReplay plugin must run before TS/Sucrase so that we can eliminate the replay code
     // before anything is type-checked (TS-only) and transpiled.
-    const order = ['excludeReplay', 'typescript', 'sucrase', '...', 'terser', 'license'];
+    const order = ['excludeReplay', 'typescript', 'sucrase', '...', 'terser', 'license', 'output-base64-worker-script'];
     const sortKeyA = order.includes(a.name) ? a.name : '...';
     const sortKeyB = order.includes(b.name) ? b.name : '...';
 

packages/node/rollup.anr-worker.config.mjs

@@ -1,17 +1,18 @@
 import { makeBaseBundleConfig } from '@sentry-internal/rollup-utils';
 
-export function createAnrWorkerCode() {
+export function createWorkerCodeBuilder(entry, outDir) {
   let base64Code;
 
-  return {
-    workerRollupConfig: makeBaseBundleConfig({
+  return [
+    makeBaseBundleConfig({
       bundleType: 'node-worker',
-      entrypoints: ['src/integrations/anr/worker.ts'],
+      entrypoints: [entry],
+      sucrase: { disableESTransforms: true },
       licenseTitle: '@sentry/node',
       outputFileBase: () => 'worker-script.js',
       packageSpecificConfig: {
         output: {
-          dir: 'build/esm/integrations/anr',
+          dir: outDir,
           sourcemap: false,
         },
         plugins: [
@@ -24,8 +25,8 @@ export function createAnrWorkerCode() {
         ],
       },
     }),
-    getBase64Code() {
+    () => {
       return base64Code;
     },
-  };
+  ];
 }

packages/node/rollup.npm.config.mjs

@@ -1,13 +1,22 @@
 import replace from '@rollup/plugin-replace';
 import { makeBaseNPMConfig, makeNPMConfigVariants, makeOtelLoaders } from '@sentry-internal/rollup-utils';
-import { createAnrWorkerCode } from './rollup.anr-worker.config.mjs';
+import { createWorkerCodeBuilder } from './rollup.anr-worker.config.mjs';
 
-const { workerRollupConfig, getBase64Code } = createAnrWorkerCode();
+const [anrWorkerConfig, getAnrBase64Code] = createWorkerCodeBuilder(
+  'src/integrations/anr/worker.ts',
+  'build/esm/integrations/anr',
+);
+
+const [localVariablesWorkerConfig, getLocalVariablesBase64Code] = createWorkerCodeBuilder(
+  'src/integrations/local-variables/worker.ts',
+  'build/esm/integrations/local-variables',
+);
 
 export default [
   ...makeOtelLoaders('./build', 'otel'),
-  // The worker needs to be built first since it's output is used in the main bundle.
-  workerRollupConfig,
+  // The workers needs to be built first since it's their output is copied in the main bundle.
+  anrWorkerConfig,
+  localVariablesWorkerConfig,
   ...makeNPMConfigVariants(
     makeBaseNPMConfig({
       packageSpecificConfig: {
@@ -23,10 +32,11 @@ export default [
         plugins: [
           replace({
             delimiters: ['###', '###'],
-            // removes some webpack warnings
+            // removes some rollup warnings
             preventAssignment: true,
             values: {
-              base64WorkerScript: () => getBase64Code(),
+              AnrWorkerScript: () => getAnrBase64Code(),
+              LocalVariablesWorkerScript: () => getLocalVariablesBase64Code(),
             },
           }),
         ],

packages/node/src/integrations/anr/index.ts

@@ -7,7 +7,9 @@ import { getCurrentScope, getGlobalScope, getIsolationScope } from '../..';
 import { NODE_VERSION } from '../../nodeVersion';
 import type { NodeClient } from '../../sdk/client';
 import type { AnrIntegrationOptions, WorkerStartData } from './common';
-import { base64WorkerScript } from './worker-script';
+
+// This string is a placeholder that gets overwritten with the worker code.
+export const base64WorkerScript = '###AnrWorkerScript###';
 
 const DEFAULT_INTERVAL = 50;
 const DEFAULT_HANG_THRESHOLD = 5000;

packages/node/src/integrations/anr/worker-script.ts

@@ -117,3 +117,16 @@ export interface LocalVariablesIntegrationOptions {
    */
   maxExceptionsPerSecond?: number;
 }
+
+export interface LocalVariablesWorkerArgs extends LocalVariablesIntegrationOptions {
+  /**
+   * Whether to enable debug logging.
+   */
+  debug: boolean;
+  /**
+   * Base path used to calculate module name.
+   *
+   * Defaults to `dirname(process.argv[1])` and falls back to `process.cwd()`
+   */
+  basePath?: string;
+}

packages/node/src/integrations/local-variables/common.ts

@@ -1,3 +1,9 @@
+import type { Integration } from '@sentry/types';
+import { NODE_VERSION } from '../../nodeVersion';
+import type { LocalVariablesIntegrationOptions } from './common';
+import { localVariablesAsyncIntegration } from './local-variables-async';
 import { localVariablesSyncIntegration } from './local-variables-sync';
 
-export const localVariablesIntegration = localVariablesSyncIntegration;
+export const localVariablesIntegration = (options: LocalVariablesIntegrationOptions = {}): Integration => {
+  return NODE_VERSION.major < 19 ? localVariablesSyncIntegration(options) : localVariablesAsyncIntegration(options);
+};

packages/node/src/integrations/local-variables/index.ts

@@ -0,0 +1,31 @@
+/**
+ * @types/node doesn't have a `node:inspector/promises` module, maybe because it's still experimental?
+ */
+declare module 'node:inspector/promises' {
+  /**
+   * Async Debugger session
+   */
+  class Session {
+    public constructor();
+
+    public connect(): void;
+    public connectToMainThread(): void;
+
+    public post(method: 'Debugger.pause' | 'Debugger.resume' | 'Debugger.enable' | 'Debugger.disable'): Promise<void>;
+    public post(
+      method: 'Debugger.setPauseOnExceptions',
+      params: Debugger.SetPauseOnExceptionsParameterType,
+    ): Promise<void>;
+    public post(
+      method: 'Runtime.getProperties',
+      params: Runtime.GetPropertiesParameterType,
+    ): Promise<Runtime.GetPropertiesReturnType>;
+
+    public on(
+      event: 'Debugger.paused',
+      listener: (message: InspectorNotification<Debugger.PausedEventDataType>) => void,
+    ): Session;
+
+    public on(event: 'Debugger.resumed', listener: () => void): Session;
+  }
+}

packages/node/src/integrations/local-variables/inspector.d.ts

@@ -0,0 +1,141 @@
+import { defineIntegration } from '@sentry/core';
+import type { Event, Exception, IntegrationFn } from '@sentry/types';
+import { LRUMap, logger } from '@sentry/utils';
+import { Worker } from 'worker_threads';
+
+import type { NodeClient } from '../../sdk/client';
+import type { FrameVariables, LocalVariablesIntegrationOptions, LocalVariablesWorkerArgs } from './common';
+import { functionNamesMatch, hashFrames } from './common';
+
+// This string is a placeholder that gets overwritten with the worker code.
+export const base64WorkerScript = '###LocalVariablesWorkerScript###';
+
+function log(...args: unknown[]): void {
+  logger.log('[LocalVariables]', ...args);
+}
+
+/**
+ * Adds local variables to exception frames
+ */
+export const localVariablesAsyncIntegration = defineIntegration(((
+  integrationOptions: LocalVariablesIntegrationOptions = {},
+) => {
+  const cachedFrames: LRUMap<string, FrameVariables[]> = new LRUMap(20);
+
+  function addLocalVariablesToException(exception: Exception): void {
+    const hash = hashFrames(exception?.stacktrace?.frames);
+
+    if (hash === undefined) {
+      return;
+    }
+
+    // Check if we have local variables for an exception that matches the hash
+    // remove is identical to get but also removes the entry from the cache
+    const cachedFrame = cachedFrames.remove(hash);
+
+    if (cachedFrame === undefined) {
+      return;
+    }
+
+    // Filter out frames where the function name is `new Promise` since these are in the error.stack frames
+    // but do not appear in the debugger call frames
+    const frames = (exception.stacktrace?.frames || []).filter(frame => frame.function !== 'new Promise');
+
+    for (let i = 0; i < frames.length; i++) {
+      // Sentry frames are in reverse order
+      const frameIndex = frames.length - i - 1;
+
+      // Drop out if we run out of frames to match up
+      if (!frames[frameIndex] || !cachedFrame[i]) {
+        break;
+      }
+
+      if (
+        // We need to have vars to add
+        cachedFrame[i].vars === undefined ||
+        // We're not interested in frames that are not in_app because the vars are not relevant
+        frames[frameIndex].in_app === false ||
+        // The function names need to match
+        !functionNamesMatch(frames[frameIndex].function, cachedFrame[i].function)
+      ) {
+        continue;
+      }
+
+      frames[frameIndex].vars = cachedFrame[i].vars;
+    }
+  }
+
+  function addLocalVariablesToEvent(event: Event): Event {
+    for (const exception of event.exception?.values || []) {
+      addLocalVariablesToException(exception);
+    }
+
+    return event;
+  }
+
+  async function startInspector(): Promise<void> {
+    // We load inspector dynamically because on some platforms Node is built without inspector support
+    const inspector = await import('inspector');
+    if (!inspector.url()) {
+      inspector.open(0);
+    }
+  }
+
+  function startWorker(options: LocalVariablesWorkerArgs): void {
+    const worker = new Worker(new URL(`data:application/javascript;base64,${base64WorkerScript}`), {
+      workerData: options,
+    });
+
+    process.on('exit', () => {
+      // eslint-disable-next-line @typescript-eslint/no-floating-promises
+      worker.terminate();
+    });
+
+    worker.on('message', ({ exceptionHash, frames }) => {
+      cachedFrames.set(exceptionHash, frames);
+    });
+
+    worker.once('error', (err: Error) => {
+      log('Worker error', err);
+    });
+
+    worker.once('exit', (code: number) => {
+      log('Worker exit', code);
+    });
+
+    // Ensure this thread can't block app exit
+    worker.unref();
+  }
+
+  return {
+    name: 'LocalVariablesAsync',
+    setup(client: NodeClient) {
+      const clientOptions = client.getOptions();
+
+      if (!clientOptions.includeLocalVariables) {
+        return;
+      }
+
+      const options: LocalVariablesWorkerArgs = {
+        ...integrationOptions,
+        debug: logger.isEnabled(),
+      };
+
+      startInspector().then(
+        () => {
+          try {
+            startWorker(options);
+          } catch (e) {
+            logger.error('Failed to start worker', e);
+          }
+        },
+        e => {
+          logger.error('Failed to start inspector', e);
+        },
+      );
+    },
+    processEvent(event: Event): Event {
+      return addLocalVariablesToEvent(event);
+    },
+  };
+}) satisfies IntegrationFn);