frontend logging fixes (#39946)

This commit is contained in:
Domas 2021-10-18 14:34:31 +03:00 committed by GitHub
parent 71264835f2
commit a531c6e26f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 91 additions and 6 deletions

View File

@ -66,6 +66,11 @@ describe('SentryEchoBackend', () => {
dsn: options.dsn,
sampleRate: options.sampleRate,
transport: EchoSrvTransport,
ignoreErrors: [
'ResizeObserver loop limit exceeded',
'ResizeObserver loop completed',
'Non-Error exception captured with keys',
],
});
expect(sentrySetUser).toHaveBeenCalledWith({
email: options.user?.email,

View File

@ -35,6 +35,11 @@ export class SentryEchoBackend implements EchoBackend<SentryEchoEvent, SentryEch
dsn: options.dsn || 'https://examplePublicKey@o0.ingest.sentry.io/0',
sampleRate: options.sampleRate,
transport: EchoSrvTransport, // will dump errors to EchoSrv
ignoreErrors: [
'ResizeObserver loop limit exceeded',
'ResizeObserver loop completed',
'Non-Error exception captured with keys',
],
};
if (options.user) {

View File

@ -1,4 +1,5 @@
import { Event, Severity } from '@sentry/browser';
import { Status } from '@sentry/types';
import { CustomEndpointTransport } from './CustomEndpointTransport';
describe('CustomEndpointTransport', () => {
@ -53,7 +54,7 @@ describe('CustomEndpointTransport', () => {
expect(fetchSpy).toHaveBeenCalledTimes(1);
// second immediate call - shot circuited because retry-after time has not expired, backend not called
await expect(transport.sendEvent(event)).rejects.toBeTruthy();
await expect(transport.sendEvent(event)).resolves.toHaveProperty('status', Status.Skipped);
expect(fetchSpy).toHaveBeenCalledTimes(1);
// wait out the retry-after and call again - great success
@ -61,4 +62,61 @@ describe('CustomEndpointTransport', () => {
await expect(transport.sendEvent(event)).resolves.toBeTruthy();
expect(fetchSpy).toHaveBeenCalledTimes(2);
});
it('will back off if backend returns Retry-After', async () => {
const rateLimiterResponse = {
status: 429,
ok: false,
headers: (new Headers({
'Retry-After': '1', // 1 second
}) as any) as Headers,
} as Response;
fetchSpy.mockResolvedValueOnce(rateLimiterResponse).mockResolvedValueOnce({ status: 200 } as Response);
const transport = new CustomEndpointTransport({ endpoint: '/log' });
// first call - backend is called, rejected because of 429
await expect(transport.sendEvent(event)).rejects.toHaveProperty('status', 429);
expect(fetchSpy).toHaveBeenCalledTimes(1);
// second immediate call - shot circuited because retry-after time has not expired, backend not called
await expect(transport.sendEvent(event)).resolves.toHaveProperty('status', Status.Skipped);
expect(fetchSpy).toHaveBeenCalledTimes(1);
// wait out the retry-after and call again - great success
await new Promise((resolve) => setTimeout(() => resolve(null), 1001));
await expect(transport.sendEvent(event)).resolves.toBeTruthy();
expect(fetchSpy).toHaveBeenCalledTimes(2);
});
it('will drop events if max concurrency is reached', async () => {
const calls: Array<(value: unknown) => void> = [];
fetchSpy.mockImplementation(
() =>
new Promise((resolve) => {
calls.push(resolve);
})
);
const transport = new CustomEndpointTransport({ endpoint: '/log', maxConcurrentRequests: 2 });
// first two requests are accepted
transport.sendEvent(event);
const event2 = transport.sendEvent(event);
expect(calls).toHaveLength(2);
// third is skipped because too many requests in flight
await expect(transport.sendEvent(event)).resolves.toHaveProperty('status', 'skipped');
expect(calls).toHaveLength(2);
// after resolving in flight requests, next request is accepted as well
calls.forEach((call) => {
call({ status: 200 });
});
await event2;
const event3 = transport.sendEvent(event);
expect(calls).toHaveLength(3);
calls[2]({ status: 200 });
await event3;
});
});

View File

@ -6,8 +6,11 @@ import { BaseTransport } from '../types';
export interface CustomEndpointTransportOptions {
endpoint: string;
fetchParameters?: Partial<RequestInit>;
maxConcurrentRequests?: number;
}
const DEFAULT_MAX_CONCURRENT_REQUESTS = 3;
/**
* This is a copy of sentry's FetchTransport, edited to be able to push to any custom url
* instead of using Sentry-specific endpoint logic.
@ -19,16 +22,20 @@ export class CustomEndpointTransport implements BaseTransport {
/** Locks transport after receiving 429 response */
private _disabledUntil: Date = new Date(Date.now());
private readonly _buffer: PromiseBuffer<Response> = new PromiseBuffer(30);
private readonly _buffer: PromiseBuffer<Response>;
constructor(public options: CustomEndpointTransportOptions) {}
constructor(public options: CustomEndpointTransportOptions) {
this._buffer = new PromiseBuffer(options.maxConcurrentRequests ?? DEFAULT_MAX_CONCURRENT_REQUESTS);
}
sendEvent(event: Event): PromiseLike<Response> {
if (new Date(Date.now()) < this._disabledUntil) {
return Promise.reject({
const reason = `Dropping frontend event due to too many requests.`;
console.warn(reason);
return Promise.resolve({
event,
reason: `Transport locked till ${this._disabledUntil} due to too many requests.`,
status: 429,
reason,
status: Status.Skipped,
});
}
@ -74,6 +81,16 @@ export class CustomEndpointTransport implements BaseTransport {
Object.assign(options, this.options.fetchParameters);
}
if (!this._buffer.isReady()) {
const reason = `Dropping frontend log event due to too many requests in flight.`;
console.warn(reason);
return Promise.resolve({
event,
reason,
status: Status.Skipped,
});
}
return this._buffer.add(
() =>
new SyncPromise<Response>((resolve, reject) => {