Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ref(types): deprecate request status enum #4316

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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