Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(release-health): Prevent sending terminal status session updates #3701

Merged
merged 11 commits into from
Jun 21, 2021
Merged
9 changes: 7 additions & 2 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -253,13 +253,18 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
}
}

const initialSessionStatus = session.status;
const terminalStates = [SessionStatus.Crashed, SessionStatus.Abnormal];

session.update({
...(crashed && { status: SessionStatus.Crashed }),
user,
userAgent,
errors: session.errors + Number(errored || crashed),
errors: Math.max(session.errors, Number(errored || crashed)),
});
this.captureSession(session);

// Only send a session update if session was not already in a terminal
if (!terminalStates.includes(initialSessionStatus)) this.captureSession(session);
ahmedetefy marked this conversation as resolved.
Show resolved Hide resolved
}

/** Deliver captured session to Sentry */
Expand Down
2 changes: 1 addition & 1 deletion packages/node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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 & node test/manual/release-health/single-session/terminal-state-sessions-sent-once.js",
"pack": "npm pack",
"circularDepCheck": "madge --circular src/index.ts"
},
Expand Down
Original file line number Diff line number Diff line change
@@ -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');
ahmedetefy marked this conversation as resolved.
Show resolved Hide resolved
}
})

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://[email protected]/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'));
}


Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
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: 'crashed',
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://[email protected]/1337',
release: '1.1',
transport: DummyTransport,
autoSessionTracking: true
});

/**
* The following code snippet will throw an exception of `mechanism.handled` equal to `false`, and so this session
* is treated as a Crashed Session.
* However we want to ensure that once a crashed terminal state is achieved, no more session updates are sent regardless
* of whether more crashes happen or not
*/
new Promise(function(resolve, reject) {
reject();
}).then(function() {
console.log('Promise Resolved');
});

new Promise(function(resolve, reject) {
reject();
}).then(function() {
console.log('Promise Resolved');
});