Merge branch 'master' into onur/graphql-tracing-integration

Author: onurtemizkan

packages/browser/src/eventbuilder.ts

@@ -23,10 +23,7 @@ export function eventFromException(options: Options, exception: unknown, hint?:
   const event = eventFromUnknownInput(exception, syntheticException, {
     attachStacktrace: options.attachStacktrace,
   });
-  addExceptionMechanism(event, {
-    handled: true,
-    type: 'generic',
-  });
+  addExceptionMechanism(event); // defaults to { type: 'generic', handled: true }
   event.level = Severity.Error;
   if (hint && hint.event_id) {
     event.event_id = hint.event_id;

packages/core/src/baseclient.ts

@@ -13,6 +13,7 @@ import {
   Transport,
 } from '@sentry/types';
 import {
+  checkOrSetAlreadyCaught,
   dateTimestampInSeconds,
   Dsn,
   isPlainObject,
@@ -29,6 +30,8 @@ import {
 import { Backend, BackendClass } from './basebackend';
 import { IntegrationIndex, setupIntegrations } from './integration';
 
+const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been captured.";
+
 /**
  * Base implementation for all JavaScript SDK clients.
  *
@@ -101,6 +104,12 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
    */
   // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
   public captureException(exception: any, hint?: EventHint, scope?: Scope): string | undefined {
+    // ensure we haven't captured this very object before
+    if (checkOrSetAlreadyCaught(exception)) {
+      logger.log(ALREADY_SEEN_ERROR);
+      return;
+    }
+
     let eventId: string | undefined = hint && hint.event_id;
 
     this._process(
@@ -140,6 +149,12 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
    * @inheritDoc
    */
   public captureEvent(event: Event, hint?: EventHint, scope?: Scope): string | undefined {
+    // ensure we haven't captured this very object before
+    if (hint?.originalException && checkOrSetAlreadyCaught(hint.originalException)) {
+      logger.log(ALREADY_SEEN_ERROR);
+      return;
+    }
+
     let eventId: string | undefined = hint && hint.event_id;
 
     this._process(

packages/core/test/lib/base.test.ts

@@ -12,6 +12,9 @@ const PUBLIC_DSN = 'https://username@domain/123';
 // eslint-disable-next-line no-var
 declare var global: any;
 
+const backendEventFromException = jest.spyOn(TestBackend.prototype, 'eventFromException');
+const clientProcess = jest.spyOn(TestClient.prototype as any, '_process');
+
 jest.mock('@sentry/utils', () => {
   const original = jest.requireActual('@sentry/utils');
   return {
@@ -57,7 +60,7 @@ describe('BaseClient', () => {
   });
 
   afterEach(() => {
-    jest.restoreAllMocks();
+    jest.clearAllMocks();
   });
 
   describe('constructor() / getDsn()', () => {
@@ -249,6 +252,30 @@ describe('BaseClient', () => {
         }),
       );
     });
+
+    test.each([
+      ['`Error` instance', new Error('Will I get caught twice?')],
+      ['plain object', { 'Will I': 'get caught twice?' }],
+      ['primitive wrapper', new String('Will I get caught twice?')],
+      // Primitives aren't tested directly here because they need to be wrapped with `objectify` *before*  being passed
+      // to `captureException` (which is how we'd end up with a primitive wrapper as tested above) in order for the
+      // already-seen check to work . Any primitive which is passed without being wrapped will be captured each time it
+      // is encountered, so this test doesn't apply.
+    ])("doesn't capture the same exception twice - %s", (_name: string, thrown: any) => {
+      const client = new TestClient({ dsn: PUBLIC_DSN });
+
+      expect(thrown.__sentry_captured__).toBeUndefined();
+
+      client.captureException(thrown);
+
+      expect(thrown.__sentry_captured__).toBe(true);
+      expect(backendEventFromException).toHaveBeenCalledTimes(1);
+
+      client.captureException(thrown);
+
+      // `captureException` should bail right away this second time around and not get as far as calling this again
+      expect(backendEventFromException).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe('captureMessage', () => {
@@ -325,6 +352,35 @@ describe('BaseClient', () => {
       expect(TestBackend.instance!.event).toBeUndefined();
     });
 
+    test.each([
+      ['`Error` instance', new Error('Will I get caught twice?')],
+      ['plain object', { 'Will I': 'get caught twice?' }],
+      ['primitive wrapper', new String('Will I get caught twice?')],
+      // Primitives aren't tested directly here because they need to be wrapped with `objectify` *before*  being passed
+      // to `captureEvent` (which is how we'd end up with a primitive wrapper as tested above) in order for the
+      // already-seen check to work . Any primitive which is passed without being wrapped will be captured each time it
+      // is encountered, so this test doesn't apply.
+    ])("doesn't capture an event wrapping the same exception twice - %s", (_name: string, thrown: any) => {
+      // Note: this is the same test as in `describe(captureException)`, except with the exception already wrapped in a
+      // hint and accompanying an event. Duplicated here because some methods skip `captureException` and go straight to
+      // `captureEvent`.
+      const client = new TestClient({ dsn: PUBLIC_DSN });
+      const event = { exception: { values: [{ type: 'Error', message: 'Will I get caught twice?' }] } };
+      const hint = { originalException: thrown };
+
+      expect(thrown.__sentry_captured__).toBeUndefined();
+
+      client.captureEvent(event, hint);
+
+      expect(thrown.__sentry_captured__).toBe(true);
+      expect(clientProcess).toHaveBeenCalledTimes(1);
+
+      client.captureEvent(event, hint);
+
+      // `captureEvent` should bail right away this second time around and not get as far as calling this again
+      expect(clientProcess).toHaveBeenCalledTimes(1);
+    });
+
     test('sends an event', () => {
       expect.assertions(2);
       const client = new TestClient({ dsn: PUBLIC_DSN });
@@ -798,7 +854,7 @@ describe('BaseClient', () => {
       expect(TestBackend.instance!.event).toBeUndefined();
     });
 
-    test('calls beforeSend gets an access to a hint as a second argument', () => {
+    test('beforeSend gets access to a hint as a second argument', () => {
       expect.assertions(2);
       const beforeSend = jest.fn((event, hint) => ({ ...event, data: hint.data }));
       const client = new TestClient({ dsn: PUBLIC_DSN, beforeSend });

packages/gatsby/gatsby-browser.js

@@ -1,7 +1,27 @@
+/* eslint-disable no-console */
 const Sentry = require('@sentry/gatsby');
 
 exports.onClientEntry = function(_, pluginParams) {
-  if (pluginParams === undefined) {
+  const isIntialized = isSentryInitialized();
+  const areOptionsDefined = areSentryOptionsDefined(pluginParams);
+
+  if (isIntialized) {
+    window.Sentry = Sentry; // For backwards compatibility
+    if (areOptionsDefined) {
+      console.warn(
+        'Sentry Logger [Warn]: The SDK was initialized in the Sentry config file, but options were found in the Gatsby config. ' +
+          'These have been ignored, merge them to the Sentry config if you want to use them.\n' +
+          'Learn more about the Gatsby SDK on https://docs.sentry.io/platforms/javascript/guides/gatsby/',
+      );
+    }
+    return;
+  }
+
+  if (!areOptionsDefined) {
+    console.error(
+      'Sentry Logger [Error]: No config for the Gatsby SDK was found.\n' +
+        'Learn how to configure it on https://docs.sentry.io/platforms/javascript/guides/gatsby/',
+    );
     return;
   }
 
@@ -12,6 +32,21 @@ exports.onClientEntry = function(_, pluginParams) {
     dsn: __SENTRY_DSN__,
     ...pluginParams,
   });
-
-  window.Sentry = Sentry;
+  window.Sentry = Sentry; // For backwards compatibility
 };
+
+function isSentryInitialized() {
+  // Although `window` should exist because we're in the browser (where this script
+  // is run), and `__SENTRY__.hub` is created when importing the Gatsby SDK, double
+  // check that in case something weird happens.
+  return !!(window && window.__SENTRY__ && window.__SENTRY__.hub && window.__SENTRY__.hub.getClient());
+}
+
+function areSentryOptionsDefined(params) {
+  if (params == undefined) return false;
+  // Even if there aren't any options, there's a `plugins` property defined as an empty array
+  if (Object.keys(params).length == 1 && Array.isArray(params.plugins) && params.plugins.length == 0) {
+    return false;
+  }
+  return true;
+}

packages/gatsby/gatsby-node.js

@@ -1,3 +1,5 @@
+const fs = require('fs');
+
 const sentryRelease = JSON.stringify(
   // Always read first as Sentry takes this as precedence
   process.env.SENTRY_RELEASE ||
@@ -15,8 +17,9 @@ const sentryRelease = JSON.stringify(
 );
 
 const sentryDsn = JSON.stringify(process.env.SENTRY_DSN || '');
+const SENTRY_USER_CONFIG = ['./sentry.config.js', './sentry.config.ts'];
 
-exports.onCreateWebpackConfig = ({ plugins, actions }) => {
+exports.onCreateWebpackConfig = ({ plugins, getConfig, actions }) => {
   actions.setWebpackConfig({
     plugins: [
       plugins.define({
@@ -25,4 +28,48 @@ exports.onCreateWebpackConfig = ({ plugins, actions }) => {
       }),
     ],
   });
+
+  // To configure the SDK, SENTRY_USER_CONFIG is prioritized over `gatsby-config.js`,
+  // since it isn't possible to set non-serializable parameters in the latter.
+  // Prioritization here means what `init` is run.
+  let configFile = null;
+  try {
+    configFile = SENTRY_USER_CONFIG.find(file => fs.existsSync(file));
+  } catch (error) {
+    // Some node versions (like v11) throw an exception on `existsSync` instead of
+    // returning false. See https://github.com/tschaub/mock-fs/issues/256
+  }
+
+  if (!configFile) {
+    return;
+  }
+  // `setWebpackConfig` merges the Webpack config, ignoring some props like `entry`. See
+  // https://www.gatsbyjs.com/docs/reference/config-files/actions/#setWebpackConfig
+  // So it's not possible to inject the Sentry properties with that method. Instead, we
+  // can replace the whole config with the modifications we need.
+  const finalConfig = injectSentryConfig(getConfig(), configFile);
+  actions.replaceWebpackConfig(finalConfig);
 };
+
+function injectSentryConfig(config, configFile) {
+  const injectedEntries = {};
+  // TODO: investigate what entries need the Sentry config injected.
+  //    We may want to skip some.
+  Object.keys(config.entry).forEach(prop => {
+    const value = config.entry[prop];
+    let injectedValue = value;
+    if (typeof value === 'string') {
+      injectedValue = [configFile, value];
+    } else if (Array.isArray(value)) {
+      injectedValue = [configFile, ...value];
+    } else {
+      // eslint-disable-next-line no-console
+      console.error(
+        `Sentry Logger [Error]: Could not inject SDK initialization code into ${prop}, unexpected format: `,
+        typeof value,
+      );
+    }
+    injectedEntries[prop] = injectedValue;
+  });
+  return { ...config, entry: injectedEntries };
+}

packages/gatsby/test/gatsby-browser.test.ts

@@ -16,6 +16,8 @@ jest.mock('@sentry/gatsby', () => {
     },
   };
 });
+global.console.warn = jest.fn();
+global.console.error = jest.fn();
 
 let tracingAddExtensionMethods = jest.fn();
 jest.mock('@sentry/tracing', () => {
@@ -50,9 +52,81 @@ describe('onClientEntry', () => {
     }
   });
 
-  it('sets window.Sentry', () => {
-    onClientEntry(undefined, {});
-    expect((window as any).Sentry).not.toBeUndefined();
+  describe('inits Sentry once', () => {
+    afterEach(() => {
+      delete (window as any).Sentry;
+      delete (window as any).__SENTRY__;
+      (global.console.warn as jest.Mock).mockClear();
+      (global.console.error as jest.Mock).mockClear();
+    });
+
+    function setMockedSentryInWindow() {
+      (window as any).__SENTRY__ = {
+        hub: {
+          getClient: () => ({
+            // Empty object mocking the client
+          }),
+        },
+      };
+    }
+
+    it('initialized in injected config, without pluginParams', () => {
+      setMockedSentryInWindow();
+      onClientEntry(undefined, { plugins: [] });
+      // eslint-disable-next-line no-console
+      expect(console.warn).not.toHaveBeenCalled();
+      // eslint-disable-next-line no-console
+      expect(console.error).not.toHaveBeenCalled();
+      expect(sentryInit).not.toHaveBeenCalled();
+      expect((window as any).Sentry).toBeDefined();
+    });
+
+    it('initialized in injected config, with pluginParams', () => {
+      setMockedSentryInWindow();
+      onClientEntry(undefined, { plugins: [], dsn: 'dsn', release: 'release' });
+      // eslint-disable-next-line no-console
+      expect((console.warn as jest.Mock).mock.calls[0]).toMatchInlineSnapshot(`
+        Array [
+          "Sentry Logger [Warn]: The SDK was initialized in the Sentry config file, but options were found in the Gatsby config. These have been ignored, merge them to the Sentry config if you want to use them.
+        Learn more about the Gatsby SDK on https://docs.sentry.io/platforms/javascript/guides/gatsby/",
+        ]
+      `);
+      // eslint-disable-next-line no-console
+      expect(console.error).not.toHaveBeenCalled();
+      expect(sentryInit).not.toHaveBeenCalled();
+      expect((window as any).Sentry).toBeDefined();
+    });
+
+    it('not initialized in injected config, without pluginParams', () => {
+      onClientEntry(undefined, { plugins: [] });
+      // eslint-disable-next-line no-console
+      expect(console.warn).not.toHaveBeenCalled();
+      // eslint-disable-next-line no-console
+      expect((console.error as jest.Mock).mock.calls[0]).toMatchInlineSnapshot(`
+        Array [
+          "Sentry Logger [Error]: No config for the Gatsby SDK was found.
+        Learn how to configure it on https://docs.sentry.io/platforms/javascript/guides/gatsby/",
+        ]
+      `);
+      expect((window as any).Sentry).not.toBeDefined();
+    });
+
+    it('not initialized in injected config, with pluginParams', () => {
+      onClientEntry(undefined, { plugins: [], dsn: 'dsn', release: 'release' });
+      // eslint-disable-next-line no-console
+      expect(console.warn).not.toHaveBeenCalled();
+      // eslint-disable-next-line no-console
+      expect(console.error).not.toHaveBeenCalled();
+      expect(sentryInit).toHaveBeenCalledTimes(1);
+      expect(sentryInit.mock.calls[0][0]).toMatchInlineSnapshot(`
+              Object {
+                "dsn": "dsn",
+                "plugins": Array [],
+                "release": "release",
+              }
+            `);
+      expect((window as any).Sentry).toBeDefined();
+    });
   });
 
   it('sets a tracesSampleRate if defined as option', () => {

packages/integrations/src/vue.ts

@@ -268,7 +268,7 @@ export class Vue implements Integration {
             if (transaction) {
               this._rootSpan = transaction.startChild({
                 description: 'Application Render',
-                op: 'Vue',
+                op: 'vue',
               });
             }
             // Use functionality from @sentry/tracing
@@ -277,7 +277,7 @@ export class Vue implements Integration {
             if (activeTransaction) {
               this._rootSpan = activeTransaction.startChild({
                 description: 'Application Render',
-                op: 'Vue',
+                op: 'vue',
               });
             }
           }

packages/nextjs/package.json

@@ -18,6 +18,7 @@
   },
   "dependencies": {
     "@sentry/core": "6.13.3",
+    "@sentry/hub": "6.13.3",
     "@sentry/integrations": "6.13.3",
     "@sentry/node": "6.13.3",
     "@sentry/react": "6.13.3",