From fe53dd970ae2a7a2f225e9f0e2dfd2e420a56986 Mon Sep 17 00:00:00 2001
From: Ahmed Etefy <ahmed.etefy12@gmail.com>
Date: Thu, 17 Jun 2021 20:59:53 +0200
Subject: [PATCH] fix(release-health): Prevent sending terminal status session
 updates

Drops sending session updates for sessions that are already in terminal
states and caps the number of errors for session at 1
---
 packages/core/src/baseclient.ts               | 10 ++-
 packages/node/package.json                    |  2 +-
 .../errors-in-session-capped-to-one.js        | 65 +++++++++++++++++++
 3 files changed, 74 insertions(+), 3 deletions(-)
 create mode 100644 packages/node/test/manual/release-health/single-session/errors-in-session-capped-to-one.js

diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
index 7ae94741b16c..584fca88143f 100644
--- a/packages/core/src/baseclient.ts
+++ b/packages/core/src/baseclient.ts
@@ -253,13 +253,19 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
       }
     }
 
+    const initialSessionStatus = session.status;
+    const sessionErrorCount = session.errors;
+    const terminalStates = [SessionStatus.Crashed, SessionStatus.Abnormal];
+
     session.update({
       ...(crashed && { status: SessionStatus.Crashed }),
       user,
       userAgent,
-      errors: session.errors + Number(errored || crashed),
+      errors: !sessionErrorCount ? sessionErrorCount + Number(errored || crashed) : sessionErrorCount,
     });
-    this.captureSession(session);
+
+    // Only send a session update if session was not already in a terminal
+    if (!terminalStates.includes(initialSessionStatus)) this.captureSession(session);
   }
 
   /** Deliver captured session to Sentry */
diff --git a/packages/node/package.json b/packages/node/package.json
index e4aa83b0fb8a..f50fe6d5932e 100644
--- a/packages/node/package.json
+++ b/packages/node/package.json
@@ -60,7 +60,7 @@
     "test:watch": "jest --watch",
     "test:express": "node test/manual/express-scope-separation/start.js",
     "test:webpack": "cd test/manual/webpack-domain/ && yarn && node npm-build.js",
-    "test:release-health": "node test/manual/release-health/single-session/healthy-session.js && node test/manual/release-health/single-session/caught-exception-errored-session.js && node test/manual/release-health/single-session/uncaught-exception-crashed-session.js && node test/manual/release-health/single-session/unhandled-rejection-crashed-session.js && node test/manual/release-health/session-aggregates/aggregates-disable-single-session.js",
+    "test:release-health": "node test/manual/release-health/single-session/healthy-session.js && node test/manual/release-health/single-session/caught-exception-errored-session.js && node test/manual/release-health/single-session/uncaught-exception-crashed-session.js && node test/manual/release-health/single-session/unhandled-rejection-crashed-session.js && node test/manual/release-health/session-aggregates/aggregates-disable-single-session.js && node test/manual/release-health/single-session/errors-in-session-capped-to-one.js",
     "pack": "npm pack",
     "circularDepCheck": "madge --circular src/index.ts"
   },
diff --git a/packages/node/test/manual/release-health/single-session/errors-in-session-capped-to-one.js b/packages/node/test/manual/release-health/single-session/errors-in-session-capped-to-one.js
new file mode 100644
index 000000000000..781b22758f7c
--- /dev/null
+++ b/packages/node/test/manual/release-health/single-session/errors-in-session-capped-to-one.js
@@ -0,0 +1,65 @@
+const Sentry = require('../../../../dist');
+const { assertSessions, constructStrippedSessionObject, BaseDummyTransport } = require('../test-utils');
+let sessionCounter = 0;
+process.on('exit', ()=> {
+  if (process.exitCode !== 1) {
+    console.log('SUCCESS: All application mode sessions were sent to node transport as expected');
+  }
+})
+
+class DummyTransport extends BaseDummyTransport {
+  sendSession(session) {
+    sessionCounter++;
+    if (sessionCounter === 1) {
+      assertSessions(constructStrippedSessionObject(session),
+        {
+          init: true,
+          status: 'ok',
+          errors: 1,
+          release: '1.1'
+        }
+      )
+    }
+    else if (sessionCounter === 2) {
+      assertSessions(constructStrippedSessionObject(session),
+        {
+          init: false,
+          status: 'ok',
+          errors: 1,
+          release: '1.1'
+        }
+      )
+    }
+    else if (sessionCounter === 3) {
+      assertSessions(constructStrippedSessionObject(session),
+        {
+          init: false,
+          status: 'exited',
+          errors: 1,
+          release: '1.1'
+        }
+      )
+    }
+    else {
+      console.log('FAIL: Received way too many Sessions!');
+      process.exit(1);
+    }
+    return super.sendSession(session);
+  }
+}
+
+Sentry.init({
+  dsn: 'http://test@example.com/1337',
+  release: '1.1',
+  transport: DummyTransport,
+  autoSessionTracking: true
+});
+/**
+ * The following code snippet will throw multiple errors, and thereby send session updates everytime an error is
+ * captured. However, the number of errors in the session should be capped at 1, regardless of how many errors there are
+ */
+for (let i = 0; i < 2; i++) {
+  Sentry.captureException(new Error('hello world'));
+}
+
+