feat: Dont blow up when someone puts console.log inside beforeBreadcrumb call

Author: kamilogorek

packages/browser/src/integrations/breadcrumbs.ts

@@ -27,6 +27,50 @@ export interface SentryWrappedXMLHttpRequest extends XMLHttpRequest {
   };
 }
 
+/**
+ * Wrapper function that'll be used for every console level
+ */
+function consoleWrapper(originalConsole: ExtensibleConsole): any {
+  return function(level: string): any {
+    if (!(level in global.console)) {
+      return;
+    }
+
+    fill(originalConsole, level, function(originalConsoleLevel: () => any): any {
+      return function(...args: any[]): any {
+        const breadcrumbData = {
+          category: 'console',
+          data: {
+            extra: {
+              arguments: args.slice(1),
+            },
+            logger: 'console',
+          },
+          level: Severity.fromString(level),
+          message: safeJoin(args, ' '),
+        };
+
+        if (level === 'assert') {
+          if (args[0] === false) {
+            breadcrumbData.message = `Assertion failed: ${safeJoin(args.slice(1), ' ') || 'console.assert'}`;
+            breadcrumbData.data.extra.arguments = args.slice(1);
+          }
+        }
+
+        getCurrentHub().addBreadcrumb(breadcrumbData, {
+          input: args,
+          level,
+        });
+
+        // this fails for some browsers. :(
+        if (originalConsoleLevel) {
+          originalConsoleLevel.apply(originalConsole, args);
+        }
+      };
+    });
+  };
+}
+
 /** JSDoc */
 function addSentryBreadcrumb(serializedData: string): void {
   // There's always something that can go wrong with deserialization...
@@ -130,48 +174,8 @@ export class Breadcrumbs implements Integration {
     if (!('console' in global)) {
       return;
     }
-
     const originalConsole = global.console as ExtensibleConsole;
-
-    ['debug', 'info', 'warn', 'error', 'log'].forEach(level => {
-      if (!(level in console)) {
-        return;
-      }
-
-      // tslint:disable-next-line
-      const originalConsoleLevel = originalConsole[level];
-
-      (global.console as ExtensibleConsole)[level] = function(...args: any[]): void {
-        const breadcrumbData = {
-          category: 'console',
-          data: {
-            extra: {
-              arguments: args.slice(1),
-            },
-            logger: 'console',
-          },
-          level: Severity.fromString(level),
-          message: safeJoin(args, ' '),
-        };
-
-        if (level === 'assert') {
-          if (args[0] === false) {
-            breadcrumbData.message = `Assertion failed: ${safeJoin(args.slice(1), ' ') || 'console.assert'}`;
-            breadcrumbData.data.extra.arguments = args.slice(1);
-          }
-        }
-
-        getCurrentHub().addBreadcrumb(breadcrumbData, {
-          input: args,
-          level,
-        });
-
-        // this fails for some browsers. :(
-        if (originalConsoleLevel) {
-          Function.prototype.apply.call(originalConsoleLevel, originalConsole, args);
-        }
-      };
-    });
+    ['debug', 'info', 'warn', 'error', 'log'].forEach(consoleWrapper(originalConsole));
   }
 
   /** JSDoc */

packages/core/src/baseclient.ts

@@ -8,12 +8,50 @@ import {
   Severity,
   Status,
 } from '@sentry/types';
-import { uuid4 } from '@sentry/utils/misc';
+import { getGlobalObject, uuid4 } from '@sentry/utils/misc';
 import { truncate } from '@sentry/utils/string';
 import { BackendClass } from './basebackend';
 import { Dsn } from './dsn';
 import { Backend, Client, Options } from './interfaces';
 
+/** JSDoc */
+interface ExtensibleConsole extends Console {
+  [key: string]: any;
+}
+
+/** JSDoc */
+async function beforeBreadcrumbConsoleLoopGuard(
+  callback: () => Breadcrumb | Promise<Breadcrumb | null> | null,
+): Promise<Breadcrumb | null> {
+  const global = getGlobalObject();
+  const levels = ['debug', 'info', 'warn', 'error', 'log'];
+
+  if (!('console' in global)) {
+    return callback();
+  }
+
+  const originalConsole = global.console as ExtensibleConsole;
+
+  // Restore all wrapped console methods
+  levels.forEach(level => {
+    if (level in global.console && originalConsole[level].__sentry__) {
+      originalConsole[level] = originalConsole[level].__sentry_original__;
+    }
+  });
+
+  // Perform callback manipulations
+  const result = await callback();
+
+  // Revert restoration to wrapped state
+  levels.forEach(level => {
+    if (level in global.console && originalConsole[level].__sentry__) {
+      originalConsole[level] = originalConsole[level].__sentry_wrapped__;
+    }
+  });
+
+  return result;
+}
+
 /**
  * Default maximum number of breadcrumbs added to an event. Can be overwritten
  * with {@link Options.maxBreadcrumbs}.
@@ -182,7 +220,9 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
 
     const timestamp = new Date().getTime() / 1000;
     const mergedBreadcrumb = { timestamp, ...breadcrumb };
-    const finalBreadcrumb = beforeBreadcrumb ? await beforeBreadcrumb(mergedBreadcrumb, hint) : mergedBreadcrumb;
+    const finalBreadcrumb = beforeBreadcrumb
+      ? await beforeBreadcrumbConsoleLoopGuard(() => beforeBreadcrumb(mergedBreadcrumb, hint))
+      : mergedBreadcrumb;
 
     if (finalBreadcrumb === null) {
       return;

packages/utils/src/object.ts

@@ -162,6 +162,8 @@ export function fill(source: { [key: string]: any }, name: string, replacement:
   source[name].__sentry__ = true;
   // tslint:disable-next-line:no-unsafe-any
   source[name].__sentry_original__ = original;
+  // tslint:disable-next-line:no-unsafe-any
+  source[name].__sentry_wrapped__ = source[name];
 }
 
 /**