Skip to content

Commit

Permalink
ref(types): deprecate session status
Browse files Browse the repository at this point in the history
  • Loading branch information
JonasBa committed Dec 16, 2021
1 parent 84e665a commit f5db0d7
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 38 deletions.
5 changes: 2 additions & 3 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import {
Integration,
IntegrationClass,
Options,
SessionStatus,
SeverityLevel,
Transport,
} from '@sentry/types';
Expand Down Expand Up @@ -267,12 +266,12 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
// 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 sessionNonTerminal = session.status === 'ok';
const shouldUpdateAndSend = (sessionNonTerminal && session.errors === 0) || (sessionNonTerminal && crashed);

if (shouldUpdateAndSend) {
session.update({
...(crashed && { status: SessionStatus.Crashed }),
...(crashed && { status: 'crashed' }),
errors: session.errors || Number(errored || crashed),
});
this.captureSession(session);
Expand Down
5 changes: 2 additions & 3 deletions packages/hub/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import {
IntegrationClass,
Primitive,
SessionContext,
SessionStatus,
SeverityLevel,
Span,
SpanContext,
Expand Down Expand Up @@ -451,8 +450,8 @@ export class Hub implements HubInterface {
if (scope) {
// End existing session if there's one
const currentSession = scope.getSession && scope.getSession();
if (currentSession && currentSession.status === SessionStatus.Ok) {
currentSession.update({ status: SessionStatus.Exited });
if (currentSession && currentSession.status === 'ok') {
currentSession.update({ status: 'exited' });
}
this.endSession();

Expand Down
8 changes: 4 additions & 4 deletions packages/hub/src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export class Session implements SessionInterface {
public timestamp: number;
public started: number;
public duration?: number = 0;
public status: SessionStatus = SessionStatus.Ok;
public status: SessionStatus = 'ok';
public environment?: string;
public ipAddress?: string;
public init: boolean = true;
Expand Down Expand Up @@ -88,11 +88,11 @@ export class Session implements SessionInterface {
}

/** JSDoc */
public close(status?: Exclude<SessionStatus, SessionStatus.Ok>): void {
public close(status?: Exclude<SessionStatus, 'ok'>): void {
if (status) {
this.update({ status });
} else if (this.status === SessionStatus.Ok) {
this.update({ status: SessionStatus.Exited });
} else if (this.status === 'ok') {
this.update({ status: 'exited' });
} else {
this.update();
}
Expand Down
22 changes: 11 additions & 11 deletions packages/hub/test/session.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SessionContext, SessionStatus } from '@sentry/types';
import { SessionContext } from '@sentry/types';
import { timestampInSeconds } from '@sentry/utils';

import { Session } from '../src/session';
Expand All @@ -16,7 +16,7 @@ describe('Session', () => {
init: true,
sid: expect.any(String),
started: expect.stringMatching(currentYear),
status: SessionStatus.Ok,
status: 'ok',
timestamp: expect.stringMatching(currentYear),
});

Expand Down Expand Up @@ -74,7 +74,7 @@ describe('Session', () => {
],
['sets an userAgent', { userAgent: 'Mozilla/5.0' }, { attrs: { user_agent: 'Mozilla/5.0' } }],
['sets errors', { errors: 3 }, { errors: 3 }],
['sets status', { status: SessionStatus.Crashed }, { status: SessionStatus.Crashed }],
['sets status', { status: 'crashed' }, { status: 'crashed' }],
];

test.each(table)('%s', (...test) => {
Expand All @@ -93,26 +93,26 @@ describe('Session', () => {
describe('close', () => {
it('exits a normal session', () => {
const session = new Session();
expect(session.status).toEqual(SessionStatus.Ok);
expect(session.status).toEqual('ok');
session.close();
expect(session.status).toEqual(SessionStatus.Exited);
expect(session.status).toEqual('exited');
});

it('updates session status when give status', () => {
const session = new Session();
expect(session.status).toEqual(SessionStatus.Ok);
expect(session.status).toEqual('ok');

session.close(SessionStatus.Abnormal);
expect(session.status).toEqual(SessionStatus.Abnormal);
session.close('abnormal');
expect(session.status).toEqual('abnormal');
});

it('only changes status ok to exited', () => {
const session = new Session();
session.update({ status: SessionStatus.Crashed });
expect(session.status).toEqual(SessionStatus.Crashed);
session.update({ status: 'crashed' });
expect(session.status).toEqual('crashed');

session.close();
expect(session.status).toEqual(SessionStatus.Crashed);
expect(session.status).toEqual('crashed');
});
});
});
2 changes: 1 addition & 1 deletion packages/node/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ function startSessionTracking(): void {
// Ref: https://nodejs.org/api/process.html#process_event_beforeexit
process.on('beforeExit', () => {
const session = hub.getScope()?.getSession();
const terminalStates = [SessionStatus.Exited, SessionStatus.Crashed];
const terminalStates: SessionStatus[] = ['exited', 'crashed'];
// Only call endSession, if the Session exists on Scope and SessionStatus is not a
// Terminal Status i.e. Exited or Crashed because
// "When a session is moved away from ok it must not be updated anymore."
Expand Down
4 changes: 2 additions & 2 deletions packages/node/test/transports/http.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Session } from '@sentry/hub';
import { Event, SessionAggregates, SessionStatus, TransportOptions } from '@sentry/types';
import { Event, SessionAggregates, TransportOptions } from '@sentry/types';
import { SentryError } from '@sentry/utils';
import * as http from 'http';
import * as HttpsProxyAgent from 'https-proxy-agent';
Expand Down Expand Up @@ -27,7 +27,7 @@ const sessionPayload: Session = {
timestamp: Date.now(),
init: true,
duration: 0,
status: SessionStatus.Exited,
status: 'exited',
update: jest.fn(),
close: jest.fn(),
toJSON: jest.fn(),
Expand Down
15 changes: 1 addition & 14 deletions packages/types/src/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,20 +55,7 @@ export interface SessionContext {
ignoreDuration?: boolean;
}

/**
* Session Status
*/
export enum SessionStatus {
/** JSDoc */
Ok = 'ok',
/** JSDoc */
Exited = 'exited',
/** JSDoc */
Crashed = 'crashed',
/** JSDoc */
Abnormal = 'abnormal',
}

export type SessionStatus = 'ok' | 'exited' | 'crashed' | 'abnormal';
export type RequestSessionStatus = 'ok' | 'errored' | 'crashed';

/** JSDoc */
Expand Down
13 changes: 13 additions & 0 deletions packages/types/src/sessionstatus.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/** JSDoc
* @deprecated Use string literals - if you require type casting, cast to SessionStatus type
*/
export enum SessionStatus {
/** JSDoc */
Ok = 'ok',
/** JSDoc */
Exited = 'exited',
/** JSDoc */
Crashed = 'crashed',
/** JSDoc */
Abnormal = 'abnormal',
}

0 comments on commit f5db0d7

Please sign in to comment.