fix: Prevent decycling from referencing original objects and pull original function name

* fix: Prevent decycling from referencing original objects
* fix: preserve correct name when wrapping
* test: Update raven-node tests for new node version

Author: kamilogorek

packages/browser/src/integrations/breadcrumbs.ts

@@ -3,7 +3,7 @@ import { Breadcrumb, Integration, SentryBreadcrumbHint, Severity } from '@sentry
 import { isFunction, isString } from '@sentry/utils/is';
 import { logger } from '@sentry/utils/logger';
 import { getEventDescription, getGlobalObject, parseUrl } from '@sentry/utils/misc';
-import { deserialize, fill, serializeObject } from '@sentry/utils/object';
+import { deserialize, fill, safeNormalize } from '@sentry/utils/object';
 import { includes, safeJoin } from '@sentry/utils/string';
 import { supportsBeacon, supportsHistory, supportsNativeFetch } from '@sentry/utils/supports';
 import { BrowserClient } from '../client';
@@ -130,7 +130,7 @@ export class Breadcrumbs implements Integration {
             category: 'console',
             data: {
               extra: {
-                arguments: serializeObject(args, 2),
+                arguments: safeNormalize(args, 3),
               },
               logger: 'console',
             },
@@ -141,7 +141,7 @@ export class Breadcrumbs implements Integration {
           if (level === 'assert') {
             if (args[0] === false) {
               breadcrumbData.message = `Assertion failed: ${safeJoin(args.slice(1), ' ') || 'console.assert'}`;
-              breadcrumbData.data.extra.arguments = serializeObject(args.slice(1), 2);
+              breadcrumbData.data.extra.arguments = safeNormalize(args.slice(1), 3);
             }
           }
 

packages/browser/src/integrations/helpers.ts

@@ -2,7 +2,7 @@ import { captureException, getCurrentHub, withScope } from '@sentry/core';
 import { Mechanism, SentryEvent, SentryWrappedFunction } from '@sentry/types';
 import { isFunction } from '@sentry/utils/is';
 import { htmlTreeAsString } from '@sentry/utils/misc';
-import { serializeObject } from '@sentry/utils/object';
+import { safeNormalize } from '@sentry/utils/object';
 
 const debounceDuration: number = 1000;
 let keypressTimeout: number | undefined;
@@ -90,7 +90,7 @@ export function wrap(
 
           processedEvent.extra = {
             ...processedEvent.extra,
-            arguments: serializeObject(args, 2),
+            arguments: safeNormalize(args, 3),
           };
 
           return processedEvent;
@@ -134,6 +134,17 @@ export function wrap(
     },
   });
 
+  // Restore original function name (not all browsers allow that)
+  try {
+    Object.defineProperty(sentryWrapped, 'name', {
+      get(): string {
+        return fn.name;
+      },
+    });
+  } catch (_oO) {
+    /*no-empty*/
+  }
+
   return sentryWrapped;
 }
 

packages/browser/test/integration/console-logs.js

@@ -1,6 +1,6 @@
 console.log('One');
 console.warn('Two', { a: 1 });
-console.error('Error 2', { b: { c: 1 } });
+console.error('Error 2', { b: { c: [] } });
 function a() {
   throw new Error('Error thrown 3');
 }

packages/browser/test/integration/test.js

@@ -1663,7 +1663,7 @@ for (var idx in frames) {
                 assert.lengthOf(sentryData.breadcrumbs, 3);
                 assert.deepEqual(sentryData.breadcrumbs[0].data.extra.arguments, ['One']);
                 assert.deepEqual(sentryData.breadcrumbs[1].data.extra.arguments, ['Two', { a: 1 }]);
-                assert.deepEqual(sentryData.breadcrumbs[2].data.extra.arguments, ['Error 2', { b: '[Object]' }]);
+                assert.deepEqual(sentryData.breadcrumbs[2].data.extra.arguments, ['Error 2', { b: { c: '[Array]' } }]);
                 done();
               }
             }

packages/browser/test/integrations/helpers.test.ts

@@ -22,6 +22,13 @@ describe('wrap()', () => {
     expect(wrap(num)).equal(num);
   });
 
+  it('should preserve correct function name when accessed', () => {
+    const namedFunction = () => 1337;
+    expect(wrap(namedFunction)).not.equal(namedFunction);
+    expect(namedFunction.name).equal('namedFunction');
+    expect(wrap(namedFunction).name).equal('namedFunction');
+  });
+
   it('bail out with the original if accessing custom props go bad', () => {
     const fn = (() => 1337) as SentryWrappedFunction;
     fn.__sentry__ = false;

packages/core/src/integrations/extraerrordata.ts

@@ -11,6 +11,11 @@ interface ExtendedError extends Error {
   [key: string]: unknown;
 }
 
+/** JSDoc */
+interface ExtraErrorDataOptions {
+  depth?: number;
+}
+
 /** Patch toString calls to return proper name for wrapped functions */
 export class ExtraErrorData implements Integration {
   /**
@@ -23,6 +28,11 @@ export class ExtraErrorData implements Integration {
    */
   public static id: string = 'ExtraErrorData';
 
+  /**
+   * @inheritDoc
+   */
+  public constructor(private readonly options: ExtraErrorDataOptions = { depth: 3 }) {}
+
   /**
    * @inheritDoc
    */
@@ -50,7 +60,7 @@ export class ExtraErrorData implements Integration {
       let extra = {
         ...event.extra,
       };
-      const normalizedErrorData = safeNormalize(errorData);
+      const normalizedErrorData = safeNormalize(errorData, this.options.depth);
       if (!isString(normalizedErrorData)) {
         extra = {
           ...event.extra,

packages/raven-node/package.json

@@ -1,7 +1,14 @@
 {
   "name": "raven",
   "description": "A standalone (Node.js) client for Sentry",
-  "keywords": ["debugging", "errors", "exceptions", "logging", "raven", "sentry"],
+  "keywords": [
+    "debugging",
+    "errors",
+    "exceptions",
+    "logging",
+    "raven",
+    "sentry"
+  ],
   "version": "2.6.4",
   "repository": "git://github.com/getsentry/raven-js.git",
   "license": "BSD-2-Clause",
@@ -13,8 +20,7 @@
   },
   "scripts": {
     "lint": "eslint .",
-    "test":
-      "NODE_ENV=test istanbul cover _mocha  -- --reporter dot && NODE_ENV=test coffee ./test/run.coffee",
+    "test": "NODE_ENV=test istanbul cover _mocha  -- --reporter dot && NODE_ENV=test coffee ./test/run.coffee",
     "test-mocha": "NODE_ENV=test mocha",
     "test-full": "npm run test && cd test/instrumentation && ./run.sh"
   },

packages/raven-node/test/raven.client.js

@@ -1411,7 +1411,12 @@ describe('raven.Client', function() {
         var y = Object.create(null);
         y.c = 'd';
         console.log(x, y);
-        client.getContext().breadcrumbs[0].message.should.equal("{ a: 'b' } { c: 'd' }");
+        client
+          .getContext()
+          .breadcrumbs[0].message.should.be.oneOf(
+            "{ a: 'b' } { c: 'd' }",
+            "[Object: null prototype] { a: 'b' } [Object: null prototype] { c: 'd' }"
+          );
         done();
       });
     });

packages/raven-node/test/raven.utils.js

@@ -238,11 +238,11 @@ describe('raven.utils', function() {
       var callback = function(frames) {
         var frame1 = frames.pop();
         frame1.in_app.should.be.false;
-        frame1.filename.should.equal('querystring.js');
+        frame1.filename.should.be.oneOf('querystring.js', 'internal/querystring.js');
 
         var frame2 = frames.pop();
         frame2.in_app.should.be.false;
-        frame2.filename.should.equal('querystring.js');
+        frame2.filename.should.be.oneOf('querystring.js', 'internal/querystring.js');
 
         done();
       };

packages/utils/src/object.ts

@@ -1,5 +1,5 @@
 import { SentryWrappedFunction } from '@sentry/types';
-import { isArray, isNaN, isPlainObject, isPrimitive, isSyntheticEvent, isUndefined } from './is';
+import { isArray, isError, isNaN, isPlainObject, isPrimitive, isSyntheticEvent, isUndefined } from './is';
 import { Memo } from './memo';
 import { truncate } from './string';
 
@@ -120,19 +120,19 @@ function jsonSize(value: any): number {
 }
 
 /** JSDoc */
-function serializeValue<T>(value: T): T | string {
+function serializeValue(value: any): string {
   const type = Object.prototype.toString.call(value);
 
+  // Node.js REPL notation
   if (typeof value === 'string') {
     return truncate(value, 40);
   } else if (type === '[object Object]') {
-    // Node.js REPL notation
     return '[Object]';
   } else if (type === '[object Array]') {
-    // Node.js REPL notation
     return '[Array]';
   } else {
-    return normalizeValue(value) as T;
+    const normalized = normalizeValue(value);
+    return isPrimitive(normalized) ? `${normalized}` : (type as string);
   }
 }
 
@@ -270,31 +270,27 @@ function objectifyError(error: ExtendedError): object {
  * - serializes Error objects
  * - filter global objects
  */
-function normalizeValue(value: any, key?: any): any {
-  if (key === 'domain' && typeof value === 'object' && (value as { _events: any })._events) {
+function normalizeValue<T>(value: T, key?: any): T | string {
+  if (key === 'domain' && typeof value === 'object' && ((value as unknown) as { _events: any })._events) {
     return '[Domain]';
   }
 
   if (key === 'domainEmitter') {
     return '[DomainEmitter]';
   }
 
-  if (typeof (global as any) !== 'undefined' && value === global) {
+  if (typeof (global as any) !== 'undefined' && (value as unknown) === global) {
     return '[Global]';
   }
 
-  if (typeof (window as any) !== 'undefined' && value === window) {
+  if (typeof (window as any) !== 'undefined' && (value as unknown) === window) {
     return '[Window]';
   }
 
-  if (typeof (document as any) !== 'undefined' && value === document) {
+  if (typeof (document as any) !== 'undefined' && (value as unknown) === document) {
     return '[Document]';
   }
 
-  if (value instanceof Error) {
-    return objectifyError(value);
-  }
-
   // tslint:disable-next-line:strict-type-predicates
   if (typeof Event !== 'undefined' && value instanceof Event) {
     return Object.getPrototypeOf(value) ? value.constructor.name : 'Event';
@@ -314,7 +310,7 @@ function normalizeValue(value: any, key?: any): any {
   }
 
   if (typeof value === 'function') {
-    return `[Function: ${(value as () => void).name || '<unknown-function-name>'}]`;
+    return `[Function: ${value.name || '<unknown-function-name>'}]`;
   }
 
   return value;
@@ -326,31 +322,34 @@ function normalizeValue(value: any, key?: any): any {
  * @param obj Object to be decycled
  * @param memo Optional Memo class handling decycling
  */
-export function decycle(obj: any, memo: Memo = new Memo()): any {
-  // tslint:disable-next-line:no-unsafe-any
-  const copy = isArray(obj) ? obj.slice() : isPlainObject(obj) ? assign({}, obj) : obj;
-  const normalized = normalizeValue(obj);
+export function decycle(obj: any, depth: number = +Infinity, memo: Memo = new Memo()): any {
+  if (depth === 0) {
+    return serializeValue(obj);
+  }
 
   // If an object was normalized to its string form, we should just bail out as theres no point in going down that branch
-  if (typeof normalized === 'string') {
+  const normalized = normalizeValue(obj);
+  if (isPrimitive(normalized)) {
     return normalized;
   }
 
-  if (!isPrimitive(obj)) {
-    if (memo.memoize(obj)) {
-      return '[Circular ~]';
-    }
-    // tslint:disable-next-line
-    for (const key in obj) {
-      // Avoid iterating over fields in the prototype if they've somehow been exposed to enumeration.
-      if (!Object.prototype.hasOwnProperty.call(obj, key)) {
-        continue;
-      }
-      // tslint:disable-next-line
-      copy[key] = decycle(obj[key], memo);
+  // tslint:disable-next-line:no-unsafe-any
+  const source = (isError(obj) ? objectifyError(obj) : obj) as {
+    [key: string]: any;
+  };
+  const copy = isArray(obj) ? [] : {};
+
+  if (memo.memoize(obj)) {
+    return '[Circular ~]';
+  }
+  for (const key in source) {
+    // Avoid iterating over fields in the prototype if they've somehow been exposed to enumeration.
+    if (!Object.prototype.hasOwnProperty.call(source, key)) {
+      continue;
     }
-    memo.unmemoize(obj);
+    (copy as { [key: string]: any })[key] = decycle(source[key], depth - 1, memo);
   }
+  memo.unmemoize(obj);
 
   return copy;
 }
@@ -362,19 +361,22 @@ export function decycle(obj: any, memo: Memo = new Memo()): any {
  * translates undefined/NaN values to "[undefined]"/"[NaN]" respectively,
  * and takes care of Error objects serialization
  */
-function serializer(options: { normalize: boolean } = { normalize: true }): (key: string, value: any) => any {
-  // tslint:disable-next-line
-  return (key: string, value: object) => (options.normalize ? normalizeValue(decycle(value), key) : decycle(value));
+function serializer(
+  options: { normalize?: boolean; depth?: number } = { normalize: true },
+): (key: string, value: any) => any {
+  return (key: string, value: object) =>
+    // tslint:disable-next-line
+    options.normalize ? normalizeValue(decycle(value, options.depth), key) : decycle(value, options.depth);
 }
 
 /**
  * safeNormalize()
  *
  * Creates a copy of the input by applying serializer function on it and parsing it back to unify the data
  */
-export function safeNormalize(input: any): any {
+export function safeNormalize(input: any, depth?: number): any {
   try {
-    return JSON.parse(JSON.stringify(input, serializer({ normalize: true })));
+    return JSON.parse(JSON.stringify(input, serializer({ normalize: true, depth })));
   } catch (_oO) {
     return '**non-serializable**';
   }