fix(errors): contain throwing LogRecordProcessor to prevent error-event loop (#303)

Author: overbalance

packages/instrumentation/src/errors/instrumentation.test.ts

@@ -72,12 +72,16 @@ const dispatchUnhandledRejection = (
   window.dispatchEvent(event);
 };
 
-// The instrumentation does not expose its diag logger for testing, so reach
-// the internal `_diag` through a single typed accessor rather than casting in
-// each test.
+// The instrumentation does not expose its `_diag` or `logger` fields for
+// testing, so reach the internals through a single typed accessor rather than
+// casting in each test.
 const internals = (inst: ErrorsInstrumentation | undefined) =>
   inst as unknown as {
-    _diag: { debug: (...args: unknown[]) => void };
+    _diag: {
+      debug: (...args: unknown[]) => void;
+      error: (...args: unknown[]) => void;
+    };
+    logger: { emit: (record: unknown) => void };
   };
 
 describe('ErrorsInstrumentation', () => {
@@ -403,4 +407,63 @@ describe('ErrorsInstrumentation', () => {
       );
     });
   });
+
+  describe('failure containment', () => {
+    beforeEach(() => {
+      instrumentation = new ErrorsInstrumentation({ enabled: false });
+      instrumentation.enable();
+    });
+
+    it('should contain a throwing LogRecordProcessor and surface it via diag', () => {
+      const accessor = internals(instrumentation);
+      const diagSpy = vi.spyOn(accessor._diag, 'error');
+      const emitSpy = vi
+        .spyOn(accessor.logger, 'emit')
+        .mockImplementation(() => {
+          throw new Error('processor exploded');
+        });
+
+      try {
+        // A throw escaping the listener would re-trigger 'error' and loop.
+        expect(() =>
+          dispatchErrorEvent(new ValidationError('boom')),
+        ).not.toThrow();
+
+        // Exactly once: the contained throw did not re-enter the listener.
+        expect(emitSpy).toHaveBeenCalledTimes(1);
+        expect(diagSpy).toHaveBeenCalledWith(
+          'failed to record exception',
+          expect.any(Error),
+        );
+      } finally {
+        emitSpy.mockRestore();
+        diagSpy.mockRestore();
+      }
+    });
+
+    it('should contain a rejection reason that throws while its properties are read', () => {
+      const diagSpy = vi.spyOn(internals(instrumentation)._diag, 'error');
+
+      // A promise can reject with any value. This one throws while its `stack`
+      // is read, exercising the extraction path before emit is ever reached.
+      const hostileReason = {} as Error;
+      Object.defineProperty(hostileReason, 'stack', {
+        get() {
+          throw new Error('stack getter exploded');
+        },
+      });
+
+      try {
+        expect(() => dispatchUnhandledRejection(hostileReason)).not.toThrow();
+
+        expect(getErrorLogs()).toHaveLength(0);
+        expect(diagSpy).toHaveBeenCalledWith(
+          'failed to record exception',
+          expect.any(Error),
+        );
+      } finally {
+        diagSpy.mockRestore();
+      }
+    });
+  });
 });

packages/instrumentation/src/errors/instrumentation.ts

@@ -97,26 +97,43 @@ export class ErrorsInstrumentation extends InstrumentationBase<ErrorsInstrumenta
       return;
     }
 
-    let errorAttributes: AnyValueMap;
-    if (typeof error === 'string') {
-      errorAttributes = { [ATTR_EXCEPTION_MESSAGE]: error };
-    } else {
-      errorAttributes = {
-        [ATTR_EXCEPTION_TYPE]: error.name,
-        [ATTR_EXCEPTION_MESSAGE]: error.message,
-        [ATTR_EXCEPTION_STACKTRACE]: error.stack,
-      };
-    }
+    // Capture as a const so the type narrowing survives into the closure below.
+    const capturedError = error;
+
+    // This runs inside a global error listener, so an escaping throw would
+    // re-trigger the listener and loop. Both attribute extraction (a rejection
+    // can carry a value whose `stack` getter throws) and emit (a broken
+    // LogRecordProcessor) can throw, so contain the whole path.
+    safeExecuteInTheMiddle(
+      () => {
+        let errorAttributes: AnyValueMap;
+        if (typeof capturedError === 'string') {
+          errorAttributes = { [ATTR_EXCEPTION_MESSAGE]: capturedError };
+        } else {
+          errorAttributes = {
+            [ATTR_EXCEPTION_TYPE]: capturedError.name,
+            [ATTR_EXCEPTION_MESSAGE]: capturedError.message,
+            [ATTR_EXCEPTION_STACKTRACE]: capturedError.stack,
+          };
+        }
 
-    const customAttributes = this._applyCustomAttributes(error);
+        const customAttributes = this._applyCustomAttributes(capturedError);
 
-    const logRecord: LogRecord = {
-      eventName: EXCEPTION_EVENT_NAME,
-      severityNumber: SeverityNumber.ERROR,
-      attributes: { ...errorAttributes, ...customAttributes },
-    };
+        const logRecord: LogRecord = {
+          eventName: EXCEPTION_EVENT_NAME,
+          severityNumber: SeverityNumber.ERROR,
+          attributes: { ...errorAttributes, ...customAttributes },
+        };
 
-    this.logger.emit(logRecord);
+        this.logger.emit(logRecord);
+      },
+      (err) => {
+        if (err) {
+          this._diag.error('failed to record exception', err);
+        }
+      },
+      true,
+    );
   }
 
   private _applyCustomAttributes(error: Error | string): Attributes {