Skip to content

Commit

Permalink
feat: 429 http code handling in node/browser transports
Browse files Browse the repository at this point in the history
  • Loading branch information
@kamilogorek
kamilogorek committed Nov 8, 2019
1 parent e880682 commit 08c703d
Show file tree
Hide file tree
Showing 10 changed files with 360 additions and 69 deletions.
40 changes: 36 additions & 4 deletions packages/browser/src/transports/fetch.ts
View file
@@ -1,16 +1,27 @@
import { Event, Response, Status } from '@sentry/types';
import { getGlobalObject, supportsReferrerPolicy } from '@sentry/utils';
import { getGlobalObject, logger, parseRetryAfterHeader, supportsReferrerPolicy, SyncPromise } from '@sentry/utils';

import { BaseTransport } from './base';

const global = getGlobalObject<Window>();

/** `fetch` based transport */
export class FetchTransport extends BaseTransport {
/** Locks transport after receiving 429 response */
private _disabledUntil: Date = new Date(Date.now());

/**
* @inheritDoc
*/
public sendEvent(event: Event): PromiseLike<Response> {
if (new Date(Date.now()) < this._disabledUntil) {
return Promise.reject({
event,
reason: `Transport locked till ${this._disabledUntil} due to too many requests.`,
status: 429,
});
}

const defaultOptions: RequestInit = {
body: JSON.stringify(event),
method: 'POST',
Expand All @@ -22,9 +33,30 @@ export class FetchTransport extends BaseTransport {
};

return this._buffer.add(
global.fetch(this.url, defaultOptions).then(response => ({
status: Status.fromHttpCode(response.status),
})),
new SyncPromise<Response>(async (resolve, reject) => {
let response;
try {
response = await global.fetch(this.url, defaultOptions);
} catch (err) {
reject(err);
return;
}

const status = Status.fromHttpCode(response.status);

if (status === Status.Success) {
resolve({ status });
return;
}

if (status === Status.RateLimit) {
const now = Date.now();
this._disabledUntil = new Date(now + parseRetryAfterHeader(now, response.headers.get('Retry-After')));
logger.warn(`Too many requests, backing off till: ${this._disabledUntil}`);
}

reject(response);
}),
);
}
}
28 changes: 23 additions & 5 deletions packages/browser/src/transports/xhr.ts
View file
@@ -1,14 +1,25 @@
import { Event, Response, Status } from '@sentry/types';
import { SyncPromise } from '@sentry/utils';
import { logger, parseRetryAfterHeader, SyncPromise } from '@sentry/utils';

import { BaseTransport } from './base';

/** `XHR` based transport */
export class XHRTransport extends BaseTransport {
/** Locks transport after receiving 429 response */
private _disabledUntil: Date = new Date(Date.now());

/**
* @inheritDoc
*/
public sendEvent(event: Event): PromiseLike<Response> {
if (new Date(Date.now()) < this._disabledUntil) {
return Promise.reject({
event,
reason: `Transport locked till ${this._disabledUntil} due to too many requests.`,
status: 429,
});
}

return this._buffer.add(
new SyncPromise<Response>((resolve, reject) => {
const request = new XMLHttpRequest();
Expand All @@ -18,10 +29,17 @@ export class XHRTransport extends BaseTransport {
return;
}

if (request.status === 200) {
resolve({
status: Status.fromHttpCode(request.status),
});
const status = Status.fromHttpCode(request.status);

if (status === Status.Success) {
resolve({ status });
return;
}

if (status === Status.RateLimit) {
const now = Date.now();
this._disabledUntil = new Date(now + parseRetryAfterHeader(now, request.getResponseHeader('Retry-After')));
logger.warn(`Too many requests, backing off till: ${this._disabledUntil}`);
}

reject(request);
Expand Down
90 changes: 76 additions & 14 deletions packages/browser/test/unit/transports/fetch.test.ts
View file
Expand Up @@ -37,25 +37,27 @@ describe('FetchTransport', () => {

fetch.returns(Promise.resolve(response));

return transport.sendEvent(payload).then(res => {
expect(res.status).equal(Status.Success);
expect(fetch.calledOnce).equal(true);
expect(
fetch.calledWith(transportUrl, {
body: JSON.stringify(payload),
method: 'POST',
referrerPolicy: 'origin',
}),
).equal(true);
});
const res = await transport.sendEvent(payload);

expect(res.status).equal(Status.Success);
expect(fetch.calledOnce).equal(true);
expect(
fetch.calledWith(transportUrl, {
body: JSON.stringify(payload),
method: 'POST',
referrerPolicy: 'origin',
}),
).equal(true);
});

it('rejects with non-200 status code', async () => {
const response = { status: 403 };

fetch.returns(Promise.reject(response));
fetch.returns(Promise.resolve(response));

return transport.sendEvent(payload).then(null, res => {
try {
await transport.sendEvent(payload);
} catch (res) {
expect(res.status).equal(403);
expect(fetch.calledOnce).equal(true);
expect(
Expand All @@ -65,7 +67,67 @@ describe('FetchTransport', () => {
referrerPolicy: 'origin',
}),
).equal(true);
});
}
});

it('pass the error to rejection when fetch fails', async () => {
const response = { status: 403 };

fetch.returns(Promise.reject(response));

try {
await transport.sendEvent(payload);
} catch (res) {
expect(res).equal(response);
}
});

it('back-off using Retry-After header', async () => {
const retryAfterSeconds = 10;
const headers = new Map();
headers.set('Retry-After', retryAfterSeconds);
const response = { status: 429, headers };
fetch.returns(Promise.resolve(response));

const now = Date.now();
const dateStub = stub(Date, 'now')
// Check for first event
.onCall(0)
.returns(now)
// Setting disableUntil
.onCall(1)
.returns(now)
// Check for second event
.onCall(2)
.returns(now + (retryAfterSeconds / 2) * 1000)
// Check for third event
.onCall(3)
.returns(now + retryAfterSeconds * 1000);

try {
await transport.sendEvent(payload);
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(undefined);
}

try {
await transport.sendEvent(payload);
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
);
}

try {
await transport.sendEvent(payload);
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(undefined);
}

dateStub.restore();
});
});
});
71 changes: 58 additions & 13 deletions packages/browser/test/unit/transports/xhr.test.ts
View file
@@ -1,5 +1,5 @@
import { expect } from 'chai';
import { fakeServer, SinonFakeServer } from 'sinon';
import { fakeServer, SinonFakeServer, stub } from 'sinon';

import { Status, Transports } from '../../../src';

Expand Down Expand Up @@ -35,27 +35,72 @@ describe('XHRTransport', () => {
it('sends a request to Sentry servers', async () => {
server.respondWith('POST', transportUrl, [200, {}, '']);

return transport.sendEvent(payload).then(res => {
expect(res.status).equal(Status.Success);
const request = server.requests[0];
expect(server.requests.length).equal(1);
expect(request.method).equal('POST');
expect(JSON.parse(request.requestBody)).deep.equal(payload);
});
const res = await transport.sendEvent(payload);

expect(res.status).equal(Status.Success);
const request = server.requests[0];
expect(server.requests.length).equal(1);
expect(request.method).equal('POST');
expect(JSON.parse(request.requestBody)).deep.equal(payload);
});

it('rejects with non-200 status code', done => {
it('rejects with non-200 status code', async () => {
server.respondWith('POST', transportUrl, [403, {}, '']);

transport.sendEvent(payload).then(null, res => {
try {
await transport.sendEvent(payload);
} catch (res) {
expect(res.status).equal(403);

const request = server.requests[0];
expect(server.requests.length).equal(1);
expect(request.method).equal('POST');
expect(JSON.parse(request.requestBody)).deep.equal(payload);
done();
});
}
});

it('back-off using Retry-After header', async () => {
const retryAfterSeconds = 10;
server.respondWith('POST', transportUrl, [429, { 'Retry-After': retryAfterSeconds }, '']);

const now = Date.now();
const dateStub = stub(Date, 'now')
// Check for first event
.onCall(0)
.returns(now)
// Setting disableUntil
.onCall(1)
.returns(now)
// Check for second event
.onCall(2)
.returns(now + (retryAfterSeconds / 2) * 1000)
// Check for third event
.onCall(3)
.returns(now + retryAfterSeconds * 1000);

try {
await transport.sendEvent(payload);
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(undefined);
}

try {
await transport.sendEvent(payload);
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
);
}

try {
await transport.sendEvent(payload);
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(undefined);
}

dateStub.restore();
});
});
});
38 changes: 28 additions & 10 deletions packages/node/src/transports/base.ts
View file
@@ -1,6 +1,6 @@
import { API } from '@sentry/core';
import { Event, Response, Status, Transport, TransportOptions } from '@sentry/types';
import { PromiseBuffer, SentryError } from '@sentry/utils';
import { logger, parseRetryAfterHeader, PromiseBuffer, SentryError } from '@sentry/utils';
import * as fs from 'fs';
import * as http from 'http';
import * as https from 'https';
Expand Down Expand Up @@ -38,6 +38,9 @@ export abstract class BaseTransport implements Transport {
/** A simple buffer holding all requests. */
protected readonly _buffer: PromiseBuffer<Response> = new PromiseBuffer(30);

/** Locks transport after receiving 429 response */
private _disabledUntil: Date = new Date(Date.now());

/** Create instance and set this.dsn */
public constructor(public options: TransportOptions) {
this._api = new API(options.dsn);
Expand Down Expand Up @@ -72,26 +75,41 @@ export abstract class BaseTransport implements Transport {

/** JSDoc */
protected async _sendWithModule(httpModule: HTTPRequest, event: Event): Promise<Response> {
if (new Date(Date.now()) < this._disabledUntil) {
return Promise.reject(new SentryError(`Transport locked till ${this._disabledUntil} due to too many requests.`));
}

if (!this._buffer.isReady()) {
return Promise.reject(new SentryError('Not adding Promise due to buffer limit reached.'));
}
return this._buffer.add(
new Promise<Response>((resolve, reject) => {
const req = httpModule.request(this._getRequestOptions(), (res: http.IncomingMessage) => {
const statusCode = res.statusCode || 500;
const status = Status.fromHttpCode(statusCode);

res.setEncoding('utf8');
if (res.statusCode && res.statusCode >= 200 && res.statusCode < 300) {
resolve({
status: Status.fromHttpCode(res.statusCode),
});

if (status === Status.Success) {
resolve({ status });
} else {
if (status === Status.RateLimit) {
const now = Date.now();
let header = res.headers ? res.headers['Retry-After'] : '';
header = Array.isArray(header) ? header[0] : header;
this._disabledUntil = new Date(now + parseRetryAfterHeader(now, header));
logger.warn(`Too many requests, backing off till: ${this._disabledUntil}`);
}

let rejectionMessage = `HTTP Error (${statusCode})`;
if (res.headers && res.headers['x-sentry-error']) {
const reason = res.headers['x-sentry-error'];
reject(new SentryError(`HTTP Error (${res.statusCode}): ${reason}`));
} else {
reject(new SentryError(`HTTP Error (${res.statusCode})`));
rejectionMessage += `: ${res.headers['x-sentry-error']}`;
}

reject(new SentryError(rejectionMessage));
}
// force the socket to drain

// Force the socket to drain
res.on('data', () => {
// Drain
});
Expand Down

0 comments on commit 08c703d

Please sign in to comment.