From 3eee491ec4df85a0001b37789e8c589b0692975b Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 11 May 2022 15:22:41 +0200 Subject: [PATCH] ref(hub): Convert `Session` class to object and functions (#5054) convert the `Session` class to a more FP style `Session` object with additional functions that replace the methods of the class. API-wise, this makes the following changes: * `new Session(context)` => `makeSession(context)` * `session.update(context)` => `updateSession(session, context)` * `session.close(status)` => `closeSession(session, status)` `session.toJSON()` is left untouched because this method is called when the Session object is serialized to the JSON object that's sent to the Sentry servers. Additionally, this modify the session-related interfaces. Interface `Session` now describes the session object while the `SessionContext` interface is used to make modifications to the session by calling `updateSession`. Note that `updateSession` and `closeSession` mutate the session object that's passed in as a parameter. I originally wanted to return a new, mutated, object and leave the original one untouched instead of changing it. However this is unfortunately not possible (without bigger modifications) as `BaseClient` makes a modification to a session after it's already passed to `sendSession` to keep it from getting re-sent as a new session. --- MIGRATION.md | 30 ++++ packages/browser/src/exports.ts | 2 +- packages/core/src/baseclient.ts | 7 +- packages/core/src/index.ts | 1 - packages/core/test/lib/base.test.ts | 6 +- packages/core/test/mocks/client.ts | 3 +- packages/hub/src/hub.ts | 11 +- packages/hub/src/index.ts | 2 +- packages/hub/src/scope.ts | 5 +- packages/hub/src/session.ts | 256 +++++++++++++++------------- packages/hub/test/session.test.ts | 28 +-- packages/node/src/index.ts | 2 +- packages/types/src/hub.ts | 4 +- packages/types/src/session.ts | 76 ++++----- 14 files changed, 241 insertions(+), 192 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 9c2663c05b46..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,7 +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 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..f64f2b11d114 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,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 - session.update({ init: false }); + updateSession(session, { init: false }); } } @@ -343,7 +344,7 @@ export abstract class BaseClient implements Client { const shouldUpdateAndSend = (sessionNonTerminal && session.errors === 0) || (sessionNonTerminal && crashed); if (shouldUpdateAndSend) { - session.update({ + updateSession(session, { ...(crashed && { status: 'crashed' }), errors: session.errors || Number(errored || crashed), }); 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/core/test/lib/base.test.ts b/packages/core/test/lib/base.test.ts index 520cd40931a6..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, Session } from '@sentry/hub'; +import { Hub, makeSession, Scope } from '@sentry/hub'; import { Event, Span } from '@sentry/types'; import { dsnToString, logger, SentryError, SyncPromise } from '@sentry/utils'; @@ -1327,7 +1327,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 +1339,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'; diff --git a/packages/hub/src/hub.ts b/packages/hub/src/hub.ts index ac98e053faf5..7f591e44c2e8 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,7 @@ export class Hub implements HubInterface { const scope = layer && layer.scope; const session = scope && scope.getSession(); if (session) { - session.close(); + closeSession(session); } this._sendSessionUpdate(); @@ -416,7 +417,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() }), @@ -428,7 +429,7 @@ export class Hub implements HubInterface { // End existing session if there's one const currentSession = scope.getSession && scope.getSession(); if (currentSession && currentSession.status === 'ok') { - currentSession.update({ status: 'exited' }); + updateSession(currentSession, { status: 'exited' }); } this.endSession(); @@ -446,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); diff --git a/packages/hub/src/index.ts b/packages/hub/src/index.ts index 92b85f370d80..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 { Session } from './session'; +export { closeSession, makeSession, updateSession } 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..b51c620d86b0 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 }); + updateSession(this._session, { user }); } this._notifyScopeListeners(); return this; diff --git a/packages/hub/src/session.ts b/packages/hub/src/session.ts index 3206ef9306dc..fed5cfa004ad 100644 --- a/packages/hub/src/session.ts +++ b/packages/hub/src/session.ts @@ -1,136 +1,154 @@ -import { Session as SessionInterface, SessionContext, SessionStatus } from '@sentry/types'; +import { Session, SessionContext, SessionStatus } from '@sentry/types'; +import { SerializedSession } from '@sentry/types/build/types/session'; import { dropUndefinedKeys, timestampInSeconds, uuid4 } from '@sentry/utils'; /** - * @inheritdoc + * 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 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 session: Session = { + sid: uuid4(), + init: true, + timestamp: startingTime, + started: startingTime, + duration: 0, + status: 'ok', + errors: 0, + ignoreDuration: false, + toJSON: () => sessionToJSON(session), + }; + + if (context) { + updateSession(session, context); } - /** 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 session; +} - if (!this.did && !context.did) { - this.did = context.user.id || context.user.email || context.user.username; - } +/** + * 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(session: Session, context: SessionContext = {}): void { + if (context.user) { + if (!session.ipAddress && context.user.ip_address) { + session.ipAddress = context.user.ip_address; } - 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 (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; + 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, - }, - }); +/** + * 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): void { + let context = {}; + if (status) { + context = { status }; + } else if (session.status === 'ok') { + context = { status: 'exited' }; } + + updateSession(session, context); +} + +/** + * Serializes a passed session object to a JSON object with a slightly different structure. + * This is necessary because the Sentry backend requires a slightly different schema of a session + * than the one the JS SDKs use internally. + * + * @param session the session to be converted + * + * @returns a JSON object of the passed session + */ +function sessionToJSON(session: Session): SerializedSession { + return dropUndefinedKeys({ + sid: `${session.sid}`, + init: session.init, + // Make sure that sec is converted to ms for date constructor + 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: { + 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..f57267fd11c0 100644 --- a/packages/hub/test/session.test.ts +++ b/packages/hub/test/session.test.ts @@ -1,11 +1,12 @@ import { SessionContext } from '@sentry/types'; import { timestampInSeconds } from '@sentry/utils'; -import { Session } from '../src/session'; +import { closeSession, makeSession, updateSession } from '../src/session'; describe('Session', () => { it('initializes with the proper defaults', () => { - const session = new Session().toJSON(); + const newSession = makeSession(); + 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); @@ -82,36 +83,39 @@ 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 session = makeSession(); const initSessionProps = session.toJSON(); - session.update(test[1]); - expect(session.toJSON()).toEqual({ ...initSessionProps, ...DEFAULT_OUT, ...test[2] }); + updateSession(session, test[1]); + const updatedSessionProps = session.toJSON(); + + 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(); + + closeSession(session); expect(session.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'); + closeSession(session, 'abnormal'); expect(session.status).toEqual('abnormal'); }); it('only changes status ok to exited', () => { - const session = new Session(); - session.update({ status: 'crashed' }); + const session = makeSession(); + updateSession(session, { status: 'crashed' }); expect(session.status).toEqual('crashed'); - session.close(); + closeSession(session, 'crashed'); expect(session.status).toEqual('crashed'); }); }); diff --git a/packages/node/src/index.ts b/packages/node/src/index.ts index 823dc73d848e..cea9a4554ccb 100644 --- a/packages/node/src/index.ts +++ b/packages/node/src/index.ts @@ -6,6 +6,7 @@ export type { Event, EventHint, Exception, + Session, // eslint-disable-next-line deprecation/deprecation Severity, SeverityLevel, @@ -30,7 +31,6 @@ export { Hub, makeMain, Scope, - Session, startTransaction, SDK_VERSION, setContext, 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..35d5e0840a3e 100644 --- a/packages/types/src/session.ts +++ b/packages/types/src/session.ts @@ -1,60 +1,39 @@ 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; } -/** - * Session Context - */ -export interface SessionContext { - sid?: string; +export interface Session { + 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 + * 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; } +export type SessionContext = Partial; + export type SessionStatus = 'ok' | 'exited' | 'crashed' | 'abnormal'; export type RequestSessionStatus = 'ok' | 'errored' | 'crashed'; @@ -87,3 +66,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; + }; +}