From 2043e2e3edc2eaeb18e7cf3bf4c0bf1a2eabf180 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 6 May 2022 15:38:38 +0200 Subject: [PATCH 01/15] add new session Interface and SessionContext, adjust usage in code and some tests --- packages/browser/src/exports.ts | 2 +- packages/core/src/baseclient.ts | 12 +- packages/core/src/index.ts | 1 - packages/hub/src/hub.ts | 13 +- packages/hub/src/index.ts | 2 +- packages/hub/src/scope.ts | 5 +- packages/hub/src/session.ts | 248 +++++++++++++++--------------- packages/hub/test/session.test.ts | 37 ++--- packages/node/src/index.ts | 2 +- packages/node/src/sdk.ts | 2 +- packages/types/src/hub.ts | 4 +- packages/types/src/session.ts | 33 +--- 12 files changed, 175 insertions(+), 186 deletions(-) diff --git a/packages/browser/src/exports.ts b/packages/browser/src/exports.ts index ac3cea82190d..fd8ff9842a22 100644 --- a/packages/browser/src/exports.ts +++ b/packages/browser/src/exports.ts @@ -13,6 +13,7 @@ export type { Stacktrace, Thread, User, + Session, } from '@sentry/types'; export type { BrowserOptions } from './client'; @@ -31,7 +32,6 @@ export { Hub, makeMain, Scope, - Session, startTransaction, SDK_VERSION, setContext, diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 3c961218ca8d..7891c508c476 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -1,5 +1,5 @@ /* eslint-disable max-lines */ -import { Scope, Session } from '@sentry/hub'; +import { Scope, updateSession } from '@sentry/hub'; import { Client, ClientOptions, @@ -12,6 +12,7 @@ import { Integration, IntegrationClass, Outcome, + Session, SessionAggregates, Severity, SeverityLevel, @@ -199,7 +200,10 @@ export abstract class BaseClient implements Client { } else { this.sendSession(session); // After sending, we set init false to indicate it's not the first occurrence - session.update({ init: false }); + // TODO this does not have any consequence b/c we don't write the update after the session change + // disabling for now + // session.update({ init: false }); + updateSession(ses); } } @@ -343,11 +347,11 @@ export abstract class BaseClient implements Client { const shouldUpdateAndSend = (sessionNonTerminal && session.errors === 0) || (sessionNonTerminal && crashed); if (shouldUpdateAndSend) { - session.update({ + const updatedSession = updateSession(session, { ...(crashed && { status: 'crashed' }), errors: session.errors || Number(errored || crashed), }); - this.captureSession(session); + this.captureSession(updatedSession); } } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 9e4afbbc86d0..0d1a686b8e5e 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -20,7 +20,6 @@ export { Hub, makeMain, Scope, - Session, } from '@sentry/hub'; export { getEnvelopeEndpointWithUrlEncodedAuth, getReportDialogEndpoint } from './api'; export { BaseClient } from './baseclient'; diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts index ac98e053faf5..7bc9a0108304 100644 --- a/packages/hub/src/hub.ts +++ b/packages/hub/src/hub.ts @@ -12,6 +12,7 @@ import { Integration, IntegrationClass, Primitive, + Session, SessionContext, Severity, SeverityLevel, @@ -31,7 +32,7 @@ import { import { IS_DEBUG_BUILD } from './flags'; import { Scope } from './scope'; -import { Session } from './session'; +import { closeSession, makeSession, updateSession } from './session'; /** * API compatibility version of this hub. @@ -395,7 +396,8 @@ export class Hub implements HubInterface { const scope = layer && layer.scope; const session = scope && scope.getSession(); if (session) { - session.close(); + const closedSession = closeSession(session); + scope && scope.setSession(closedSession); } this._sendSessionUpdate(); @@ -416,7 +418,7 @@ export class Hub implements HubInterface { const global = getGlobalObject<{ navigator?: { userAgent?: string } }>(); const { userAgent } = global.navigator || {}; - const session = new Session({ + const session = makeSession({ release, environment, ...(scope && { user: scope.getUser() }), @@ -426,9 +428,10 @@ export class Hub implements HubInterface { if (scope) { // End existing session if there's one - const currentSession = scope.getSession && scope.getSession(); + const currentSession = scope.getSession && (scope.getSession() as Session); if (currentSession && currentSession.status === 'ok') { - currentSession.update({ status: 'exited' }); + const updatedSession = updateSession({ status: 'exited' }); + scope.setSession(updatedSession); } this.endSession(); diff --git a/packages/hub/src/index.ts b/packages/hub/src/index.ts index 92b85f370d80..c837a76f726a 100644 --- a/packages/hub/src/index.ts +++ b/packages/hub/src/index.ts @@ -1,7 +1,7 @@ export type { Carrier, Layer } from './hub'; export { addGlobalEventProcessor, Scope } from './scope'; -export { Session } from './session'; +export { updateSession, closeSession, sessionToJSON } from './session'; export { SessionFlusher } from './sessionflusher'; export { getCurrentHub, getHubFromCarrier, getMainCarrier, Hub, makeMain, setHubOnCarrier } from './hub'; export { diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 271827519792..336a9d630d19 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -13,6 +13,7 @@ import { RequestSession, Scope as ScopeInterface, ScopeContext, + Session, Severity, SeverityLevel, Span, @@ -29,7 +30,7 @@ import { } from '@sentry/utils'; import { IS_DEBUG_BUILD } from './flags'; -import { Session } from './session'; +import { updateSession } from './session'; /** * Absolute maximum number of breadcrumbs added to an event. @@ -136,7 +137,7 @@ export class Scope implements ScopeInterface { public setUser(user: User | null): this { this._user = user || {}; if (this._session) { - this._session.update({ user }); + this._session = updateSession(this._session, { user }); } this._notifyScopeListeners(); return this; diff --git a/packages/hub/src/session.ts b/packages/hub/src/session.ts index 3206ef9306dc..28f7cbc48d2f 100644 --- a/packages/hub/src/session.ts +++ b/packages/hub/src/session.ts @@ -1,136 +1,144 @@ -import { Session as SessionInterface, SessionContext, SessionStatus } from '@sentry/types'; +import { Session, SessionContext, SessionStatus } from '@sentry/types'; import { dropUndefinedKeys, timestampInSeconds, uuid4 } from '@sentry/utils'; /** - * @inheritdoc + * TODO jsdoc + * @param context + * @returns */ -export class Session implements SessionInterface { - public userAgent?: string; - public errors: number = 0; - public release?: string; - public sid: string = uuid4(); - public did?: string; - public timestamp: number; - public started: number; - public duration?: number = 0; - public status: SessionStatus = 'ok'; - public environment?: string; - public ipAddress?: string; - public init: boolean = true; - public ignoreDuration: boolean = false; +export function makeSession(context?: Omit): Session { + // Both timestamp and started are in seconds since the UNIX epoch. + const startingTime = timestampInSeconds(); - public constructor(context?: Omit) { - // Both timestamp and started are in seconds since the UNIX epoch. - const startingTime = timestampInSeconds(); - this.timestamp = startingTime; - this.started = startingTime; - if (context) { - this.update(context); - } - } + const basicSession = { + timestamp: startingTime, + started: startingTime, + }; - /** JSDoc */ - // eslint-disable-next-line complexity - public update(context: SessionContext = {}): void { - if (context.user) { - if (!this.ipAddress && context.user.ip_address) { - this.ipAddress = context.user.ip_address; - } + return { + ...basicSession, + ...(context ? updateSession(basicSession, context) : undefined), + }; +} - if (!this.did && !context.did) { - this.did = context.user.id || context.user.email || context.user.username; - } - } +/** + * TODO jsdoc + * @param session + * @param context + * @returns + */ +// eslint-disable-next-line complexity +export function updateSession(originalSession: Session, context: SessionContext = {}): Session { + const session = { ...originalSession }; - this.timestamp = context.timestamp || timestampInSeconds(); - if (context.ignoreDuration) { - this.ignoreDuration = context.ignoreDuration; - } - if (context.sid) { - // Good enough uuid validation. — Kamil - this.sid = context.sid.length === 32 ? context.sid : uuid4(); - } - if (context.init !== undefined) { - this.init = context.init; - } - if (!this.did && context.did) { - this.did = `${context.did}`; - } - if (typeof context.started === 'number') { - this.started = context.started; + if (context.user) { + if (!session.ipAddress && context.user.ip_address) { + session.ipAddress = context.user.ip_address; } - if (this.ignoreDuration) { - this.duration = undefined; - } else if (typeof context.duration === 'number') { - this.duration = context.duration; - } else { - const duration = this.timestamp - this.started; - this.duration = duration >= 0 ? duration : 0; - } - if (context.release) { - this.release = context.release; - } - if (context.environment) { - this.environment = context.environment; - } - if (!this.ipAddress && context.ipAddress) { - this.ipAddress = context.ipAddress; - } - if (!this.userAgent && context.userAgent) { - this.userAgent = context.userAgent; - } - if (typeof context.errors === 'number') { - this.errors = context.errors; - } - if (context.status) { - this.status = context.status; + + if (!session.did && !context.did) { + session.did = context.user.id || context.user.email || context.user.username; } } - /** JSDoc */ - public close(status?: Exclude): void { - if (status) { - this.update({ status }); - } else if (this.status === 'ok') { - this.update({ status: 'exited' }); - } else { - this.update(); - } + session.timestamp = context.timestamp || timestampInSeconds(); + + if (context.ignoreDuration) { + session.ignoreDuration = context.ignoreDuration; + } + if (context.sid) { + // Good enough uuid validation. — Kamil + session.sid = context.sid.length === 32 ? context.sid : uuid4(); + } + if (context.init !== undefined) { + session.init = context.init; + } + if (!session.did && context.did) { + session.did = `${context.did}`; + } + if (typeof context.started === 'number') { + session.started = context.started; + } + if (session.ignoreDuration) { + session.duration = undefined; + } else if (typeof context.duration === 'number') { + session.duration = context.duration; + } else { + const duration = session.timestamp - (session.started || 0); + session.duration = duration >= 0 ? duration : 0; + } + if (context.release) { + session.release = context.release; + } + if (context.environment) { + session.environment = context.environment; + } + if (!session.ipAddress && context.ipAddress) { + session.ipAddress = context.ipAddress; + } + if (!session.userAgent && context.userAgent) { + session.userAgent = context.userAgent; + } + if (typeof context.errors === 'number') { + session.errors = context.errors; + } + if (context.status) { + session.status = context.status; } - /** JSDoc */ - public toJSON(): { - init: boolean; - sid: string; - did?: string; - timestamp: string; - started: string; - duration?: number; - status: SessionStatus; - errors: number; - attrs?: { - release?: string; - environment?: string; - user_agent?: string; - ip_address?: string; - }; - } { - return dropUndefinedKeys({ - sid: `${this.sid}`, - init: this.init, - // Make sure that sec is converted to ms for date constructor - started: new Date(this.started * 1000).toISOString(), - timestamp: new Date(this.timestamp * 1000).toISOString(), - status: this.status, - errors: this.errors, - did: typeof this.did === 'number' || typeof this.did === 'string' ? `${this.did}` : undefined, - duration: this.duration, - attrs: { - release: this.release, - environment: this.environment, - ip_address: this.ipAddress, - user_agent: this.userAgent, - }, - }); + return session; +} + +/** + * TODO doc + * @param status + */ +export function closeSession(session: Session, status?: Exclude): Session { + let context = {}; + if (status) { + context = { status }; + } else if (session === 'ok') { + context = { status: 'exited' }; } + + return updateSession(session, context); +} + +/** + * TODO doc + * @returns + */ +export function sessionToJSON(session: Session): { + init: boolean; + sid: string; + did?: string; + timestamp: string; + started: string; + duration?: number; + status: SessionStatus; + errors: number; + attrs?: { + release?: string; + environment?: string; + user_agent?: string; + ip_address?: string; + }; +} { + return dropUndefinedKeys({ + sid: `${session.sid}`, + init: session.init !== undefined ? session.init : true, + // Make sure that sec is converted to ms for date constructor + started: new Date((session.started || 0) * 1000).toISOString(), + timestamp: new Date((session.timestamp || 0) * 1000).toISOString(), + status: session.status || 'ok', + errors: session.errors || 0, + did: typeof session.did === 'number' || typeof session.did === 'string' ? `${session.did}` : undefined, + duration: session.duration || 0, + attrs: { + release: session.release, + environment: session.environment, + ip_address: session.ipAddress, + user_agent: session.userAgent, + }, + }); } diff --git a/packages/hub/test/session.test.ts b/packages/hub/test/session.test.ts index f25e5ad4189b..22e272c8eb90 100644 --- a/packages/hub/test/session.test.ts +++ b/packages/hub/test/session.test.ts @@ -1,11 +1,11 @@ import { SessionContext } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; - -import { Session } from '../src/session'; +import { closeSession, makeSession, sessionToJSON, updateSession } from '../src/session'; describe('Session', () => { it('initializes with the proper defaults', () => { - const session = new Session().toJSON(); + const newSession = makeSession(); + const session = sessionToJSON(newSession); // Grab current year to check if we are converting from sec -> ms correctly const currentYear = new Date(timestampInSeconds() * 1000).toISOString().slice(0, 4); @@ -82,37 +82,38 @@ describe('Session', () => { // the out variable in a test explicitly refers to it. const DEFAULT_OUT = { duration: expect.any(Number), timestamp: expect.any(String) }; - const session = new Session(); - const initSessionProps = session.toJSON(); + const session = makeSession(); + const initSessionProps = sessionToJSON(session); - session.update(test[1]); - expect(session.toJSON()).toEqual({ ...initSessionProps, ...DEFAULT_OUT, ...test[2] }); + const updatedSession = updateSession(session, test[1]); + const updatedSessionProps = sessionToJSON(updatedSession); + expect(updatedSessionProps).toEqual({ ...initSessionProps, ...DEFAULT_OUT, ...test[2] }); }); }); describe('close', () => { it('exits a normal session', () => { - const session = new Session(); + const session = makeSession(); expect(session.status).toEqual('ok'); - session.close(); - expect(session.status).toEqual('exited'); + const closedSession = closeSession(session); + expect(closedSession.status).toEqual('exited'); }); it('updates session status when give status', () => { - const session = new Session(); + const session = makeSession(); expect(session.status).toEqual('ok'); - session.close('abnormal'); - expect(session.status).toEqual('abnormal'); + const closedSession = closeSession(session, 'abnormal'); + expect(closedSession.status).toEqual('abnormal'); }); it('only changes status ok to exited', () => { - const session = new Session(); - session.update({ status: 'crashed' }); - expect(session.status).toEqual('crashed'); + const session = makeSession(); + const updatedSession = updateSession(session, { status: 'crashed' }); + expect(updatedSession.status).toEqual('crashed'); - session.close(); - expect(session.status).toEqual('crashed'); + const closedSession = closeSession(session, 'abnormal'); + expect(closedSession.status).toEqual('crashed'); }); }); }); diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 823dc73d848e..c02e29256c4e 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -7,6 +7,7 @@ export type { EventHint, Exception, // eslint-disable-next-line deprecation/deprecation + Session, Severity, SeverityLevel, StackFrame, @@ -30,7 +31,6 @@ export { Hub, makeMain, Scope, - Session, startTransaction, SDK_VERSION, setContext, diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts index 34ce79ef480b..db93b68f057f 100644 --- a/packages/node/src/sdk.ts +++ b/packages/node/src/sdk.ts @@ -249,6 +249,6 @@ function startSessionTracking(): void { // Terminal Status i.e. Exited or Crashed because // "When a session is moved away from ok it must not be updated anymore." // Ref: https://develop.sentry.dev/sdk/sessions/ - if (session && !terminalStates.includes(session.status)) hub.endSession(); + if (session && session.status && !terminalStates.includes(session.status)) hub.endSession(); }); } diff --git a/packages/types/src/hub.ts b/packages/types/src/hub.ts index 9a67968a4fc1..a34d4372e08d 100644 --- a/packages/types/src/hub.ts +++ b/packages/types/src/hub.ts @@ -5,7 +5,7 @@ import { Extra, Extras } from './extra'; import { Integration, IntegrationClass } from './integration'; import { Primitive } from './misc'; import { Scope } from './scope'; -import { Session, SessionContext } from './session'; +import { Session } from './session'; import { Severity, SeverityLevel } from './severity'; import { CustomSamplingContext, Transaction, TransactionContext } from './transaction'; import { User } from './user'; @@ -215,7 +215,7 @@ export interface Hub { * * @returns The session which was just started */ - startSession(context?: SessionContext): Session; + startSession(context?: Session): Session; /** * Ends the session that lives on the current scope and sends it to Sentry diff --git a/packages/types/src/session.ts b/packages/types/src/session.ts index 237e4c41808d..ff6a0d69f2a5 100644 --- a/packages/types/src/session.ts +++ b/packages/types/src/session.ts @@ -1,34 +1,5 @@ import { User } from './user'; -/** - * @inheritdoc - */ -export interface Session extends SessionContext { - /** JSDoc */ - update(context?: SessionContext): void; - - /** JSDoc */ - close(status?: SessionStatus): void; - - /** JSDoc */ - toJSON(): { - init: boolean; - sid: string; - did?: string; - timestamp: string; - started: string; - duration?: number; - status: SessionStatus; - errors: number; - attrs?: { - release?: string; - environment?: string; - user_agent?: string; - ip_address?: string; - }; - }; -} - export interface RequestSession { status?: RequestSessionStatus; } @@ -36,7 +7,7 @@ export interface RequestSession { /** * Session Context */ -export interface SessionContext { +export interface Session { sid?: string; did?: string; init?: boolean; @@ -55,6 +26,8 @@ export interface SessionContext { ignoreDuration?: boolean; } +export type SessionContext = Partial; + export type SessionStatus = 'ok' | 'exited' | 'crashed' | 'abnormal'; export type RequestSessionStatus = 'ok' | 'errored' | 'crashed'; From c8fa82ec75b3a6a0c57a863887512855d344dfce Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 6 May 2022 15:49:28 +0200 Subject: [PATCH 02/15] fix hub tests --- packages/core/src/baseclient.ts | 5 +++-- packages/hub/src/session.ts | 14 ++++++++++---- packages/hub/test/session.test.ts | 2 +- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 7891c508c476..3604a8da570e 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -200,10 +200,11 @@ export abstract class BaseClient implements Client { } else { this.sendSession(session); // After sending, we set init false to indicate it's not the first occurrence - // TODO this does not have any consequence b/c we don't write the update after the session change + + // TODO(v7) this does not have any consequence b/c we don't write the update after the session change // disabling for now // session.update({ init: false }); - updateSession(ses); + // updateSession(session, { init: false }); } } diff --git a/packages/hub/src/session.ts b/packages/hub/src/session.ts index 28f7cbc48d2f..806a9c2096e8 100644 --- a/packages/hub/src/session.ts +++ b/packages/hub/src/session.ts @@ -10,14 +10,20 @@ export function makeSession(context?: Omit // Both timestamp and started are in seconds since the UNIX epoch. const startingTime = timestampInSeconds(); - const basicSession = { + const basicSession: Session = { + errors: 0, + sid: uuid4(), timestamp: startingTime, started: startingTime, + duration: 0, + status: 'ok', + init: true, + ignoreDuration: false, }; return { ...basicSession, - ...(context ? updateSession(basicSession, context) : undefined), + ...(context ? updateSession(basicSession, context) : {}), }; } @@ -97,7 +103,7 @@ export function closeSession(session: Session, status?: Exclude { const updatedSession = updateSession(session, { status: 'crashed' }); expect(updatedSession.status).toEqual('crashed'); - const closedSession = closeSession(session, 'abnormal'); + const closedSession = closeSession(session, 'crashed'); expect(closedSession.status).toEqual('crashed'); }); }); From 2a980611a2abf10066997af97246089b61c44543 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 6 May 2022 15:56:56 +0200 Subject: [PATCH 03/15] fix core unit tests --- packages/core/test/lib/base.test.ts | 7 ++++--- packages/core/test/mocks/client.ts | 3 +-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 520cd40931a6..34aab3bcee45 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -1,4 +1,5 @@ -import { Hub, Scope, Session } from '@sentry/hub'; +import { Hub, Scope } from '@sentry/hub'; +import { makeSession } from '@sentry/hub/src/session'; import { Event, Span } from '@sentry/types'; import { dsnToString, logger, SentryError, SyncPromise } from '@sentry/utils'; @@ -1327,7 +1328,7 @@ describe('BaseClient', () => { const options = getDefaultTestClientOptions({ dsn: PUBLIC_DSN }); const client = new TestClient(options); - const session = new Session({ release: 'test' }); + const session = makeSession({ release: 'test' }); client.captureSession(session); @@ -1339,7 +1340,7 @@ describe('BaseClient', () => { const options = getDefaultTestClientOptions({ enabled: false, dsn: PUBLIC_DSN }); const client = new TestClient(options); - const session = new Session({ release: 'test' }); + const session = makeSession({ release: 'test' }); client.captureSession(session); diff --git a/packages/core/test/mocks/client.ts b/packages/core/test/mocks/client.ts index d46eb000d480..9caef7b0e0cf 100644 --- a/packages/core/test/mocks/client.ts +++ b/packages/core/test/mocks/client.ts @@ -1,5 +1,4 @@ -import { Session } from '@sentry/hub'; -import { ClientOptions, Event, Integration, Outcome, Severity, SeverityLevel } from '@sentry/types'; +import { ClientOptions, Event, Integration, Outcome, Session, Severity, SeverityLevel } from '@sentry/types'; import { resolvedSyncPromise } from '@sentry/utils'; import { BaseClient } from '../../src/baseclient'; From 2356f3ec78a51107db07f6f0c88190cf7bb7039e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 9 May 2022 13:45:49 +0200 Subject: [PATCH 04/15] re-add toJSON to Session interface, mutate session update/close functions --- packages/core/src/baseclient.ts | 10 ++-- packages/hub/src/hub.ts | 6 +-- packages/hub/src/scope.ts | 2 +- packages/hub/src/session.ts | 82 ++++++++++++++++--------------- packages/hub/test/session.test.ts | 22 +++++---- packages/types/src/session.ts | 25 ++++++++++ 6 files changed, 86 insertions(+), 61 deletions(-) diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 3604a8da570e..f64f2b11d114 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -200,11 +200,7 @@ export abstract class BaseClient implements Client { } else { this.sendSession(session); // After sending, we set init false to indicate it's not the first occurrence - - // TODO(v7) this does not have any consequence b/c we don't write the update after the session change - // disabling for now - // session.update({ init: false }); - // updateSession(session, { init: false }); + updateSession(session, { init: false }); } } @@ -348,11 +344,11 @@ export abstract class BaseClient implements Client { const shouldUpdateAndSend = (sessionNonTerminal && session.errors === 0) || (sessionNonTerminal && crashed); if (shouldUpdateAndSend) { - const updatedSession = updateSession(session, { + updateSession(session, { ...(crashed && { status: 'crashed' }), errors: session.errors || Number(errored || crashed), }); - this.captureSession(updatedSession); + this.captureSession(session); } } diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts index 7bc9a0108304..e83e47429424 100644 --- a/packages/hub/src/hub.ts +++ b/packages/hub/src/hub.ts @@ -396,8 +396,7 @@ export class Hub implements HubInterface { const scope = layer && layer.scope; const session = scope && scope.getSession(); if (session) { - const closedSession = closeSession(session); - scope && scope.setSession(closedSession); + closeSession(session); } this._sendSessionUpdate(); @@ -430,8 +429,7 @@ export class Hub implements HubInterface { // End existing session if there's one const currentSession = scope.getSession && (scope.getSession() as Session); if (currentSession && currentSession.status === 'ok') { - const updatedSession = updateSession({ status: 'exited' }); - scope.setSession(updatedSession); + updateSession(currentSession, { status: 'exited' }); } this.endSession(); diff --git a/packages/hub/src/scope.ts b/packages/hub/src/scope.ts index 336a9d630d19..b51c620d86b0 100644 --- a/packages/hub/src/scope.ts +++ b/packages/hub/src/scope.ts @@ -137,7 +137,7 @@ export class Scope implements ScopeInterface { public setUser(user: User | null): this { this._user = user || {}; if (this._session) { - this._session = updateSession(this._session, { user }); + updateSession(this._session, { user }); } this._notifyScopeListeners(); return this; diff --git a/packages/hub/src/session.ts b/packages/hub/src/session.ts index 806a9c2096e8..b6292d3039c3 100644 --- a/packages/hub/src/session.ts +++ b/packages/hub/src/session.ts @@ -1,16 +1,20 @@ import { Session, SessionContext, SessionStatus } from '@sentry/types'; +import { SerializedSession } from '@sentry/types/build/types/session'; import { dropUndefinedKeys, timestampInSeconds, uuid4 } from '@sentry/utils'; /** - * TODO jsdoc - * @param context - * @returns + * Creates a new `Session` object by setting certain default parameters. If optional @param context + * is passed, the passed properties are applied to the session object. + * + * @param context (optional) additional properties to be applied to the returned session object + * + * @returns a new `Session` object */ export function makeSession(context?: Omit): Session { // Both timestamp and started are in seconds since the UNIX epoch. const startingTime = timestampInSeconds(); - const basicSession: Session = { + const session: Session = { errors: 0, sid: uuid4(), timestamp: startingTime, @@ -19,24 +23,29 @@ export function makeSession(context?: Omit status: 'ok', init: true, ignoreDuration: false, + toJSON: () => sessionToJSON(session), }; - return { - ...basicSession, - ...(context ? updateSession(basicSession, context) : {}), - }; + if (context) { + updateSession(session, context); + } + + return session; } /** - * TODO jsdoc - * @param session - * @param context - * @returns + * Updates a session object with the properties passed in the context. + * + * Note that this function mutates the passed object and returns void. + * (Had to do this instead of returning a new and updated session because closing and sending a session + * makes an update to the session after it was passed to the sending logic. + * @see BaseClient.captureSession ) + * + * @param session the `Session` to update + * @param context the `SessionContext` holding the properties that should be updated in @param session */ // eslint-disable-next-line complexity -export function updateSession(originalSession: Session, context: SessionContext = {}): Session { - const session = { ...originalSession }; - +export function updateSession(session: Session, context: SessionContext = {}): void { if (context.user) { if (!session.ipAddress && context.user.ip_address) { session.ipAddress = context.user.ip_address; @@ -91,15 +100,20 @@ export function updateSession(originalSession: Session, context: SessionContext if (context.status) { session.status = context.status; } - - return session; } /** - * TODO doc - * @param status + * Closes a session by setting its status and updating the session object with it. + * Internally calls `updateSession` to update the passed session object. + * + * Note that this function mutates the passed session (@see updateSession for explanation). + * + * @param session the `Session` object to be closed + * @param status the `SessionStatus` with which the session was closed. If you don't pass a status, + * this function will keep the previously set status, unless it was `'ok'` in which case + * it is changed to `'exited'`. */ -export function closeSession(session: Session, status?: Exclude): Session { +export function closeSession(session: Session, status?: Exclude): void { let context = {}; if (status) { context = { status }; @@ -107,29 +121,19 @@ export function closeSession(session: Session, status?: Exclude { const session = makeSession(); const initSessionProps = sessionToJSON(session); - const updatedSession = updateSession(session, test[1]); - const updatedSessionProps = sessionToJSON(updatedSession); + updateSession(session, test[1]); + const updatedSessionProps = sessionToJSON(session); + expect(updatedSessionProps).toEqual({ ...initSessionProps, ...DEFAULT_OUT, ...test[2] }); }); }); @@ -95,25 +96,26 @@ describe('Session', () => { it('exits a normal session', () => { const session = makeSession(); expect(session.status).toEqual('ok'); - const closedSession = closeSession(session); - expect(closedSession.status).toEqual('exited'); + + closeSession(session); + expect(session.status).toEqual('exited'); }); it('updates session status when give status', () => { const session = makeSession(); expect(session.status).toEqual('ok'); - const closedSession = closeSession(session, 'abnormal'); - expect(closedSession.status).toEqual('abnormal'); + closeSession(session, 'abnormal'); + expect(session.status).toEqual('abnormal'); }); it('only changes status ok to exited', () => { const session = makeSession(); - const updatedSession = updateSession(session, { status: 'crashed' }); - expect(updatedSession.status).toEqual('crashed'); + updateSession(session, { status: 'crashed' }); + expect(session.status).toEqual('crashed'); - const closedSession = closeSession(session, 'crashed'); - expect(closedSession.status).toEqual('crashed'); + closeSession(session, 'crashed'); + expect(session.status).toEqual('crashed'); }); }); }); diff --git a/packages/types/src/session.ts b/packages/types/src/session.ts index ff6a0d69f2a5..57cc8584c5ff 100644 --- a/packages/types/src/session.ts +++ b/packages/types/src/session.ts @@ -7,6 +7,7 @@ export interface RequestSession { /** * Session Context */ +// TODO: make properties required that were set by ctor export interface Session { sid?: string; did?: string; @@ -24,6 +25,13 @@ export interface Session { errors?: number; user?: User | null; ignoreDuration?: boolean; + + /** + * Overrides default JSON serialization of the Session because + * the Sentry servers expect a slightly different schema of a session + * which is described in the interface @see SerializedSession in this file. + */ + toJSON(): SerializedSession; } export type SessionContext = Partial; @@ -60,3 +68,20 @@ export interface AggregationCounts { exited?: number; crashed?: number; } + +export interface SerializedSession { + init: boolean; + sid: string; + did?: string; + timestamp: string; + started: string; + duration?: number; + status: SessionStatus; + errors: number; + attrs?: { + release?: string; + environment?: string; + user_agent?: string; + ip_address?: string; + }; +} From 595384880ce68cbc3e13f2980bc1494730ff5f9d Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 9 May 2022 14:03:53 +0200 Subject: [PATCH 05/15] make default-intitialized properties required in `Session` interface --- packages/hub/src/session.ts | 14 +++++++------- packages/types/src/session.ts | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/hub/src/session.ts b/packages/hub/src/session.ts index b6292d3039c3..088b39291d1d 100644 --- a/packages/hub/src/session.ts +++ b/packages/hub/src/session.ts @@ -15,13 +15,13 @@ export function makeSession(context?: Omit const startingTime = timestampInSeconds(); const session: Session = { - errors: 0, sid: uuid4(), + init: true, timestamp: startingTime, started: startingTime, duration: 0, status: 'ok', - init: true, + errors: 0, ignoreDuration: false, toJSON: () => sessionToJSON(session), }; @@ -79,7 +79,7 @@ export function updateSession(session: Session, context: SessionContext = {}): v } else if (typeof context.duration === 'number') { session.duration = context.duration; } else { - const duration = session.timestamp - (session.started || 0); + const duration = session.timestamp - session.started; session.duration = duration >= 0 ? duration : 0; } if (context.release) { @@ -138,10 +138,10 @@ export function sessionToJSON(session: Session): SerializedSession { sid: `${session.sid}`, init: session.init !== undefined ? session.init : true, // Make sure that sec is converted to ms for date constructor - started: new Date((session.started || 0) * 1000).toISOString(), - timestamp: new Date((session.timestamp || 0) * 1000).toISOString(), - status: session.status || 'ok', - errors: session.errors || 0, + started: new Date(session.started * 1000).toISOString(), + timestamp: new Date(session.timestamp * 1000).toISOString(), + status: session.status, + errors: session.errors, did: typeof session.did === 'number' || typeof session.did === 'string' ? `${session.did}` : undefined, duration: session.duration, attrs: { diff --git a/packages/types/src/session.ts b/packages/types/src/session.ts index 57cc8584c5ff..adaaca923a37 100644 --- a/packages/types/src/session.ts +++ b/packages/types/src/session.ts @@ -9,22 +9,22 @@ export interface RequestSession { */ // TODO: make properties required that were set by ctor export interface Session { - sid?: string; + sid: string; did?: string; - init?: boolean; + init: boolean; // seconds since the UNIX epoch - timestamp?: number; + timestamp: number; // seconds since the UNIX epoch - started?: number; + started: number; duration?: number; - status?: SessionStatus; + status: SessionStatus; release?: string; environment?: string; userAgent?: string; ipAddress?: string; - errors?: number; + errors: number; user?: User | null; - ignoreDuration?: boolean; + ignoreDuration: boolean; /** * Overrides default JSON serialization of the Session because From 8971da4a5a90f86a243f845857ca48d815211cb4 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 9 May 2022 14:29:30 +0200 Subject: [PATCH 06/15] fix linter error (hub) --- packages/hub/test/session.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/hub/test/session.test.ts b/packages/hub/test/session.test.ts index e227b5150eb2..dd6f4e3414b4 100644 --- a/packages/hub/test/session.test.ts +++ b/packages/hub/test/session.test.ts @@ -1,5 +1,6 @@ import { SessionContext } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; + import { closeSession, makeSession, sessionToJSON, updateSession } from '../src/session'; describe('Session', () => { From 607b2bb1189b8ab504e8f7e7154a9692dafdaba1 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 9 May 2022 15:01:46 +0200 Subject: [PATCH 07/15] fix linter error (node) --- packages/node/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index c02e29256c4e..cea9a4554ccb 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -6,8 +6,8 @@ export type { Event, EventHint, Exception, - // eslint-disable-next-line deprecation/deprecation Session, + // eslint-disable-next-line deprecation/deprecation Severity, SeverityLevel, StackFrame, From 2c65f63d6a3ad027d8070c117d8cdd5fc03c492c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 9 May 2022 15:45:06 +0200 Subject: [PATCH 08/15] do not sessionToJSON --- packages/hub/src/index.ts | 2 +- packages/hub/src/session.ts | 2 +- packages/hub/test/session.test.ts | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/hub/src/index.ts b/packages/hub/src/index.ts index c837a76f726a..11a61a9830d9 100644 --- a/packages/hub/src/index.ts +++ b/packages/hub/src/index.ts @@ -1,7 +1,7 @@ export type { Carrier, Layer } from './hub'; export { addGlobalEventProcessor, Scope } from './scope'; -export { updateSession, closeSession, sessionToJSON } from './session'; +export { updateSession, closeSession } from './session'; export { SessionFlusher } from './sessionflusher'; export { getCurrentHub, getHubFromCarrier, getMainCarrier, Hub, makeMain, setHubOnCarrier } from './hub'; export { diff --git a/packages/hub/src/session.ts b/packages/hub/src/session.ts index 088b39291d1d..6bbca1dfb6d8 100644 --- a/packages/hub/src/session.ts +++ b/packages/hub/src/session.ts @@ -133,7 +133,7 @@ export function closeSession(session: Session, status?: Exclude { it('initializes with the proper defaults', () => { const newSession = makeSession(); - const session = sessionToJSON(newSession); + const session = newSession.toJSON(); // Grab current year to check if we are converting from sec -> ms correctly const currentYear = new Date(timestampInSeconds() * 1000).toISOString().slice(0, 4); @@ -84,10 +84,10 @@ describe('Session', () => { const DEFAULT_OUT = { duration: expect.any(Number), timestamp: expect.any(String) }; const session = makeSession(); - const initSessionProps = sessionToJSON(session); + const initSessionProps = session.toJSON(); updateSession(session, test[1]); - const updatedSessionProps = sessionToJSON(session); + const updatedSessionProps = session.toJSON(); expect(updatedSessionProps).toEqual({ ...initSessionProps, ...DEFAULT_OUT, ...test[2] }); }); From d27fbce53c5c2a379a81148be7b2e27ca3bf8e3b Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 9 May 2022 15:49:15 +0200 Subject: [PATCH 09/15] cleanup, remove done todo --- packages/types/src/session.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/types/src/session.ts b/packages/types/src/session.ts index adaaca923a37..35d5e0840a3e 100644 --- a/packages/types/src/session.ts +++ b/packages/types/src/session.ts @@ -4,10 +4,6 @@ export interface RequestSession { status?: RequestSessionStatus; } -/** - * Session Context - */ -// TODO: make properties required that were set by ctor export interface Session { sid: string; did?: string; @@ -30,6 +26,8 @@ export interface Session { * Overrides default JSON serialization of the Session because * the Sentry servers expect a slightly different schema of a session * which is described in the interface @see SerializedSession in this file. + * + * @return a Sentry-backend conforming JSON object of the session */ toJSON(): SerializedSession; } From 59dd0ca0e5e86c95153589c14b77ea18c8942df2 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 9 May 2022 17:50:29 +0200 Subject: [PATCH 10/15] remove unnecessary definedness checks after interface update --- packages/hub/src/session.ts | 2 +- packages/node/src/sdk.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/hub/src/session.ts b/packages/hub/src/session.ts index 6bbca1dfb6d8..fed5cfa004ad 100644 --- a/packages/hub/src/session.ts +++ b/packages/hub/src/session.ts @@ -136,7 +136,7 @@ export function closeSession(session: Session, status?: Exclude Date: Mon, 9 May 2022 17:54:28 +0200 Subject: [PATCH 11/15] add migration note --- MIGRATION.md | 1 + 1 file changed, 1 insertion(+) diff --git a/MIGRATION.md b/MIGRATION.md index 9c2663c05b46..3a0e5ef9c22c 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -314,6 +314,7 @@ For our efforts to reduce bundle size of the SDK we had to remove and refactor p - Remove `SDK_NAME` export from `@sentry/browser`, `@sentry/node`, `@sentry/tracing` and `@sentry/vue` packages. - Removed `eventStatusFromHttpCode` to save on bundle size. - Replace `BrowserTracing` `maxTransactionDuration` option with `finalTimeout` option +- Replace `Session` class with a session object and functions (see [#5054](https://github.com/getsentry/sentry-javascript/pull/5054)). ## Sentry Angular SDK Changes From 3967cf90cc9a158b483c0f6bfb3a519e4bdbdb84 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Mon, 9 May 2022 18:38:22 +0200 Subject: [PATCH 12/15] Add "Session Changes" section to Migration docs --- MIGRATION.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/MIGRATION.md b/MIGRATION.md index 3a0e5ef9c22c..9a27dd6f3f26 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -292,6 +292,32 @@ changes in v7. They will be removed in the next major release which is why we st corresponding string literals. Here's how to adjust [`Severity`](#severity-severitylevel-and-severitylevels) and [`SpanStatus`](#spanstatus). +## Session Changes + +Note: These changes are not relevant for the majority of Sentry users but if you are building an +SDK on top of the Javascript SDK, you might need to make some adaptions. +The internal `Session` class was refactored and replaced with a more functional approach in +[#5054](https://github.com/getsentry/sentry-javascript/pull/5054). +Instead of the class, we now export a `Session` interface from `@sentry/types` and three utility functions +to create and update a `Session` object from `@sentry/hub`. +This short example shows what has changed and how to deal with the new functions: + +```js +// New in v7: +import { makeSession, updateSession, closeSession } from '@sentry/hub'; + +const session = makeSession({ release: 'v1.0' }); +updateSession(session, { environment: 'prod' }); +closeSession(session, 'ok'); + +// Before: +import { Session } from '@sentry/hub'; + +const session = new Session({ release: 'v1.0' }); +session.update({ environment: 'prod' }); +session.close('ok'); +``` + ## General API Changes For our efforts to reduce bundle size of the SDK we had to remove and refactor parts of the package which introduced a few changes to the API: @@ -313,8 +339,11 @@ For our efforts to reduce bundle size of the SDK we had to remove and refactor p - Rename `UserAgent` integration to `HttpContext`. (see [#5027](https://github.com/getsentry/sentry-javascript/pull/5027)) - Remove `SDK_NAME` export from `@sentry/browser`, `@sentry/node`, `@sentry/tracing` and `@sentry/vue` packages. - Removed `eventStatusFromHttpCode` to save on bundle size. +<<<<<<< HEAD - Replace `BrowserTracing` `maxTransactionDuration` option with `finalTimeout` option - Replace `Session` class with a session object and functions (see [#5054](https://github.com/getsentry/sentry-javascript/pull/5054)). +======= +>>>>>>> 5c81e32c4 (Add "Session Changes" section to Migration docs) ## Sentry Angular SDK Changes From 7b60e2274ca298d188af92849c471c94babc3e9f Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 11 May 2022 10:30:07 +0200 Subject: [PATCH 13/15] export all Session functions from hub index, remove unnecessary Session cast --- packages/core/test/lib/base.test.ts | 3 +-- packages/hub/src/hub.ts | 2 +- packages/hub/src/index.ts | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 34aab3bcee45..0f086b40c31a 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -1,5 +1,4 @@ -import { Hub, Scope } from '@sentry/hub'; -import { makeSession } from '@sentry/hub/src/session'; +import { Hub, Scope, makeSession } from '@sentry/hub'; import { Event, Span } from '@sentry/types'; import { dsnToString, logger, SentryError, SyncPromise } from '@sentry/utils'; diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts index e83e47429424..636116b47816 100644 --- a/packages/hub/src/hub.ts +++ b/packages/hub/src/hub.ts @@ -427,7 +427,7 @@ export class Hub implements HubInterface { if (scope) { // End existing session if there's one - const currentSession = scope.getSession && (scope.getSession() as Session); + const currentSession = scope.getSession && scope.getSession(); if (currentSession && currentSession.status === 'ok') { updateSession(currentSession, { status: 'exited' }); } diff --git a/packages/hub/src/index.ts b/packages/hub/src/index.ts index 11a61a9830d9..16c4f3680dd8 100644 --- a/packages/hub/src/index.ts +++ b/packages/hub/src/index.ts @@ -1,7 +1,7 @@ export type { Carrier, Layer } from './hub'; export { addGlobalEventProcessor, Scope } from './scope'; -export { updateSession, closeSession } from './session'; +export { closeSession, makeSession, updateSession } from './session'; export { SessionFlusher } from './sessionflusher'; export { getCurrentHub, getHubFromCarrier, getMainCarrier, Hub, makeMain, setHubOnCarrier } from './hub'; export { From f4e60c1c44ed0cb3e0710673c65284e7b2b1bdd9 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 11 May 2022 10:50:44 +0200 Subject: [PATCH 14/15] fix linter error in core --- packages/core/test/lib/base.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 0f086b40c31a..7a5431d1f9cb 100644 --- a/packages/core/test/lib/base.test.ts +++ b/packages/core/test/lib/base.test.ts @@ -1,4 +1,4 @@ -import { Hub, Scope, makeSession } from '@sentry/hub'; +import { Hub, makeSession, Scope } from '@sentry/hub'; import { Event, Span } from '@sentry/types'; import { dsnToString, logger, SentryError, SyncPromise } from '@sentry/utils'; From 6dc3e506efe4b79c85d879f83abf46be58cf0277 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 11 May 2022 13:11:09 +0200 Subject: [PATCH 15/15] remove `scope.getSession` definedness check --- packages/hub/src/hub.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts index 636116b47816..7f591e44c2e8 100644 --- a/packages/hub/src/hub.ts +++ b/packages/hub/src/hub.ts @@ -447,7 +447,7 @@ export class Hub implements HubInterface { const { scope, client } = this.getStackTop(); if (!scope) return; - const session = scope.getSession && scope.getSession(); + const session = scope.getSession(); if (session) { if (client && client.captureSession) { client.captureSession(session);