Improve

Author: timfish

packages/node/src/integrations/localvariables.ts

@@ -10,26 +10,6 @@ import { Event, EventProcessor, Hub, Integration, StackFrame, StackParser } from
 import { Debugger, InspectorNotification, Runtime, Session } from 'inspector';
 import { LRUMap } from 'lru_map';
 
-interface ExceptionData {
-  description: string;
-}
-
-interface FrameVars {
-  function: string;
-  vars?: Record<string, unknown>;
-}
-
-/**
- * Creates a unique hash from the stack frames
- */
-function hashFrames(frames: StackFrame[] | undefined): string | undefined {
-  if (frames === undefined) {
-    return;
-  }
-
-  return frames.reduce((acc, frame) => `${acc},${frame.function},${frame.lineno},${frame.colno}`, '');
-}
-
 /**
  * Promise API is available as `Experimental` and in Node 19 only.
  *
@@ -60,81 +40,124 @@ class AsyncSession extends Session {
   }
 }
 
+// Add types for the exception event data
+type PausedExceptionEvent = Debugger.PausedEventDataType & {
+  data: {
+    // This contains error.stack
+    description: string;
+  };
+};
+
+/** Could this be an anonymous function? */
+function isAnonymous(name: string | undefined): boolean {
+  return !!name && ['', '?', '<anonymous>'].includes(name);
+}
+
+/** Do the function names appear to match? */
+function functionNamesMatch(a: string | undefined, b: string | undefined): boolean {
+  return a === b || (isAnonymous(a) && isAnonymous(b));
+}
+
+/** Creates a unique hash from stack frames */
+function hashFrames(frames: StackFrame[] | undefined): string | undefined {
+  if (frames === undefined) {
+    return;
+  }
+
+  return frames.reduce((acc, frame) => `${acc},${frame.function},${frame.lineno},${frame.colno}`, '');
+}
+
+type HashFromStackFn = (stack: string | undefined) => string | undefined;
+
+/**
+ * Creates a function used to hash stack strings
+ *
+ * We use the stack parser to create a unique hash from the exception stack trace
+ * This is used to lookup vars when the exception passes through the event processor
+ */
+function createHashFn(stackParser: StackParser | undefined): HashFromStackFn {
+  return (stack: string | undefined) => {
+    if (stackParser === undefined || stack === undefined) {
+      return undefined;
+    }
+
+    return hashFrames(stackParser(stack, 1));
+  };
+}
+
+interface FrameVariables {
+  function: string;
+  vars?: Record<string, unknown>;
+}
+
 /**
  * Adds local variables to exception frames
  */
 export class LocalVariables implements Integration {
   public static id: string = 'LocalVariables';
 
-  public name: string = LocalVariables.id;
+  public readonly name: string = LocalVariables.id;
 
-  private readonly _session: AsyncSession;
-  private readonly _cachedFrameVars: LRUMap<string, Promise<FrameVars[]>> = new LRUMap(50);
-  // We use the stack parser to create a unique hash from the exception stack trace
-  // This is used to lookup vars when
-  private _stackParser: StackParser | undefined;
+  private readonly _session: AsyncSession = new AsyncSession();
+  private readonly _cachedFrames: LRUMap<string, Promise<FrameVariables[]>> = new LRUMap(50);
 
   public constructor() {
-    this._session = new AsyncSession();
     this._session.connect();
     this._session.on('Debugger.paused', this._handlePaused.bind(this));
     this._session.post('Debugger.enable');
-    // We only care about uncaught exceptions
+    // We only want to pause on uncaught exceptions
     this._session.post('Debugger.setPauseOnExceptions', { state: 'uncaught' });
   }
 
   /**
    * @inheritDoc
    */
   public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void, getCurrentHub: () => Hub): void {
-    this._stackParser = getCurrentHub().getClient()?.getOptions().stackParser;
+    this._stackHasher = createHashFn(getCurrentHub().getClient()?.getOptions().stackParser);
 
     addGlobalEventProcessor(async event => this._addLocalVariables(event));
   }
 
+  /**
+   * We use the stack parser to create a unique hash from the exception stack trace
+   * This is used to lookup vars when
+   */
+  private _stackHasher: HashFromStackFn = _ => undefined;
+
   /**
    * Handle the pause event
    */
-  private async _handlePaused(event: InspectorNotification<Debugger.PausedEventDataType>): Promise<void> {
-    // We only care about exceptions for now
-    if (event.params.reason !== 'exception') {
+  private async _handlePaused({
+    params: { reason, data, callFrames },
+  }: InspectorNotification<PausedExceptionEvent>): Promise<void> {
+    if (reason !== 'exception' && reason !== 'promiseRejection') {
       return;
     }
 
-    const exceptionData = event.params?.data as ExceptionData | undefined;
-
-    // event.params.data.description contains the original error.stack
-    const exceptionHash =
-      exceptionData?.description && this._stackParser
-        ? hashFrames(this._stackParser(exceptionData.description, 1))
-        : undefined;
+    // data.description contains the original error.stack
+    const exceptionHash = this._stackHasher(data?.description);
 
     if (exceptionHash == undefined) {
       return;
     }
 
-    // We add the un-awaited promise to the cache rather than await here otherwise the event processor
-    // can be called before we're finished getting all the vars
-    const framesPromise: Promise<FrameVars[]> = Promise.all(
-      event.params.callFrames.map(async callFrame => {
-        const localScope = callFrame.scopeChain.find(scope => scope.type === 'local');
+    const framePromises = callFrames.map(async ({ scopeChain, functionName, this: obj }) => {
+      const localScope = scopeChain.find(scope => scope.type === 'local');
 
-        if (localScope?.object.objectId) {
-          const vars = await this._unrollProps(await this._session.getProperties(localScope.object.objectId));
+      const fn = obj.className !== 'global' ? `${obj.className}.${functionName}` : functionName;
 
-          const fn =
-            callFrame.this.className !== 'global'
-              ? `${callFrame.this.className}.${callFrame.functionName}`
-              : callFrame.functionName;
+      if (localScope?.object.objectId === undefined) {
+        return { function: fn };
+      }
 
-          return { function: fn, vars };
-        }
+      const vars = await this._unrollProps(await this._session.getProperties(localScope.object.objectId));
 
-        return { function: callFrame.functionName };
-      }),
-    );
+      return { function: fn, vars };
+    });
 
-    this._cachedFrameVars.set(exceptionHash, framesPromise);
+    // We add the un-awaited promise to the cache rather than await here otherwise the event processor
+    // can be called before we're finished getting all the vars
+    this._cachedFrames.set(exceptionHash, Promise.all(framePromises));
   }
 
   /**
@@ -191,34 +214,35 @@ export class LocalVariables implements Integration {
     }
 
     // Check if we have local variables for an exception that matches the hash
-    const cachedFrameVars = await this._cachedFrameVars.get(hash);
+    const cachedFrames = await this._cachedFrames.get(hash);
 
-    if (cachedFrameVars === undefined) {
+    if (cachedFrames === undefined) {
       return event;
     }
 
     const frameCount = event?.exception?.values?.[0]?.stacktrace?.frames?.length || 0;
 
     for (let i = 0; i < frameCount; i++) {
+      // Sentry frames are already in reverse order
       const frameIndex = frameCount - i - 1;
 
-      // Drop out if we run out of frames to match
-      if (!event?.exception?.values?.[0]?.stacktrace?.frames?.[frameIndex] || !cachedFrameVars[i]) {
+      // Drop out if we run out of frames to match up
+      if (!event?.exception?.values?.[0]?.stacktrace?.frames?.[frameIndex] || !cachedFrames[i]) {
         break;
       }
 
       if (
+        // We need to have vars to add
+        cachedFrames[i].vars === undefined ||
         // We're not interested in frames that are not in_app because the vars are not relevant
         event.exception.values[0].stacktrace.frames[frameIndex].in_app === false ||
         // The function names need to match
-        event.exception.values[0].stacktrace.frames[frameIndex].function !== cachedFrameVars[i].function ||
-        // We need to have vars to add
-        cachedFrameVars[i].vars === undefined
+        !functionNamesMatch(event.exception.values[0].stacktrace.frames[frameIndex].function, cachedFrames[i].function)
       ) {
         continue;
       }
 
-      event.exception.values[0].stacktrace.frames[frameIndex].vars = cachedFrameVars[i].vars;
+      event.exception.values[0].stacktrace.frames[frameIndex].vars = cachedFrames[i].vars;
     }
 
     return event;