Skip to content

Commit 08c703d

Browse files
kamilogorekkamilogorek
committed
feat: 429 http code handling in node/browser transports
1 parent e880682 commit 08c703d

File tree

10 files changed

+360
-69
lines changed

10 files changed

+360
-69
lines changed

‎packages/browser/src/transports/fetch.ts

+36-4
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,27 @@
11
import { Event, Response, Status } from '@sentry/types';
2-
import { getGlobalObject, supportsReferrerPolicy } from '@sentry/utils';
2+
import { getGlobalObject, logger, parseRetryAfterHeader, supportsReferrerPolicy, SyncPromise } from '@sentry/utils';
33

44
import { BaseTransport } from './base';
55

66
const global = getGlobalObject<Window>();
77

88
/** `fetch` based transport */
99
export class FetchTransport extends BaseTransport {
10+
/** Locks transport after receiving 429 response */
11+
private _disabledUntil: Date = new Date(Date.now());
12+
1013
/**
1114
* @inheritDoc
1215
*/
1316
public sendEvent(event: Event): PromiseLike<Response> {
17+
if (new Date(Date.now()) < this._disabledUntil) {
18+
return Promise.reject({
19+
event,
20+
reason: `Transport locked till ${this._disabledUntil} due to too many requests.`,
21+
status: 429,
22+
});
23+
}
24+
1425
const defaultOptions: RequestInit = {
1526
body: JSON.stringify(event),
1627
method: 'POST',
@@ -22,9 +33,30 @@ export class FetchTransport extends BaseTransport {
2233
};
2334

2435
return this._buffer.add(
25-
global.fetch(this.url, defaultOptions).then(response => ({
26-
status: Status.fromHttpCode(response.status),
27-
})),
36+
new SyncPromise<Response>(async (resolve, reject) => {
37+
let response;
38+
try {
39+
response = await global.fetch(this.url, defaultOptions);
40+
} catch (err) {
41+
reject(err);
42+
return;
43+
}
44+
45+
const status = Status.fromHttpCode(response.status);
46+
47+
if (status === Status.Success) {
48+
resolve({ status });
49+
return;
50+
}
51+
52+
if (status === Status.RateLimit) {
53+
const now = Date.now();
54+
this._disabledUntil = new Date(now + parseRetryAfterHeader(now, response.headers.get('Retry-After')));
55+
logger.warn(`Too many requests, backing off till: ${this._disabledUntil}`);
56+
}
57+
58+
reject(response);
59+
}),
2860
);
2961
}
3062
}

‎packages/browser/src/transports/xhr.ts

+23-5
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,25 @@
11
import { Event, Response, Status } from '@sentry/types';
2-
import { SyncPromise } from '@sentry/utils';
2+
import { logger, parseRetryAfterHeader, SyncPromise } from '@sentry/utils';
33

44
import { BaseTransport } from './base';
55

66
/** `XHR` based transport */
77
export class XHRTransport extends BaseTransport {
8+
/** Locks transport after receiving 429 response */
9+
private _disabledUntil: Date = new Date(Date.now());
10+
811
/**
912
* @inheritDoc
1013
*/
1114
public sendEvent(event: Event): PromiseLike<Response> {
15+
if (new Date(Date.now()) < this._disabledUntil) {
16+
return Promise.reject({
17+
event,
18+
reason: `Transport locked till ${this._disabledUntil} due to too many requests.`,
19+
status: 429,
20+
});
21+
}
22+
1223
return this._buffer.add(
1324
new SyncPromise<Response>((resolve, reject) => {
1425
const request = new XMLHttpRequest();
@@ -18,10 +29,17 @@ export class XHRTransport extends BaseTransport {
1829
return;
1930
}
2031

21-
if (request.status === 200) {
22-
resolve({
23-
status: Status.fromHttpCode(request.status),
24-
});
32+
const status = Status.fromHttpCode(request.status);
33+
34+
if (status === Status.Success) {
35+
resolve({ status });
36+
return;
37+
}
38+
39+
if (status === Status.RateLimit) {
40+
const now = Date.now();
41+
this._disabledUntil = new Date(now + parseRetryAfterHeader(now, request.getResponseHeader('Retry-After')));
42+
logger.warn(`Too many requests, backing off till: ${this._disabledUntil}`);
2543
}
2644

2745
reject(request);

‎packages/browser/test/unit/transports/fetch.test.ts

+76-14
Original file line numberDiff line numberDiff line change
@@ -37,25 +37,27 @@ describe('FetchTransport', () => {
3737

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

40-
return transport.sendEvent(payload).then(res => {
41-
expect(res.status).equal(Status.Success);
42-
expect(fetch.calledOnce).equal(true);
43-
expect(
44-
fetch.calledWith(transportUrl, {
45-
body: JSON.stringify(payload),
46-
method: 'POST',
47-
referrerPolicy: 'origin',
48-
}),
49-
).equal(true);
50-
});
40+
const res = await transport.sendEvent(payload);
41+
42+
expect(res.status).equal(Status.Success);
43+
expect(fetch.calledOnce).equal(true);
44+
expect(
45+
fetch.calledWith(transportUrl, {
46+
body: JSON.stringify(payload),
47+
method: 'POST',
48+
referrerPolicy: 'origin',
49+
}),
50+
).equal(true);
5151
});
5252

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

56-
fetch.returns(Promise.reject(response));
56+
fetch.returns(Promise.resolve(response));
5757

58-
return transport.sendEvent(payload).then(null, res => {
58+
try {
59+
await transport.sendEvent(payload);
60+
} catch (res) {
5961
expect(res.status).equal(403);
6062
expect(fetch.calledOnce).equal(true);
6163
expect(
@@ -65,7 +67,67 @@ describe('FetchTransport', () => {
6567
referrerPolicy: 'origin',
6668
}),
6769
).equal(true);
68-
});
70+
}
71+
});
72+
73+
it('pass the error to rejection when fetch fails', async () => {
74+
const response = { status: 403 };
75+
76+
fetch.returns(Promise.reject(response));
77+
78+
try {
79+
await transport.sendEvent(payload);
80+
} catch (res) {
81+
expect(res).equal(response);
82+
}
83+
});
84+
85+
it('back-off using Retry-After header', async () => {
86+
const retryAfterSeconds = 10;
87+
const headers = new Map();
88+
headers.set('Retry-After', retryAfterSeconds);
89+
const response = { status: 429, headers };
90+
fetch.returns(Promise.resolve(response));
91+
92+
const now = Date.now();
93+
const dateStub = stub(Date, 'now')
94+
// Check for first event
95+
.onCall(0)
96+
.returns(now)
97+
// Setting disableUntil
98+
.onCall(1)
99+
.returns(now)
100+
// Check for second event
101+
.onCall(2)
102+
.returns(now + (retryAfterSeconds / 2) * 1000)
103+
// Check for third event
104+
.onCall(3)
105+
.returns(now + retryAfterSeconds * 1000);
106+
107+
try {
108+
await transport.sendEvent(payload);
109+
} catch (res) {
110+
expect(res.status).equal(429);
111+
expect(res.reason).equal(undefined);
112+
}
113+
114+
try {
115+
await transport.sendEvent(payload);
116+
} catch (res) {
117+
expect(res.status).equal(429);
118+
expect(res.reason).equal(
119+
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
120+
);
121+
}
122+
123+
try {
124+
await transport.sendEvent(payload);
125+
} catch (res) {
126+
expect(res.status).equal(429);
127+
expect(res.reason).equal(undefined);
128+
}
129+
130+
dateStub.restore();
69131
});
70132
});
71133
});

‎packages/browser/test/unit/transports/xhr.test.ts

+58-13
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from 'chai';
2-
import { fakeServer, SinonFakeServer } from 'sinon';
2+
import { fakeServer, SinonFakeServer, stub } from 'sinon';
33

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

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

38-
return transport.sendEvent(payload).then(res => {
39-
expect(res.status).equal(Status.Success);
40-
const request = server.requests[0];
41-
expect(server.requests.length).equal(1);
42-
expect(request.method).equal('POST');
43-
expect(JSON.parse(request.requestBody)).deep.equal(payload);
44-
});
38+
const res = await transport.sendEvent(payload);
39+
40+
expect(res.status).equal(Status.Success);
41+
const request = server.requests[0];
42+
expect(server.requests.length).equal(1);
43+
expect(request.method).equal('POST');
44+
expect(JSON.parse(request.requestBody)).deep.equal(payload);
4545
});
4646

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

50-
transport.sendEvent(payload).then(null, res => {
50+
try {
51+
await transport.sendEvent(payload);
52+
} catch (res) {
5153
expect(res.status).equal(403);
52-
5354
const request = server.requests[0];
5455
expect(server.requests.length).equal(1);
5556
expect(request.method).equal('POST');
5657
expect(JSON.parse(request.requestBody)).deep.equal(payload);
57-
done();
58-
});
58+
}
59+
});
60+
61+
it('back-off using Retry-After header', async () => {
62+
const retryAfterSeconds = 10;
63+
server.respondWith('POST', transportUrl, [429, { 'Retry-After': retryAfterSeconds }, '']);
64+
65+
const now = Date.now();
66+
const dateStub = stub(Date, 'now')
67+
// Check for first event
68+
.onCall(0)
69+
.returns(now)
70+
// Setting disableUntil
71+
.onCall(1)
72+
.returns(now)
73+
// Check for second event
74+
.onCall(2)
75+
.returns(now + (retryAfterSeconds / 2) * 1000)
76+
// Check for third event
77+
.onCall(3)
78+
.returns(now + retryAfterSeconds * 1000);
79+
80+
try {
81+
await transport.sendEvent(payload);
82+
} catch (res) {
83+
expect(res.status).equal(429);
84+
expect(res.reason).equal(undefined);
85+
}
86+
87+
try {
88+
await transport.sendEvent(payload);
89+
} catch (res) {
90+
expect(res.status).equal(429);
91+
expect(res.reason).equal(
92+
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
93+
);
94+
}
95+
96+
try {
97+
await transport.sendEvent(payload);
98+
} catch (res) {
99+
expect(res.status).equal(429);
100+
expect(res.reason).equal(undefined);
101+
}
102+
103+
dateStub.restore();
59104
});
60105
});
61106
});

‎packages/node/src/transports/base.ts

+28-10
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { API } from '@sentry/core';
22
import { Event, Response, Status, Transport, TransportOptions } from '@sentry/types';
3-
import { PromiseBuffer, SentryError } from '@sentry/utils';
3+
import { logger, parseRetryAfterHeader, PromiseBuffer, SentryError } from '@sentry/utils';
44
import * as fs from 'fs';
55
import * as http from 'http';
66
import * as https from 'https';
@@ -38,6 +38,9 @@ export abstract class BaseTransport implements Transport {
3838
/** A simple buffer holding all requests. */
3939
protected readonly _buffer: PromiseBuffer<Response> = new PromiseBuffer(30);
4040

41+
/** Locks transport after receiving 429 response */
42+
private _disabledUntil: Date = new Date(Date.now());
43+
4144
/** Create instance and set this.dsn */
4245
public constructor(public options: TransportOptions) {
4346
this._api = new API(options.dsn);
@@ -72,26 +75,41 @@ export abstract class BaseTransport implements Transport {
7275

7376
/** JSDoc */
7477
protected async _sendWithModule(httpModule: HTTPRequest, event: Event): Promise<Response> {
78+
if (new Date(Date.now()) < this._disabledUntil) {
79+
return Promise.reject(new SentryError(`Transport locked till ${this._disabledUntil} due to too many requests.`));
80+
}
81+
7582
if (!this._buffer.isReady()) {
7683
return Promise.reject(new SentryError('Not adding Promise due to buffer limit reached.'));
7784
}
7885
return this._buffer.add(
7986
new Promise<Response>((resolve, reject) => {
8087
const req = httpModule.request(this._getRequestOptions(), (res: http.IncomingMessage) => {
88+
const statusCode = res.statusCode || 500;
89+
const status = Status.fromHttpCode(statusCode);
90+
8191
res.setEncoding('utf8');
82-
if (res.statusCode && res.statusCode >= 200 && res.statusCode < 300) {
83-
resolve({
84-
status: Status.fromHttpCode(res.statusCode),
85-
});
92+
93+
if (status === Status.Success) {
94+
resolve({ status });
8695
} else {
96+
if (status === Status.RateLimit) {
97+
const now = Date.now();
98+
let header = res.headers ? res.headers['Retry-After'] : '';
99+
header = Array.isArray(header) ? header[0] : header;
100+
this._disabledUntil = new Date(now + parseRetryAfterHeader(now, header));
101+
logger.warn(`Too many requests, backing off till: ${this._disabledUntil}`);
102+
}
103+
104+
let rejectionMessage = `HTTP Error (${statusCode})`;
87105
if (res.headers && res.headers['x-sentry-error']) {
88-
const reason = res.headers['x-sentry-error'];
89-
reject(new SentryError(`HTTP Error (${res.statusCode}): ${reason}`));
90-
} else {
91-
reject(new SentryError(`HTTP Error (${res.statusCode})`));
106+
rejectionMessage += `: ${res.headers['x-sentry-error']}`;
92107
}
108+
109+
reject(new SentryError(rejectionMessage));
93110
}
94-
// force the socket to drain
111+
112+
// Force the socket to drain
95113
res.on('data', () => {
96114
// Drain
97115
});

0 commit comments

Comments
 (0)