Skip to content

ref(types): deprecate outcome enum #4315

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

Merged
merged 3 commits into from
Dec 16, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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://[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
7 changes: 3 additions & 4 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,
Outcome,
SessionStatus,
SeverityLevel,
Transport,
Expand Down Expand Up @@ -541,7 +540,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 +551,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 +565,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
3 changes: 1 addition & 2 deletions packages/tracing/src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { getCurrentHub, Hub } from '@sentry/hub';
import {
Event,
Measurements,
Outcome,
Transaction as TransactionInterface,
TransactionContext,
TransactionMetadata,
Expand Down Expand Up @@ -107,7 +106,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
const client = this._hub.getClient();
const transport = client && client.getTransport && client.getTransport();
if (transport && transport.recordLostEvent) {
transport.recordLostEvent(Outcome.SampleRate, 'transaction');
transport.recordLostEvent('sample_rate', 'transaction');
}
return undefined;
}
Expand Down
3 changes: 1 addition & 2 deletions packages/tracing/test/idletransaction.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { BrowserClient, Transports } from '@sentry/browser';
import { Hub } from '@sentry/hub';
import { Outcome } from '@sentry/types';

import {
DEFAULT_IDLE_TIMEOUT,
Expand Down Expand Up @@ -174,7 +173,7 @@ describe('IdleTransaction', () => {
transaction.initSpanRecorder(10);
transaction.finish(transaction.startTimestamp + 10);

expect(spy).toHaveBeenCalledWith(Outcome.SampleRate, 'transaction');
expect(spy).toHaveBeenCalledWith('sample_rate', 'transaction');
});

describe('_initTimeout', () => {
Expand Down
15 changes: 7 additions & 8 deletions packages/types/src/transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@ import { Response } from './response';
import { SdkMetadata } from './sdkmetadata';
import { Session, SessionAggregates } from './session';

export enum Outcome {
BeforeSend = 'before_send',
EventProcessor = 'event_processor',
NetworkError = 'network_error',
QueueOverflow = 'queue_overflow',
RateLimitBackoff = 'ratelimit_backoff',
SampleRate = 'sample_rate',
}
export type Outcome =
| 'before_send'
| 'event_processor'
| 'network_error'
| 'queue_overflow'
| 'ratelimit_backoff'
| 'sample_rate';

/** Transport used sending data to Sentry */
export interface Transport {
Expand Down