Skip to content

Commit

Permalink
ref(types): deprecate request status enum (#4316)
Browse files Browse the repository at this point in the history
* ref(types): deprecate request status

* ref(types): deprecate session status

* ref(types): remove unused logLevel (#4317) (#4320)
  • Loading branch information
JonasBa authored Dec 16, 2021
1 parent 6f9069e commit e10697d
Show file tree
Hide file tree
Showing 19 changed files with 112 additions and 128 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
6 changes: 3 additions & 3 deletions packages/hub/src/sessionflusher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,13 @@ export class SessionFlusher implements SessionFlusherLike {
}

switch (status) {
case RequestSessionStatus.Errored:
case 'errored':
aggregationCounts.errored = (aggregationCounts.errored || 0) + 1;
return aggregationCounts.errored;
case RequestSessionStatus.Ok:
case 'ok':
aggregationCounts.exited = (aggregationCounts.exited || 0) + 1;
return aggregationCounts.exited;
case RequestSessionStatus.Crashed:
default:
aggregationCounts.crashed = (aggregationCounts.crashed || 0) + 1;
return aggregationCounts.crashed;
}
Expand Down
26 changes: 13 additions & 13 deletions packages/hub/test/scope.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Event, EventHint, RequestSessionStatus } from '@sentry/types';
import { Event, EventHint } from '@sentry/types';
import { getGlobalObject } from '@sentry/utils';

import { addGlobalEventProcessor, Scope } from '../src';
Expand Down Expand Up @@ -147,7 +147,7 @@ describe('Scope', () => {

test('_requestSession clone', () => {
const parentScope = new Scope();
parentScope.setRequestSession({ status: RequestSessionStatus.Errored });
parentScope.setRequestSession({ status: 'errored' });
const scope = Scope.clone(parentScope);
expect(parentScope.getRequestSession()).toEqual(scope.getRequestSession());
});
Expand All @@ -174,16 +174,16 @@ describe('Scope', () => {
// Test that ensures if the status value of `status` of `_requestSession` is changed in a child scope
// that it should also change in parent scope because we are copying the reference to the object
const parentScope = new Scope();
parentScope.setRequestSession({ status: RequestSessionStatus.Errored });
parentScope.setRequestSession({ status: 'errored' });

const scope = Scope.clone(parentScope);
const requestSession = scope.getRequestSession();
if (requestSession) {
requestSession.status = RequestSessionStatus.Ok;
requestSession.status = 'ok';
}

expect(parentScope.getRequestSession()).toEqual({ status: RequestSessionStatus.Ok });
expect(scope.getRequestSession()).toEqual({ status: RequestSessionStatus.Ok });
expect(parentScope.getRequestSession()).toEqual({ status: 'ok' });
expect(scope.getRequestSession()).toEqual({ status: 'ok' });
});
});

Expand Down Expand Up @@ -375,7 +375,7 @@ describe('Scope', () => {
scope.setUser({ id: '1' });
scope.setFingerprint(['abcd']);
scope.addBreadcrumb({ message: 'test' });
scope.setRequestSession({ status: RequestSessionStatus.Ok });
scope.setRequestSession({ status: 'ok' });
expect((scope as any)._extra).toEqual({ a: 2 });
scope.clear();
expect((scope as any)._extra).toEqual({});
Expand All @@ -402,7 +402,7 @@ describe('Scope', () => {
scope.setUser({ id: '1337' });
scope.setLevel('info');
scope.setFingerprint(['foo']);
scope.setRequestSession({ status: RequestSessionStatus.Ok });
scope.setRequestSession({ status: 'ok' });
});

test('given no data, returns the original scope', () => {
Expand Down Expand Up @@ -450,7 +450,7 @@ describe('Scope', () => {
localScope.setUser({ id: '42' });
localScope.setLevel('warning');
localScope.setFingerprint(['bar']);
(localScope as any)._requestSession = { status: RequestSessionStatus.Ok };
(localScope as any)._requestSession = { status: 'ok' };

const updatedScope = scope.update(localScope) as any;

Expand All @@ -472,7 +472,7 @@ describe('Scope', () => {
expect(updatedScope._user).toEqual({ id: '42' });
expect(updatedScope._level).toEqual('warning');
expect(updatedScope._fingerprint).toEqual(['bar']);
expect(updatedScope._requestSession.status).toEqual(RequestSessionStatus.Ok);
expect(updatedScope._requestSession.status).toEqual('ok');
});

test('given an empty instance of Scope, it should preserve all the original scope data', () => {
Expand All @@ -493,7 +493,7 @@ describe('Scope', () => {
expect(updatedScope._user).toEqual({ id: '1337' });
expect(updatedScope._level).toEqual('info');
expect(updatedScope._fingerprint).toEqual(['foo']);
expect(updatedScope._requestSession.status).toEqual(RequestSessionStatus.Ok);
expect(updatedScope._requestSession.status).toEqual('ok');
});

test('given a plain object, it should merge two together, with the passed object having priority', () => {
Expand All @@ -504,7 +504,7 @@ describe('Scope', () => {
level: 'warning',
tags: { bar: '3', baz: '4' },
user: { id: '42' },
requestSession: { status: RequestSessionStatus.Errored },
requestSession: { status: 'errored' },
};
const updatedScope = scope.update(localAttributes) as any;

Expand All @@ -526,7 +526,7 @@ describe('Scope', () => {
expect(updatedScope._user).toEqual({ id: '42' });
expect(updatedScope._level).toEqual('warning');
expect(updatedScope._fingerprint).toEqual(['bar']);
expect(updatedScope._requestSession).toEqual({ status: RequestSessionStatus.Errored });
expect(updatedScope._requestSession).toEqual({ status: 'errored' });
});
});

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');
});
});
});
24 changes: 11 additions & 13 deletions packages/hub/test/sessionflusher.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { RequestSessionStatus } from '@sentry/types';

import { SessionFlusher } from '../src/sessionflusher';

describe('Session Flusher', () => {
Expand Down Expand Up @@ -28,16 +26,16 @@ describe('Session Flusher', () => {
const flusher = new SessionFlusher(transport, { release: '1.0.0', environment: 'dev' });

const date = new Date('2021-04-08T12:18:23.043Z');
let count = (flusher as any)._incrementSessionStatusCount(RequestSessionStatus.Ok, date);
let count = (flusher as any)._incrementSessionStatusCount('ok', date);
expect(count).toEqual(1);
count = (flusher as any)._incrementSessionStatusCount(RequestSessionStatus.Ok, date);
count = (flusher as any)._incrementSessionStatusCount('ok', date);
expect(count).toEqual(2);
count = (flusher as any)._incrementSessionStatusCount(RequestSessionStatus.Errored, date);
count = (flusher as any)._incrementSessionStatusCount('errored', date);
expect(count).toEqual(1);
date.setMinutes(date.getMinutes() + 1);
count = (flusher as any)._incrementSessionStatusCount(RequestSessionStatus.Ok, date);
count = (flusher as any)._incrementSessionStatusCount('ok', date);
expect(count).toEqual(1);
count = (flusher as any)._incrementSessionStatusCount(RequestSessionStatus.Errored, date);
count = (flusher as any)._incrementSessionStatusCount('errored', date);
expect(count).toEqual(1);

expect(flusher.getSessionAggregates().aggregates).toEqual([
Expand All @@ -51,8 +49,8 @@ describe('Session Flusher', () => {
const flusher = new SessionFlusher(transport, { release: '1.0.0' });

const date = new Date('2021-04-08T12:18:23.043Z');
(flusher as any)._incrementSessionStatusCount(RequestSessionStatus.Ok, date);
(flusher as any)._incrementSessionStatusCount(RequestSessionStatus.Errored, date);
(flusher as any)._incrementSessionStatusCount('ok', date);
(flusher as any)._incrementSessionStatusCount('errored', date);

expect(flusher.getSessionAggregates()).toEqual({
aggregates: [{ errored: 1, exited: 1, started: '2021-04-08T12:18:00.000Z' }],
Expand All @@ -77,8 +75,8 @@ describe('Session Flusher', () => {
const flusher = new SessionFlusher(transport, { release: '1.0.0', environment: 'dev' });
const flusherFlushFunc = jest.spyOn(flusher, 'flush');
const date = new Date('2021-04-08T12:18:23.043Z');
(flusher as any)._incrementSessionStatusCount(RequestSessionStatus.Ok, date);
(flusher as any)._incrementSessionStatusCount(RequestSessionStatus.Ok, date);
(flusher as any)._incrementSessionStatusCount('ok', date);
(flusher as any)._incrementSessionStatusCount('ok', date);

expect(sendSession).toHaveBeenCalledTimes(0);

Expand Down Expand Up @@ -113,8 +111,8 @@ describe('Session Flusher', () => {
const flusher = new SessionFlusher(transport, { release: '1.0.x' });
const flusherFlushFunc = jest.spyOn(flusher, 'flush');
const date = new Date('2021-04-08T12:18:23.043Z');
(flusher as any)._incrementSessionStatusCount(RequestSessionStatus.Ok, date);
(flusher as any)._incrementSessionStatusCount(RequestSessionStatus.Ok, date);
(flusher as any)._incrementSessionStatusCount('ok', date);
(flusher as any)._incrementSessionStatusCount('ok', date);
flusher.close();

expect(flusherFlushFunc).toHaveBeenCalledTimes(1);
Expand Down
10 changes: 5 additions & 5 deletions packages/node/src/client.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
import { SessionFlusher } from '@sentry/hub';
import { Event, EventHint, RequestSessionStatus } from '@sentry/types';
import { Event, EventHint } from '@sentry/types';
import { logger } from '@sentry/utils';

import { NodeBackend } from './backend';
Expand Down Expand Up @@ -48,8 +48,8 @@ export class NodeClient extends BaseClient<NodeBackend, NodeOptions> {

// Necessary checks to ensure this is code block is executed only within a request
// Should override the status only if `requestSession.status` is `Ok`, which is its initial stage
if (requestSession && requestSession.status === RequestSessionStatus.Ok) {
requestSession.status = RequestSessionStatus.Errored;
if (requestSession && requestSession.status === 'ok') {
requestSession.status = 'errored';
}
}

Expand All @@ -74,8 +74,8 @@ export class NodeClient extends BaseClient<NodeBackend, NodeOptions> {

// Ensure that this is happening within the bounds of a request, and make sure not to override
// Session Status if Errored / Crashed
if (requestSession && requestSession.status === RequestSessionStatus.Ok) {
requestSession.status = RequestSessionStatus.Errored;
if (requestSession && requestSession.status === 'ok') {
requestSession.status = 'errored';
}
}
}
Expand Down
9 changes: 5 additions & 4 deletions packages/node/src/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { captureException, getCurrentHub, startTransaction, withScope } from '@sentry/core';
import { extractTraceparentData, Span } from '@sentry/tracing';
import { Event, ExtractedNodeRequestData, RequestSessionStatus, Transaction } from '@sentry/types';
import { Event, ExtractedNodeRequestData, Transaction } from '@sentry/types';
import { isPlainObject, isString, logger, normalize, stripUrlQueryAndFragment } from '@sentry/utils';
import * as cookie from 'cookie';
import * as domain from 'domain';
Expand Down Expand Up @@ -424,7 +424,7 @@ export function requestHandler(
const scope = currentHub.getScope();
if (scope) {
// Set `status` of `RequestSession` to Ok, at the beginning of the request
scope.setRequestSession({ status: RequestSessionStatus.Ok });
scope.setRequestSession({ status: 'ok' });
}
}
});
Expand Down Expand Up @@ -517,8 +517,9 @@ export function errorHandler(options?: {
// If an error bubbles to the `errorHandler`, then this is an unhandled error, and should be reported as a
// Crashed session. The `_requestSession.status` is checked to ensure that this error is happening within
// the bounds of a request, and if so the status is updated
if (requestSession && requestSession.status !== undefined)
requestSession.status = RequestSessionStatus.Crashed;
if (requestSession && requestSession.status !== undefined) {
requestSession.status = 'crashed';
}
}
}

Expand Down
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
Loading

0 comments on commit e10697d

Please sign in to comment.