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 outcome enum #4315

Merged
merged 3 commits into from
Dec 16, 2021
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
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://123@sentry.io/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