diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index f4e1825bff5b..f2186d4ea5dc 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -228,7 +228,6 @@ export abstract class BaseClient implement protected _updateSessionFromEvent(session: Session, event: Event): void { let crashed = false; let errored = false; - let userAgent; const exceptions = event.exception && event.exception.values; if (exceptions) { @@ -243,24 +242,19 @@ export abstract class BaseClient implement } } - const user = event.user; - if (!session.userAgent) { - const headers = event.request ? event.request.headers : {}; - for (const key in headers) { - if (key.toLowerCase() === 'user-agent') { - userAgent = headers[key]; - break; - } - } - } + // A session is updated and that session update is sent in only one of the two following scenarios: + // 1. Session with non terminal status and 0 errors + an error occurred -> Will set error count to 1 and send update + // 2. Session with non terminal status and 1 error + a crash occurred -> Will set status crashed and send update + const sessionNonTerminal = session.status === SessionStatus.Ok; + const shouldUpdateAndSend = (sessionNonTerminal && session.errors === 0) || (sessionNonTerminal && crashed); - session.update({ - ...(crashed && { status: SessionStatus.Crashed }), - user, - userAgent, - errors: session.errors + Number(errored || crashed), - }); - this.captureSession(session); + if (shouldUpdateAndSend) { + session.update({ + ...(crashed && { status: SessionStatus.Crashed }), + errors: session.errors || Number(errored || crashed), + }); + this.captureSession(session); + } } /** Deliver captured session to Sentry */ diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts index 160a73e8b29f..1b14f6f5b899 100644 --- a/packages/hub/src/hub.ts +++ b/packages/hub/src/hub.ts @@ -424,10 +424,16 @@ export class Hub implements HubInterface { public startSession(context?: SessionContext): Session { const { scope, client } = this.getStackTop(); const { release, environment } = (client && client.getOptions()) || {}; + + // Will fetch userAgent if called from browser sdk + const global = getGlobalObject<{ navigator?: { userAgent?: string } }>(); + const { userAgent } = global.navigator || {}; + const session = new Session({ release, environment, ...(scope && { user: scope.getUser() }), + ...(userAgent && { userAgent }), ...context, }); diff --git a/packages/hub/src/session.ts b/packages/hub/src/session.ts index 6e2755793cdb..654754d2aee7 100644 --- a/packages/hub/src/session.ts +++ b/packages/hub/src/session.ts @@ -33,11 +33,11 @@ export class Session implements SessionInterface { // eslint-disable-next-line complexity public update(context: SessionContext = {}): void { if (context.user) { - if (context.user.ip_address) { + if (!this.ipAddress && context.user.ip_address) { this.ipAddress = context.user.ip_address; } - if (!context.did) { + if (!this.did && !context.did) { this.did = context.user.id || context.user.email || context.user.username; } } @@ -53,7 +53,7 @@ export class Session implements SessionInterface { if (context.init !== undefined) { this.init = context.init; } - if (context.did) { + if (!this.did && context.did) { this.did = `${context.did}`; } if (typeof context.started === 'number') { @@ -73,10 +73,10 @@ export class Session implements SessionInterface { if (context.environment) { this.environment = context.environment; } - if (context.ipAddress) { + if (!this.ipAddress && context.ipAddress) { this.ipAddress = context.ipAddress; } - if (context.userAgent) { + if (!this.userAgent && context.userAgent) { this.userAgent = context.userAgent; } if (typeof context.errors === 'number') { diff --git a/packages/hub/test/session.test.ts b/packages/hub/test/session.test.ts index 83dbeae3fc6e..d334becd4821 100644 --- a/packages/hub/test/session.test.ts +++ b/packages/hub/test/session.test.ts @@ -68,9 +68,9 @@ describe('Session', () => { ['sets an environment', { environment: 'staging' }, { attrs: { environment: 'staging' } }], ['sets an ipAddress', { ipAddress: '0.0.0.0' }, { attrs: { ip_address: '0.0.0.0' } }], [ - 'overwrites user ip_address did with custom ipAddress', + 'should not overwrite user ip_address did with custom ipAddress', { ipAddress: '0.0.0.0', user: { ip_address: '1.1.1.1' } }, - { attrs: { ip_address: '0.0.0.0' } }, + { attrs: { ip_address: '1.1.1.1' } }, ], ['sets an userAgent', { userAgent: 'Mozilla/5.0' }, { attrs: { user_agent: 'Mozilla/5.0' } }], ['sets errors', { errors: 3 }, { errors: 3 }], diff --git a/packages/node/package.json b/packages/node/package.json index e4aa83b0fb8a..331e25052b9c 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 & node test/manual/release-health/single-session/terminal-state-sessions-sent-once.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..d5224d467391 --- /dev/null +++ b/packages/node/test/manual/release-health/single-session/errors-in-session-capped-to-one.js @@ -0,0 +1,50 @@ +const Sentry = require('../../../../dist'); +const { + assertSessions, + constructStrippedSessionObject, + BaseDummyTransport, + validateSessionCountFunction, +} = require('../test-utils'); + +const sessionCounts = { + sessionCounter: 0, + expectedSessions: 2, +}; + +validateSessionCountFunction(sessionCounts); + +class DummyTransport extends BaseDummyTransport { + sendSession(session) { + sessionCounts.sessionCounter++; + if (sessionCounts.sessionCounter === 1) { + assertSessions(constructStrippedSessionObject(session), { + init: true, + status: 'ok', + errors: 1, + release: '1.1', + }); + } else if (sessionCounts.sessionCounter === 2) { + assertSessions(constructStrippedSessionObject(session), { + init: false, + status: 'exited', + errors: 1, + release: '1.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')); +} diff --git a/packages/node/test/manual/release-health/single-session/terminal-state-sessions-sent-once.js b/packages/node/test/manual/release-health/single-session/terminal-state-sessions-sent-once.js new file mode 100644 index 000000000000..4336e94ef7c9 --- /dev/null +++ b/packages/node/test/manual/release-health/single-session/terminal-state-sessions-sent-once.js @@ -0,0 +1,55 @@ +const Sentry = require('../../../../dist'); +const { + assertSessions, + constructStrippedSessionObject, + BaseDummyTransport, + validateSessionCountFunction, +} = require('../test-utils'); + +const sessionCounts = { + sessionCounter: 0, + expectedSessions: 1, +}; + +validateSessionCountFunction(sessionCounts); + +class DummyTransport extends BaseDummyTransport { + sendSession(session) { + sessionCounts.sessionCounter++; + + if (sessionCounts.sessionCounter === 1) { + assertSessions(constructStrippedSessionObject(session), { + init: true, + status: 'crashed', + errors: 1, + release: '1.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 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'); +}); diff --git a/packages/node/test/manual/release-health/test-utils.js b/packages/node/test/manual/release-health/test-utils.js index 7b0b93b59825..f38ff1dac57c 100644 --- a/packages/node/test/manual/release-health/test-utils.js +++ b/packages/node/test/manual/release-health/test-utils.js @@ -26,4 +26,17 @@ class BaseDummyTransport { } } -module.exports = { assertSessions, constructStrippedSessionObject, BaseDummyTransport }; +function validateSessionCountFunction(sessionCounts) { + process.on('exit', exitCode => { + const { sessionCounter, expectedSessions } = sessionCounts; + if (sessionCounter !== expectedSessions) { + console.log(`FAIL: Expected ${expectedSessions} Sessions, Received ${sessionCounter}.`); + process.exitCode = 1; + } + if ((exitCode === 0) & !process.exitCode) { + console.log('SUCCESS: All application mode sessions were sent to node transport as expected'); + } + }); +} + +module.exports = { assertSessions, constructStrippedSessionObject, BaseDummyTransport, validateSessionCountFunction };