Support sentry-webpack-plugin release injection in browser and node SDKs (getsentry#2132)

tstirrat15 · HazAT · commit a44dcb8b21ac · 2019-06-27T16:25:25.000+02:00
* Add clarification in the contributing.md doc

* Add release setting via global var
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -46,11 +46,11 @@ in the project root.
 
 ## Running the Test Suite
 
-You can run all test at once by calling `yarn test` in the project root or in individual sub packages.
+You can run all test at once by calling `yarn test` in the project root or in individual sub packages. Note that you must run `yarn build` before the test command will work.
 
 ## Lint
 
-You can run all test at once by calling `yarn lint` in the project root or in individual sub packages.
+You can run all test at once by calling `yarn lint` in the project root or in individual sub packages. Note that you must run `yarn build` before the lint command will work.
 
 ## Contributing Back Code
 
diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts
@@ -1,4 +1,5 @@
 import { getCurrentHub, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
+import { getGlobalObject } from '@sentry/utils';
 
 import { BrowserOptions } from './backend';
 import { BrowserClient, ReportDialogOptions } from './client';
@@ -76,6 +77,13 @@ export function init(options: BrowserOptions = {}): void {
   if (options.defaultIntegrations === undefined) {
     options.defaultIntegrations = defaultIntegrations;
   }
+  if (options.release === undefined) {
+    const window = getGlobalObject<Window>() as any;
+    // This supports the variable that sentry-webpack-plugin injects
+    if (window.SENTRY_RELEASE && window.SENTRY_RELEASE.id) {
+      options.release = window.SENTRY_RELEASE.id;
+    }
+  }
   initAndBind(BrowserClient, options);
 }
 
diff --git a/packages/browser/test/index.test.ts b/packages/browser/test/index.test.ts
@@ -166,3 +166,19 @@ describe('SentryBrowser', () => {
     });
   });
 });
+
+
+describe('SentryBrowser initialization', () => {
+  it('should use window.SENTRY_RELEASE to set release on initialization if available', () => {
+    global.SENTRY_RELEASE = { id: 'foobar' };
+    init({ dsn });
+    expect(global.__SENTRY__.hub._stack[0].client.getOptions().release).to.equal('foobar');
+    // Manually tear down global set. Is there a nicer way to do this?
+    global.SENTRY_RELEASE = undefined;
+  });
+  it('should have initialization proceed as normal if window.SENTRY_RELEASE is not set', () => {
+    // This is mostly a happy-path test to ensure that the initialization doesn't throw an error.
+    init({ dsn });
+    expect(global.__SENTRY__.hub._stack[0].client.getOptions().release).to.be.undefined;
+  });
+})
diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts
@@ -1,5 +1,6 @@
 import { getCurrentHub, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
 import { getMainCarrier, setHubOnCarrier } from '@sentry/hub';
+import { getGlobalObject } from '@sentry/utils';
 import * as domain from 'domain';
 
 import { NodeOptions } from './backend';
@@ -84,8 +85,16 @@ export function init(options: NodeOptions = {}): void {
     options.dsn = process.env.SENTRY_DSN;
   }
 
-  if (options.release === undefined && process.env.SENTRY_RELEASE) {
-    options.release = process.env.SENTRY_RELEASE;
+  if (options.release === undefined) {
+    const global = getGlobalObject<Window>() as any;
+    // Prefer env var over global
+    if (process.env.SENTRY_RELEASE) {
+      options.release = process.env.SENTRY_RELEASE;
+    }
+    // This supports the variable that sentry-webpack-plugin injects
+    else if (global.SENTRY_RELEASE && global.SENTRY_RELEASE.id) {
+      options.release = global.SENTRY_RELEASE.id;
+    }
   }
 
   if (options.environment === undefined && process.env.SENTRY_ENVIRONMENT) {
diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts
@@ -238,3 +238,18 @@ describe('SentryNode', () => {
     });
   });
 });
+
+describe('SentryNode initialization', () => {
+  test('global.SENTRY_RELEASE is used to set release on initialization if available', () => {
+    global.SENTRY_RELEASE = { id: 'foobar' };
+    init({ dsn });
+    expect(global.__SENTRY__.hub._stack[0].client.getOptions().release).toEqual('foobar');
+    // Unsure if this is needed under jest.
+    global.SENTRY_RELEASE = undefined;
+  });
+  test('initialization proceeds as normal if global.SENTRY_RELEASE is not set', () => {
+    // This is mostly a happy-path test to ensure that the initialization doesn't throw an error.
+    init({ dsn });
+    expect(global.__SENTRY__.hub._stack[0].client.getOptions().release).toBeUndefined();
+  });
+})
