Skip to content

Commit

Permalink
ref(hub): Convert Session class to object and functions (#5054)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Lms24 authored and AbhiPrasad committed May 30, 2022
1 parent 6d2e788 commit 3eee491
Show file tree
Hide file tree
Showing 14 changed files with 241 additions and 192 deletions.
30 changes: 30 additions & 0 deletions MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion packages/browser/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export type {
Stacktrace,
Thread,
User,
Session,
} from '@sentry/types';

export type { BrowserOptions } from './client';
Expand All @@ -31,7 +32,6 @@ export {
Hub,
makeMain,
Scope,
Session,
startTransaction,
SDK_VERSION,
setContext,
Expand Down
7 changes: 4 additions & 3 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable max-lines */
import { Scope, Session } from '@sentry/hub';
import { Scope, updateSession } from '@sentry/hub';
import {
Client,
ClientOptions,
Expand All @@ -12,6 +12,7 @@ import {
Integration,
IntegrationClass,
Outcome,
Session,
SessionAggregates,
Severity,
SeverityLevel,
Expand Down Expand Up @@ -199,7 +200,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
} 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 });
}
}

Expand Down Expand Up @@ -343,7 +344,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
const shouldUpdateAndSend = (sessionNonTerminal && session.errors === 0) || (sessionNonTerminal && crashed);

if (shouldUpdateAndSend) {
session.update({
updateSession(session, {
...(crashed && { status: 'crashed' }),
errors: session.errors || Number(errored || crashed),
});
Expand Down
1 change: 0 additions & 1 deletion packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export {
Hub,
makeMain,
Scope,
Session,
} from '@sentry/hub';
export { getEnvelopeEndpointWithUrlEncodedAuth, getReportDialogEndpoint } from './api';
export { BaseClient } from './baseclient';
Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/lib/base.test.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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);

Expand All @@ -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);

Expand Down
3 changes: 1 addition & 2 deletions packages/core/test/mocks/client.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
11 changes: 6 additions & 5 deletions packages/hub/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
Integration,
IntegrationClass,
Primitive,
Session,
SessionContext,
Severity,
SeverityLevel,
Expand All @@ -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.
Expand Down Expand Up @@ -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();

Expand All @@ -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() }),
Expand All @@ -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();

Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion packages/hub/src/index.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
5 changes: 3 additions & 2 deletions packages/hub/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
RequestSession,
Scope as ScopeInterface,
ScopeContext,
Session,
Severity,
SeverityLevel,
Span,
Expand All @@ -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.
Expand Down Expand Up @@ -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;
Expand Down
Loading

0 comments on commit 3eee491

Please sign in to comment.