Skip to content

Commit

Permalink
ref(types): deprecate request status
Browse files Browse the repository at this point in the history
  • Loading branch information
@JonasBa
JonasBa committed Dec 16, 2021
1 parent 6f9069e commit f99bdd1
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 74 deletions.
6 changes: 3 additions & 3 deletions packages/hub/src/sessionflusher.ts
View file
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
View file
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
24 changes: 11 additions & 13 deletions packages/hub/test/sessionflusher.test.ts
View file
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
View file
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
View file
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
37 changes: 18 additions & 19 deletions packages/node/test/client.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { Scope, SessionFlusher } from '@sentry/hub';
import { RequestSessionStatus } from '@sentry/types';

import { NodeClient } from '../src';

Expand All @@ -17,12 +16,12 @@ describe('NodeClient', () => {
test('when autoSessionTracking is enabled, and requestHandler is not used -> requestStatus should not be set', () => {
client = new NodeClient({ dsn: PUBLIC_DSN, autoSessionTracking: true, release: '1.4' });
const scope = new Scope();
scope.setRequestSession({ status: RequestSessionStatus.Ok });
scope.setRequestSession({ status: 'ok' });

client.captureException(new Error('test exception'), undefined, scope);

const requestSession = scope.getRequestSession();
expect(requestSession!.status).toEqual(RequestSessionStatus.Ok);
expect(requestSession!.status).toEqual('ok');
});
test('when autoSessionTracking is disabled -> requestStatus should not be set', () => {
client = new NodeClient({ dsn: PUBLIC_DSN, autoSessionTracking: false, release: '1.4' });
Expand All @@ -31,12 +30,12 @@ describe('NodeClient', () => {
client.initSessionFlusher();

const scope = new Scope();
scope.setRequestSession({ status: RequestSessionStatus.Ok });
scope.setRequestSession({ status: 'ok' });

client.captureException(new Error('test exception'), undefined, scope);

const requestSession = scope.getRequestSession();
expect(requestSession!.status).toEqual(RequestSessionStatus.Ok);
expect(requestSession!.status).toEqual('ok');
});
test('when autoSessionTracking is enabled + requestSession status is Crashed -> requestStatus should not be overridden', () => {
client = new NodeClient({ dsn: PUBLIC_DSN, autoSessionTracking: true, release: '1.4' });
Expand All @@ -45,12 +44,12 @@ describe('NodeClient', () => {
client.initSessionFlusher();

const scope = new Scope();
scope.setRequestSession({ status: RequestSessionStatus.Crashed });
scope.setRequestSession({ status: 'crashed' });

client.captureException(new Error('test exception'), undefined, scope);

const requestSession = scope.getRequestSession();
expect(requestSession!.status).toEqual(RequestSessionStatus.Crashed);
expect(requestSession!.status).toEqual('crashed');
});
test('when autoSessionTracking is enabled + error occurs within request bounds -> requestStatus should be set to Errored', () => {
client = new NodeClient({ dsn: PUBLIC_DSN, autoSessionTracking: true, release: '1.4' });
Expand All @@ -59,12 +58,12 @@ describe('NodeClient', () => {
client.initSessionFlusher();

const scope = new Scope();
scope.setRequestSession({ status: RequestSessionStatus.Ok });
scope.setRequestSession({ status: 'ok' });

client.captureException(new Error('test exception'), undefined, scope);

const requestSession = scope.getRequestSession();
expect(requestSession!.status).toEqual(RequestSessionStatus.Errored);
expect(requestSession!.status).toEqual('errored');
});
test('when autoSessionTracking is enabled + error occurs outside of request bounds -> requestStatus should not be set to Errored', () => {
client = new NodeClient({ dsn: PUBLIC_DSN, autoSessionTracking: true, release: '1.4' });
Expand All @@ -89,15 +88,15 @@ describe('NodeClient', () => {
client.initSessionFlusher();

const scope = new Scope();
scope.setRequestSession({ status: RequestSessionStatus.Ok });
scope.setRequestSession({ status: 'ok' });
client.captureEvent(
{ message: 'message', exception: { values: [{ type: 'exception type 1' }] } },
undefined,
scope,
);

const requestSession = scope.getRequestSession();
expect(requestSession!.status).toEqual(RequestSessionStatus.Ok);
expect(requestSession!.status).toEqual('ok');
});

test('When captureEvent is called with an exception, requestSession status should be set to Errored', () => {
Expand All @@ -107,12 +106,12 @@ describe('NodeClient', () => {
client.initSessionFlusher();

const scope = new Scope();
scope.setRequestSession({ status: RequestSessionStatus.Ok });
scope.setRequestSession({ status: 'ok' });

client.captureEvent({ message: 'message', exception: { values: [{ type: 'exception type 1' }] } }, {}, scope);

const requestSession = scope.getRequestSession();
expect(requestSession!.status).toEqual(RequestSessionStatus.Errored);
expect(requestSession!.status).toEqual('errored');
});

test('When captureEvent is called without an exception, requestSession status should not be set to Errored', () => {
Expand All @@ -122,12 +121,12 @@ describe('NodeClient', () => {
client.initSessionFlusher();

const scope = new Scope();
scope.setRequestSession({ status: RequestSessionStatus.Ok });
scope.setRequestSession({ status: 'ok' });

client.captureEvent({ message: 'message' }, {}, scope);

const requestSession = scope.getRequestSession();
expect(requestSession!.status).toEqual(RequestSessionStatus.Ok);
expect(requestSession!.status).toEqual('ok');
});

test('When captureEvent is called with an exception but outside of a request, then requestStatus should not be set', () => {
Expand All @@ -154,26 +153,26 @@ describe('NodeClient', () => {
client.initSessionFlusher();

const scope = new Scope();
scope.setRequestSession({ status: RequestSessionStatus.Ok });
scope.setRequestSession({ status: 'ok' });
client.captureEvent({ message: 'message', type: 'transaction' }, undefined, scope);

const requestSession = scope.getRequestSession();
expect(requestSession!.status).toEqual(RequestSessionStatus.Ok);
expect(requestSession!.status).toEqual('ok');
});

test('When captureEvent is called with an exception but requestHandler is not used, then requestSession status should not be set', () => {
client = new NodeClient({ dsn: PUBLIC_DSN, autoSessionTracking: true, release: '1.3' });

const scope = new Scope();
scope.setRequestSession({ status: RequestSessionStatus.Ok });
scope.setRequestSession({ status: 'ok' });
client.captureEvent(
{ message: 'message', exception: { values: [{ type: 'exception type 1' }] } },
undefined,
scope,
);

const requestSession = scope.getRequestSession();
expect(requestSession!.status).toEqual(RequestSessionStatus.Ok);
expect(requestSession!.status).toEqual('ok');
});
});
});
Expand Down
Loading

0 comments on commit f99bdd1

Please sign in to comment.