Skip to content

Commit

Permalink
ref(types): deprecate outcome enum (#4315)
Browse files Browse the repository at this point in the history
* ref(types): deprecate outcome enum

* fix(types): drop transportoutcome

* ref(types): deprecate request status enum (#4316)

* 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 3704584 commit e8b0fb9
Show file tree
Hide file tree
Showing 28 changed files with 159 additions and 183 deletions.
8 changes: 4 additions & 4 deletions packages/browser/src/transports/fetch.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { eventToSentryRequest, sessionToSentryRequest } from '@sentry/core';
import { Event, Outcome, Response, SentryRequest, Session, TransportOptions } from '@sentry/types';
import { Event, Response, SentryRequest, Session, TransportOptions } from '@sentry/types';
import { SentryError, supportsReferrerPolicy, SyncPromise } from '@sentry/utils';

import { BaseTransport } from './base';
Expand Down Expand Up @@ -37,7 +37,7 @@ export class FetchTransport extends BaseTransport {
*/
private _sendRequest(sentryRequest: SentryRequest, originalPayload: Event | Session): PromiseLike<Response> {
if (this._isRateLimited(sentryRequest.type)) {
this.recordLostEvent(Outcome.RateLimitBackoff, sentryRequest.type);
this.recordLostEvent('ratelimit_backoff', sentryRequest.type);

return Promise.reject({
event: originalPayload,
Expand Down Expand Up @@ -89,9 +89,9 @@ export class FetchTransport extends BaseTransport {
.then(undefined, reason => {
// It's either buffer rejection or any other xhr/fetch error, which are treated as NetworkError.
if (reason instanceof SentryError) {
this.recordLostEvent(Outcome.QueueOverflow, sentryRequest.type);
this.recordLostEvent('queue_overflow', sentryRequest.type);
} else {
this.recordLostEvent(Outcome.NetworkError, sentryRequest.type);
this.recordLostEvent('network_error', sentryRequest.type);
}
throw reason;
});
Expand Down
8 changes: 4 additions & 4 deletions packages/browser/src/transports/xhr.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { eventToSentryRequest, sessionToSentryRequest } from '@sentry/core';
import { Event, Outcome, Response, SentryRequest, Session } from '@sentry/types';
import { Event, Response, SentryRequest, Session } from '@sentry/types';
import { SentryError, SyncPromise } from '@sentry/utils';

import { BaseTransport } from './base';
Expand All @@ -26,7 +26,7 @@ export class XHRTransport extends BaseTransport {
*/
private _sendRequest(sentryRequest: SentryRequest, originalPayload: Event | Session): PromiseLike<Response> {
if (this._isRateLimited(sentryRequest.type)) {
this.recordLostEvent(Outcome.RateLimitBackoff, sentryRequest.type);
this.recordLostEvent('ratelimit_backoff', sentryRequest.type);

return Promise.reject({
event: originalPayload,
Expand Down Expand Up @@ -66,9 +66,9 @@ export class XHRTransport extends BaseTransport {
.then(undefined, reason => {
// It's either buffer rejection or any other xhr/fetch error, which are treated as NetworkError.
if (reason instanceof SentryError) {
this.recordLostEvent(Outcome.QueueOverflow, sentryRequest.type);
this.recordLostEvent('queue_overflow', sentryRequest.type);
} else {
this.recordLostEvent(Outcome.NetworkError, sentryRequest.type);
this.recordLostEvent('network_error', sentryRequest.type);
}
throw reason;
});
Expand Down
32 changes: 15 additions & 17 deletions packages/browser/test/unit/transports/base.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { Outcome } from '@sentry/types';

import { BaseTransport } from '../../../src/transports/base';

const testDsn = 'https://[email protected]/42';
Expand Down Expand Up @@ -44,12 +42,12 @@ describe('BaseTransport', () => {
it('sends beacon request when there are outcomes captured and visibility changed to `hidden`', () => {
const transport = new SimpleTransport({ dsn: testDsn, sendClientReports: true });

transport.recordLostEvent(Outcome.BeforeSend, 'event');
transport.recordLostEvent('before_send', 'event');

visibilityState = 'hidden';
document.dispatchEvent(new Event('visibilitychange'));

const outcomes = [{ reason: Outcome.BeforeSend, category: 'error', quantity: 1 }];
const outcomes = [{ reason: 'before_send', category: 'error', quantity: 1 }];

expect(sendBeaconSpy).toHaveBeenCalledWith(
envelopeEndpoint,
Expand All @@ -59,7 +57,7 @@ describe('BaseTransport', () => {

it('doesnt send beacon request when there are outcomes captured, but visibility state did not change to `hidden`', () => {
const transport = new SimpleTransport({ dsn: testDsn, sendClientReports: true });
transport.recordLostEvent(Outcome.BeforeSend, 'event');
transport.recordLostEvent('before_send', 'event');

visibilityState = 'visible';
document.dispatchEvent(new Event('visibilitychange'));
Expand All @@ -70,21 +68,21 @@ describe('BaseTransport', () => {
it('correctly serializes request with different categories/reasons pairs', () => {
const transport = new SimpleTransport({ dsn: testDsn, sendClientReports: true });

transport.recordLostEvent(Outcome.BeforeSend, 'event');
transport.recordLostEvent(Outcome.BeforeSend, 'event');
transport.recordLostEvent(Outcome.SampleRate, 'transaction');
transport.recordLostEvent(Outcome.NetworkError, 'session');
transport.recordLostEvent(Outcome.NetworkError, 'session');
transport.recordLostEvent(Outcome.RateLimitBackoff, 'event');
transport.recordLostEvent('before_send', 'event');
transport.recordLostEvent('before_send', 'event');
transport.recordLostEvent('sample_rate', 'transaction');
transport.recordLostEvent('network_error', 'session');
transport.recordLostEvent('network_error', 'session');
transport.recordLostEvent('ratelimit_backoff', 'event');

visibilityState = 'hidden';
document.dispatchEvent(new Event('visibilitychange'));

const outcomes = [
{ reason: Outcome.BeforeSend, category: 'error', quantity: 2 },
{ reason: Outcome.SampleRate, category: 'transaction', quantity: 1 },
{ reason: Outcome.NetworkError, category: 'session', quantity: 2 },
{ reason: Outcome.RateLimitBackoff, category: 'error', quantity: 1 },
{ reason: 'before_send', category: 'error', quantity: 2 },
{ reason: 'sample_rate', category: 'transaction', quantity: 1 },
{ reason: 'network_error', category: 'session', quantity: 2 },
{ reason: 'ratelimit_backoff', category: 'error', quantity: 1 },
];

expect(sendBeaconSpy).toHaveBeenCalledWith(
Expand All @@ -97,12 +95,12 @@ describe('BaseTransport', () => {
const tunnel = 'https://hello.com/world';
const transport = new SimpleTransport({ dsn: testDsn, sendClientReports: true, tunnel });

transport.recordLostEvent(Outcome.BeforeSend, 'event');
transport.recordLostEvent('before_send', 'event');

visibilityState = 'hidden';
document.dispatchEvent(new Event('visibilitychange'));

const outcomes = [{ reason: Outcome.BeforeSend, category: 'error', quantity: 1 }];
const outcomes = [{ reason: 'before_send', category: 'error', quantity: 1 }];

expect(sendBeaconSpy).toHaveBeenCalledWith(
tunnel,
Expand Down
9 changes: 4 additions & 5 deletions packages/browser/test/unit/transports/fetch.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { Outcome } from '@sentry/types';
import { SentryError } from '@sentry/utils';

import { Event, Response, Transports } from '../../../src';
Expand Down Expand Up @@ -117,7 +116,7 @@ describe('FetchTransport', () => {
try {
await transport.sendEvent(eventPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.NetworkError, 'event');
expect(spy).toHaveBeenCalledWith('network_error', 'event');
}
});

Expand All @@ -129,7 +128,7 @@ describe('FetchTransport', () => {
try {
await transport.sendEvent(transactionPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.QueueOverflow, 'transaction');
expect(spy).toHaveBeenCalledWith('queue_overflow', 'transaction');
}
});

Expand Down Expand Up @@ -490,13 +489,13 @@ describe('FetchTransport', () => {
try {
await transport.sendEvent(eventPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.RateLimitBackoff, 'event');
expect(spy).toHaveBeenCalledWith('ratelimit_backoff', 'event');
}

try {
await transport.sendEvent(transactionPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.RateLimitBackoff, 'transaction');
expect(spy).toHaveBeenCalledWith('ratelimit_backoff', 'transaction');
}
});
});
Expand Down
9 changes: 4 additions & 5 deletions packages/browser/test/unit/transports/xhr.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { Outcome } from '@sentry/types';
import { SentryError } from '@sentry/utils';
import { fakeServer, SinonFakeServer } from 'sinon';

Expand Down Expand Up @@ -79,7 +78,7 @@ describe('XHRTransport', () => {
try {
await transport.sendEvent(eventPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.NetworkError, 'event');
expect(spy).toHaveBeenCalledWith('network_error', 'event');
}
});

Expand All @@ -91,7 +90,7 @@ describe('XHRTransport', () => {
try {
await transport.sendEvent(transactionPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.QueueOverflow, 'transaction');
expect(spy).toHaveBeenCalledWith('queue_overflow', 'transaction');
}
});

Expand Down Expand Up @@ -416,13 +415,13 @@ describe('XHRTransport', () => {
try {
await transport.sendEvent(eventPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.RateLimitBackoff, 'event');
expect(spy).toHaveBeenCalledWith('ratelimit_backoff', 'event');
}

try {
await transport.sendEvent(transactionPayload);
} catch (_) {
expect(spy).toHaveBeenCalledWith(Outcome.RateLimitBackoff, 'transaction');
expect(spy).toHaveBeenCalledWith('ratelimit_backoff', 'transaction');
}
});
});
Expand Down
12 changes: 5 additions & 7 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import {
Integration,
IntegrationClass,
Options,
Outcome,
SessionStatus,
SeverityLevel,
Transport,
} from '@sentry/types';
Expand Down Expand Up @@ -268,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 Expand Up @@ -541,7 +539,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
// 0.0 === 0% events are sent
// Sampling for transaction happens somewhere else
if (!isTransaction && typeof sampleRate === 'number' && Math.random() > sampleRate) {
recordLostEvent(Outcome.SampleRate, 'event');
recordLostEvent('sample_rate', 'event');
return SyncPromise.reject(
new SentryError(
`Discarding event because it's not included in the random sample (sampling rate = ${sampleRate})`,
Expand All @@ -552,7 +550,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
return this._prepareEvent(event, scope, hint)
.then(prepared => {
if (prepared === null) {
recordLostEvent(Outcome.EventProcessor, event.type || 'event');
recordLostEvent('event_processor', event.type || 'event');
throw new SentryError('An event processor returned null, will not send event.');
}

Expand All @@ -566,7 +564,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
})
.then(processedEvent => {
if (processedEvent === null) {
recordLostEvent(Outcome.BeforeSend, event.type || 'event');
recordLostEvent('before_send', event.type || 'event');
throw new SentryError('`beforeSend` returned `null`, will not send event.');
}

Expand Down
8 changes: 4 additions & 4 deletions packages/core/test/lib/base.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Hub, Scope, Session } from '@sentry/hub';
import { Event, Outcome, Span, Transport } from '@sentry/types';
import { Event, Span, Transport } from '@sentry/types';
import { logger, SentryError, SyncPromise } from '@sentry/utils';

import * as integrationModule from '../../src/integration';
Expand Down Expand Up @@ -882,7 +882,7 @@ describe('BaseClient', () => {

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

expect(recordLostEventSpy).toHaveBeenCalledWith(Outcome.BeforeSend, 'event');
expect(recordLostEventSpy).toHaveBeenCalledWith('before_send', 'event');
});

test('eventProcessor can drop the even when it returns null', () => {
Expand Down Expand Up @@ -914,7 +914,7 @@ describe('BaseClient', () => {
scope.addEventProcessor(() => null);
client.captureEvent({ message: 'hello' }, {}, scope);

expect(recordLostEventSpy).toHaveBeenCalledWith(Outcome.EventProcessor, 'event');
expect(recordLostEventSpy).toHaveBeenCalledWith('event_processor', 'event');
});

test('eventProcessor sends an event and logs when it crashes', () => {
Expand Down Expand Up @@ -958,7 +958,7 @@ describe('BaseClient', () => {
);

client.captureEvent({ message: 'hello' }, {});
expect(recordLostEventSpy).toHaveBeenCalledWith(Outcome.SampleRate, 'event');
expect(recordLostEventSpy).toHaveBeenCalledWith('sample_rate', 'event');
});
});

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
Loading

0 comments on commit e8b0fb9

Please sign in to comment.