From 204eae720b8f21ad34e6ed2a475da56156ae0aa0 Mon Sep 17 00:00:00 2001 From: Ahmed Etefy Date: Thu, 15 Apr 2021 11:22:12 +0200 Subject: [PATCH 1/7] Added a function that gets a sentry release from env dynamically if release is not provided on init --- packages/node/src/sdk.ts | 41 +++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index 9aa7c2f60858..154a47166314 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -93,15 +93,7 @@ export function init(options: NodeOptions = {}): void { } if (options.release === undefined) { - const global = getGlobalObject(); - // 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; - } + options.release = getSentryRelease(); } if (options.environment === undefined && process.env.SENTRY_ENVIRONMENT) { @@ -152,3 +144,34 @@ export async function close(timeout?: number): Promise { } return Promise.reject(false); } + +/** + * A function that returns a Sentry release string dynamically from env variables + */ +export function getSentryRelease(): string { + // Fetches the variable that sentry-webpack-plugin injects + const global = getGlobalObject(); + let globalSentryRelease = undefined; + if (global.SENTRY_RELEASE && global.SENTRY_RELEASE.id) { + globalSentryRelease = global.SENTRY_RELEASE.id; + } + + const sentryRelease = JSON.stringify( + // Always read first as Sentry takes this as precedence + process.env.SENTRY_RELEASE || + // This supports the variable that sentry-webpack-plugin injects + globalSentryRelease || + // GitHub Actions - https://help.github.com/en/actions/configuring-and-managing-workflows/using-environment-variables#default-environment-variables + process.env.GITHUB_SHA || + // Netlify - https://docs.netlify.com/configure-builds/environment-variables/#build-metadata + process.env.COMMIT_REF || + // Vercel - https://vercel.com/docs/v2/build-step#system-environment-variables + process.env.VERCEL_GIT_COMMIT_SHA || + // Zeit (now known as Vercel) + process.env.ZEIT_GITHUB_COMMIT_SHA || + process.env.ZEIT_GITLAB_COMMIT_SHA || + process.env.ZEIT_BITBUCKET_COMMIT_SHA || + '', + ); + return sentryRelease; +} From 0f73bb1202aa73879fbc1fce571693288e2aeef3 Mon Sep 17 00:00:00 2001 From: Ahmed Etefy Date: Thu, 15 Apr 2021 12:46:55 +0200 Subject: [PATCH 2/7] Fixing PR comments --- packages/node/src/sdk.ts | 42 ++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index 154a47166314..8934803b2dba 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -149,29 +149,29 @@ export async function close(timeout?: number): Promise { * A function that returns a Sentry release string dynamically from env variables */ export function getSentryRelease(): string { - // Fetches the variable that sentry-webpack-plugin injects - const global = getGlobalObject(); - let globalSentryRelease = undefined; + // Always read first as Sentry takes this as precedence + if (process.env.SENTRY_RELEASE) { + return process.env.SENTRY_RELEASE; + } + + // This supports the variable that sentry-webpack-plugin injects + const global = getGlobalObject(); if (global.SENTRY_RELEASE && global.SENTRY_RELEASE.id) { - globalSentryRelease = global.SENTRY_RELEASE.id; + return global.SENTRY_RELEASE.id; } - const sentryRelease = JSON.stringify( - // Always read first as Sentry takes this as precedence - process.env.SENTRY_RELEASE || - // This supports the variable that sentry-webpack-plugin injects - globalSentryRelease || - // GitHub Actions - https://help.github.com/en/actions/configuring-and-managing-workflows/using-environment-variables#default-environment-variables - process.env.GITHUB_SHA || - // Netlify - https://docs.netlify.com/configure-builds/environment-variables/#build-metadata - process.env.COMMIT_REF || - // Vercel - https://vercel.com/docs/v2/build-step#system-environment-variables - process.env.VERCEL_GIT_COMMIT_SHA || - // Zeit (now known as Vercel) - process.env.ZEIT_GITHUB_COMMIT_SHA || - process.env.ZEIT_GITLAB_COMMIT_SHA || - process.env.ZEIT_BITBUCKET_COMMIT_SHA || - '', + return ( + // This supports the variable that sentry-webpack-plugin injects + // GitHub Actions - https://help.github.com/en/actions/configuring-and-managing-workflows/using-environment-variables#default-environment-variables + process.env.GITHUB_SHA || + // Netlify - https://docs.netlify.com/configure-builds/environment-variables/#build-metadata + process.env.COMMIT_REF || + // Vercel - https://vercel.com/docs/v2/build-step#system-environment-variables + process.env.VERCEL_GIT_COMMIT_SHA || + // Zeit (now known as Vercel) + process.env.ZEIT_GITHUB_COMMIT_SHA || + process.env.ZEIT_GITLAB_COMMIT_SHA || + process.env.ZEIT_BITBUCKET_COMMIT_SHA || + '' ); - return sentryRelease; } From e4abba375a8d2207c68e90a5c856a408db824135 Mon Sep 17 00:00:00 2001 From: Ahmed Etefy Date: Thu, 15 Apr 2021 12:52:31 +0200 Subject: [PATCH 3/7] Removed redundant documentation comment --- packages/node/src/sdk.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index 8934803b2dba..448c3fb2092d 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -161,7 +161,6 @@ export function getSentryRelease(): string { } return ( - // This supports the variable that sentry-webpack-plugin injects // GitHub Actions - https://help.github.com/en/actions/configuring-and-managing-workflows/using-environment-variables#default-environment-variables process.env.GITHUB_SHA || // Netlify - https://docs.netlify.com/configure-builds/environment-variables/#build-metadata From 8c6bc87df0abf0e987da60cb58056fa87259a807 Mon Sep 17 00:00:00 2001 From: Ahmed Etefy Date: Thu, 15 Apr 2021 13:00:30 +0200 Subject: [PATCH 4/7] Removed redundant export statement --- packages/node/src/sdk.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index 448c3fb2092d..ab357912bafc 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -148,7 +148,7 @@ export async function close(timeout?: number): Promise { /** * A function that returns a Sentry release string dynamically from env variables */ -export function getSentryRelease(): string { +function getSentryRelease(): string { // Always read first as Sentry takes this as precedence if (process.env.SENTRY_RELEASE) { return process.env.SENTRY_RELEASE; From 4ffbafa30774285957527b89906a2f0074a244b3 Mon Sep 17 00:00:00 2001 From: Ahmed Etefy Date: Thu, 15 Apr 2021 13:03:33 +0200 Subject: [PATCH 5/7] Fixed failing test because if no release set, it is empty string not undefined --- packages/node/test/index.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index 1d4efc8f0dfb..48ca08711f81 100644 --- a/packages/node/test/index.test.ts +++ b/packages/node/test/index.test.ts @@ -281,7 +281,7 @@ describe('SentryNode initialization', () => { 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(); + expect(global.__SENTRY__.hub._stack[0].client.getOptions().release).toEqual(''); }); describe('SDK metadata', () => { From bec0bcb553f42459764f7c8a1840f4e4dc1d1116 Mon Sep 17 00:00:00 2001 From: Ahmed Etefy Date: Thu, 15 Apr 2021 13:47:45 +0200 Subject: [PATCH 6/7] Added logic that checks if no release can be generated from environment, then release should be undefined --- packages/node/src/sdk.ts | 10 ++++++---- packages/node/test/index.test.ts | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index ab357912bafc..e702506fb892 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -93,7 +93,10 @@ export function init(options: NodeOptions = {}): void { } if (options.release === undefined) { - options.release = getSentryRelease(); + const detectedRelease = getSentryRelease(); + if (detectedRelease !== undefined) { + options.release = detectedRelease; + } } if (options.environment === undefined && process.env.SENTRY_ENVIRONMENT) { @@ -148,7 +151,7 @@ export async function close(timeout?: number): Promise { /** * A function that returns a Sentry release string dynamically from env variables */ -function getSentryRelease(): string { +function getSentryRelease(): string | undefined { // Always read first as Sentry takes this as precedence if (process.env.SENTRY_RELEASE) { return process.env.SENTRY_RELEASE; @@ -170,7 +173,6 @@ function getSentryRelease(): string { // Zeit (now known as Vercel) process.env.ZEIT_GITHUB_COMMIT_SHA || process.env.ZEIT_GITLAB_COMMIT_SHA || - process.env.ZEIT_BITBUCKET_COMMIT_SHA || - '' + process.env.ZEIT_BITBUCKET_COMMIT_SHA ); } diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index 48ca08711f81..1d4efc8f0dfb 100644 --- a/packages/node/test/index.test.ts +++ b/packages/node/test/index.test.ts @@ -281,7 +281,7 @@ describe('SentryNode initialization', () => { 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).toEqual(''); + expect(global.__SENTRY__.hub._stack[0].client.getOptions().release).toBeUndefined(); }); describe('SDK metadata', () => { From 0081b14224eb20096440d6c460b9c4824308fca0 Mon Sep 17 00:00:00 2001 From: Ahmed Etefy Date: Thu, 15 Apr 2021 14:09:32 +0200 Subject: [PATCH 7/7] Removed test that ensures initialization does not throw error if global sentry release is set --- packages/node/test/index.test.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/node/test/index.test.ts b/packages/node/test/index.test.ts index 1d4efc8f0dfb..1f4ddff69268 100644 --- a/packages/node/test/index.test.ts +++ b/packages/node/test/index.test.ts @@ -278,11 +278,6 @@ describe('SentryNode initialization', () => { // 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(); - }); describe('SDK metadata', () => { it('should set SDK data when Sentry.init() is called', () => {