Skip to content

Commit

Permalink
ref(types): Stop using Severity enum (#4926)
Browse files Browse the repository at this point in the history
Note: This and #4896 are together a (slightly updated) repeat of #4497, to get it onto the main v7 branch.

Our original v7 plan called for deprecating and then removing the `Severity` enum which lives in `@sentry/types`, because enums transpile to a two-way lookup function with a fairly hefty footprint (see this SO answer[1] from one of the maintainers of TS). We therefore added a new `SeverityLevel` type, which is the union of all of `Severity`'s underlying values, and directed people to use that instead. Though we subsequently decided not to remove `Severity` (at least in v7), we agreed that we should stop using it internally. This implements that change.

Key Changes:

- `Severity` and `severityFromString` have been redeprecated.
- The original plan to have `SeverityLevel` live in `@sentry/utils` has been reverted, and it now lives only in `@sentry/types`. 
- The internal `SeverityLevels` array on which we were basing `SeverityLevel` has been removed. While we lose the elegance of the derived type, we gain the ability to truly only export types from `@sentry/types`.
- Wherever we accept a `Severity` value, we now also accept a `SeverityLevel`.
- All internal uses of `Severity` values have been replaced with the equivalent string constants.
- A new `severityLevelFromString` function has been introduced, and is now used in place of `SeverityFromString`.
- The tests for `severityFromString` have been cleaned up and replaced with equivalent tests for `SeverityLevelFromString`.

[1] https://stackoverflow.com/a/28818850
  • Loading branch information
lobsterkatie authored and AbhiPrasad committed May 30, 2022
1 parent d9ce51f commit e96fdcd
Show file tree
Hide file tree
Showing 35 changed files with 199 additions and 119 deletions.
9 changes: 7 additions & 2 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { BaseClient, getEnvelopeEndpointWithUrlEncodedAuth, initAPIDetails, Scope, SDK_VERSION } from '@sentry/core';
import { Event, EventHint, Options, Severity, Transport, TransportOptions } from '@sentry/types';
import { Event, EventHint, Options, Severity, SeverityLevel, Transport, TransportOptions } from '@sentry/types';
import { getGlobalObject, logger, stackParserFromOptions, supportsFetch } from '@sentry/utils';

import { eventFromException, eventFromMessage } from './eventbuilder';
Expand Down Expand Up @@ -89,7 +89,12 @@ export class BrowserClient extends BaseClient<BrowserOptions> {
/**
* @inheritDoc
*/
public eventFromMessage(message: string, level: Severity = Severity.Info, hint?: EventHint): PromiseLike<Event> {
public eventFromMessage(
message: string,
// eslint-disable-next-line deprecation/deprecation
level: Severity | SeverityLevel = 'info',
hint?: EventHint,
): PromiseLike<Event> {
return eventFromMessage(
stackParserFromOptions(this._options),
message,
Expand Down
7 changes: 4 additions & 3 deletions packages/browser/src/eventbuilder.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Event, EventHint, Exception, Severity, StackFrame, StackParser } from '@sentry/types';
import { Event, EventHint, Exception, Severity, SeverityLevel, StackFrame, StackParser } from '@sentry/types';
import {
addExceptionMechanism,
addExceptionTypeValue,
Expand Down Expand Up @@ -150,7 +150,7 @@ export function eventFromException(
const syntheticException = (hint && hint.syntheticException) || undefined;
const event = eventFromUnknownInput(stackParser, exception, syntheticException, attachStacktrace);
addExceptionMechanism(event); // defaults to { type: 'generic', handled: true }
event.level = Severity.Error;
event.level = 'error';
if (hint && hint.event_id) {
event.event_id = hint.event_id;
}
Expand All @@ -164,7 +164,8 @@ export function eventFromException(
export function eventFromMessage(
stackParser: StackParser,
message: string,
level: Severity = Severity.Info,
// eslint-disable-next-line deprecation/deprecation
level: Severity | SeverityLevel = 'info',
hint?: EventHint,
attachStacktrace?: boolean,
): PromiseLike<Event> {
Expand Down
4 changes: 2 additions & 2 deletions packages/browser/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ export {
EventStatus,
Exception,
Response,
// eslint-disable-next-line deprecation/deprecation
Severity,
SeverityLevel,
StackFrame,
Stacktrace,
Thread,
User,
} from '@sentry/types';

export { SeverityLevel } from '@sentry/utils';

export {
addGlobalEventProcessor,
addBreadcrumb,
Expand Down
8 changes: 4 additions & 4 deletions packages/browser/src/integrations/breadcrumbs.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
/* eslint-disable max-lines */
import { getCurrentHub } from '@sentry/core';
import { Event, Integration, Severity } from '@sentry/types';
import { Event, Integration } from '@sentry/types';
import {
addInstrumentationHandler,
getEventDescription,
getGlobalObject,
htmlTreeAsString,
parseUrl,
safeJoin,
severityFromString,
severityLevelFromString,
} from '@sentry/utils';

/** JSDoc */
Expand Down Expand Up @@ -157,7 +157,7 @@ function _consoleBreadcrumb(handlerData: { [key: string]: any }): void {
arguments: handlerData.args,
logger: 'console',
},
level: severityFromString(handlerData.level),
level: severityLevelFromString(handlerData.level),
message: safeJoin(handlerData.args, ' '),
};

Expand Down Expand Up @@ -230,7 +230,7 @@ function _fetchBreadcrumb(handlerData: { [key: string]: any }): void {
{
category: 'fetch',
data: handlerData.fetchData,
level: Severity.Error,
level: 'error',
type: 'http',
},
{
Expand Down
6 changes: 3 additions & 3 deletions packages/browser/src/integrations/globalhandlers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
import { getCurrentHub } from '@sentry/core';
import { Event, EventHint, Hub, Integration, Primitive, Severity, StackParser } from '@sentry/types';
import { Event, EventHint, Hub, Integration, Primitive, StackParser } from '@sentry/types';
import {
addExceptionMechanism,
addInstrumentationHandler,
Expand Down Expand Up @@ -100,7 +100,7 @@ function _installGlobalOnErrorHandler(): void {
column,
);

event.level = Severity.Error;
event.level = 'error';

addMechanismAndCapture(hub, error, event, 'onerror');
},
Expand Down Expand Up @@ -146,7 +146,7 @@ function _installGlobalOnUnhandledRejectionHandler(): void {
? _eventFromRejectionWithPrimitive(error)
: eventFromUnknownInput(stackParser, error, undefined, attachStacktrace, true);

event.level = Severity.Error;
event.level = 'error';

addMechanismAndCapture(hub, error, event, 'onunhandledrejection');
return;
Expand Down
16 changes: 14 additions & 2 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
IntegrationClass,
Options,
Severity,
SeverityLevel,
Transport,
} from '@sentry/types';
import {
Expand Down Expand Up @@ -131,7 +132,13 @@ export abstract class BaseClient<O extends Options> implements Client<O> {
/**
* @inheritDoc
*/
public captureMessage(message: string, level?: Severity, hint?: EventHint, scope?: Scope): string | undefined {
public captureMessage(
message: string,
// eslint-disable-next-line deprecation/deprecation
level?: Severity | SeverityLevel,
hint?: EventHint,
scope?: Scope,
): string | undefined {
let eventId: string | undefined = hint && hint.event_id;

const promisedEvent = isPrimitive(message)
Expand Down Expand Up @@ -685,7 +692,12 @@ export abstract class BaseClient<O extends Options> implements Client<O> {
/**
* @inheritDoc
*/
public abstract eventFromMessage(_message: string, _level?: Severity, _hint?: EventHint): PromiseLike<Event>;
public abstract eventFromMessage(
_message: string,
// eslint-disable-next-line deprecation/deprecation
_level?: Severity | SeverityLevel,
_hint?: EventHint,
): PromiseLike<Event>;
}

/**
Expand Down
8 changes: 6 additions & 2 deletions packages/core/test/mocks/client.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Session } from '@sentry/hub';
import { Event, Options, Severity, Transport } from '@sentry/types';
import { Event, Options, Severity, SeverityLevel, Transport } from '@sentry/types';
import { resolvedSyncPromise } from '@sentry/utils';

import { BaseClient } from '../../src/baseclient';
Expand Down Expand Up @@ -38,7 +38,11 @@ export class TestClient extends BaseClient<TestOptions> {
});
}

public eventFromMessage(message: string, level: Severity = Severity.Info): PromiseLike<Event> {
public eventFromMessage(
message: string,
// eslint-disable-next-line deprecation/deprecation
level: Severity | SeverityLevel = 'info',
): PromiseLike<Event> {
return resolvedSyncPromise({ message, level });
}

Expand Down
8 changes: 7 additions & 1 deletion packages/hub/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
Primitive,
SessionContext,
Severity,
SeverityLevel,
Transaction,
TransactionContext,
User,
Expand Down Expand Up @@ -213,7 +214,12 @@ export class Hub implements HubInterface {
/**
* @inheritDoc
*/
public captureMessage(message: string, level?: Severity, hint?: EventHint): string {
public captureMessage(
message: string,
// eslint-disable-next-line deprecation/deprecation
level?: Severity | SeverityLevel,
hint?: EventHint,
): string {
const eventId = (this._lastEventId = hint && hint.event_id ? hint.event_id : uuid4());
let finalHint = hint;

Expand Down
9 changes: 7 additions & 2 deletions packages/hub/src/scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
Scope as ScopeInterface,
ScopeContext,
Severity,
SeverityLevel,
Span,
Transaction,
User,
Expand Down Expand Up @@ -61,7 +62,8 @@ export class Scope implements ScopeInterface {
protected _fingerprint?: string[];

/** Severity */
protected _level?: Severity;
// eslint-disable-next-line deprecation/deprecation
protected _level?: Severity | SeverityLevel;

/** Transaction Name */
protected _transactionName?: string;
Expand Down Expand Up @@ -208,7 +210,10 @@ export class Scope implements ScopeInterface {
/**
* @inheritDoc
*/
public setLevel(level: Severity): this {
public setLevel(
// eslint-disable-next-line deprecation/deprecation
level: Severity | SeverityLevel,
): this {
this._level = level;
this._notifyScopeListeners();
return this;
Expand Down
22 changes: 11 additions & 11 deletions packages/hub/test/scope.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Event, EventHint, Severity } from '@sentry/types';
import { Event, EventHint } from '@sentry/types';
import { getGlobalObject } from '@sentry/utils';

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

test('setLevel', () => {
const scope = new Scope();
scope.setLevel(Severity.Critical);
expect((scope as any)._level).toEqual(Severity.Critical);
scope.setLevel('critical');
expect((scope as any)._level).toEqual('critical');
});

test('setTransactionName', () => {
Expand Down Expand Up @@ -137,8 +137,8 @@ describe('Scope', () => {

test('chaining', () => {
const scope = new Scope();
scope.setLevel(Severity.Critical).setUser({ id: '1' });
expect((scope as any)._level).toEqual(Severity.Critical);
scope.setLevel('critical').setUser({ id: '1' });
expect((scope as any)._level).toEqual('critical');
expect((scope as any)._user).toEqual({ id: '1' });
});
});
Expand Down Expand Up @@ -202,7 +202,7 @@ describe('Scope', () => {
scope.setTag('a', 'b');
scope.setUser({ id: '1' });
scope.setFingerprint(['abcd']);
scope.setLevel(Severity.Warning);
scope.setLevel('warning');
scope.setTransactionName('/abc');
scope.addBreadcrumb({ message: 'test' });
scope.setContext('os', { id: '1' });
Expand Down Expand Up @@ -294,11 +294,11 @@ describe('Scope', () => {
test('scope level should have priority over event level', () => {
expect.assertions(1);
const scope = new Scope();
scope.setLevel(Severity.Warning);
scope.setLevel('warning');
const event: Event = {};
event.level = Severity.Critical;
event.level = 'critical';
return scope.applyToEvent(event).then(processedEvent => {
expect(processedEvent!.level).toEqual(Severity.Warning);
expect(processedEvent!.level).toEqual('warning');
});
});

Expand Down Expand Up @@ -410,7 +410,7 @@ describe('Scope', () => {
scope.setContext('foo', { id: '1' });
scope.setContext('bar', { id: '2' });
scope.setUser({ id: '1337' });
scope.setLevel(Severity.Info);
scope.setLevel('info');
scope.setFingerprint(['foo']);
scope.setRequestSession({ status: 'ok' });
});
Expand Down Expand Up @@ -458,7 +458,7 @@ describe('Scope', () => {
localScope.setContext('bar', { id: '3' });
localScope.setContext('baz', { id: '4' });
localScope.setUser({ id: '42' });
localScope.setLevel(Severity.Warning);
localScope.setLevel('warning');
localScope.setFingerprint(['bar']);
(localScope as any)._requestSession = { status: 'ok' };

Expand Down
4 changes: 2 additions & 2 deletions packages/integrations/src/captureconsole.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { EventProcessor, Hub, Integration } from '@sentry/types';
import { CONSOLE_LEVELS, fill, getGlobalObject, safeJoin, severityFromString } from '@sentry/utils';
import { CONSOLE_LEVELS, fill, getGlobalObject, safeJoin, severityLevelFromString } from '@sentry/utils';

const global = getGlobalObject<Window | NodeJS.Global>();

Expand Down Expand Up @@ -48,7 +48,7 @@ export class CaptureConsole implements Integration {

if (hub.getIntegration(CaptureConsole)) {
hub.withScope(scope => {
scope.setLevel(severityFromString(level));
scope.setLevel(severityLevelFromString(level));
scope.setExtra('arguments', args);
scope.addEventProcessor(event => {
event.logger = 'console';
Expand Down
7 changes: 6 additions & 1 deletion packages/minimal/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
Extras,
Primitive,
Severity,
SeverityLevel,
Transaction,
TransactionContext,
User,
Expand Down Expand Up @@ -52,7 +53,11 @@ export function captureException(exception: any, captureContext?: CaptureContext
* @param Severity Define the level of the message.
* @returns The generated eventId.
*/
export function captureMessage(message: string, captureContext?: CaptureContext | Severity): string {
export function captureMessage(
message: string,
// eslint-disable-next-line deprecation/deprecation
captureContext?: CaptureContext | Severity | SeverityLevel,
): string {
const syntheticException = new Error(message);

// This is necessary to provide explicit scopes upgrade, without changing the original
Expand Down
13 changes: 6 additions & 7 deletions packages/minimal/test/lib/minimal.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { getCurrentHub, getHubFromCarrier, Scope } from '@sentry/hub';
import { Severity } from '@sentry/types';

import {
_callOnClient,
Expand Down Expand Up @@ -165,8 +164,8 @@ describe('Minimal', () => {
const client: any = new TestClient({});
const scope = getCurrentHub().pushScope();
getCurrentHub().bindClient(client);
scope.setLevel(Severity.Warning);
expect(global.__SENTRY__.hub._stack[1].scope._level).toEqual(Severity.Warning);
scope.setLevel('warning');
expect(global.__SENTRY__.hub._stack[1].scope._level).toEqual('warning');
});
});

Expand Down Expand Up @@ -245,16 +244,16 @@ describe('Minimal', () => {

test('withScope', () => {
withScope(scope => {
scope.setLevel(Severity.Warning);
scope.setLevel('warning');
scope.setFingerprint(['1']);
withScope(scope2 => {
scope2.setLevel(Severity.Info);
scope2.setLevel('info');
scope2.setFingerprint(['2']);
withScope(scope3 => {
scope3.clear();
expect(global.__SENTRY__.hub._stack[1].scope._level).toEqual(Severity.Warning);
expect(global.__SENTRY__.hub._stack[1].scope._level).toEqual('warning');
expect(global.__SENTRY__.hub._stack[1].scope._fingerprint).toEqual(['1']);
expect(global.__SENTRY__.hub._stack[2].scope._level).toEqual(Severity.Info);
expect(global.__SENTRY__.hub._stack[2].scope._level).toEqual('info');
expect(global.__SENTRY__.hub._stack[2].scope._fingerprint).toEqual(['2']);
expect(global.__SENTRY__.hub._stack[3].scope._level).toBeUndefined();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Sentry.init({
Sentry.addBreadcrumb({
category: 'foo',
message: 'bar',
level: Sentry.Severity.Critical,
level: 'critical',
});

Sentry.addBreadcrumb({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Sentry.init({
Sentry.addBreadcrumb({
category: 'foo',
message: 'bar',
level: Sentry.Severity.Critical,
level: 'critical',
});

Sentry.captureMessage('test_simple');
Loading

0 comments on commit e96fdcd

Please sign in to comment.