Skip to content

ref: Log which request type has been limited #3687

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.

Sign up for GitHub

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 2 commits into from
Jun 16, 2021
Merged

ref: Log which request type has been limited #3687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0a26ba2
ref: Log which request type has been limited
kamilogorek Jun 16, 2021
8ac7e70
Update tests
kamilogorek Jun 16, 2021
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
3 changes: 2 additions & 1 deletion packages/browser/src/transports/base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ export abstract class BaseTransport implements Transport {
* https://developer.mozilla.org/en-US/docs/Web/API/Headers/get
*/
const limited = this._handleRateLimit(headers);
if (limited) logger.warn(`Too many requests, backing off until: ${this._disabledUntil(requestType)}`);
if (limited)
logger.warn(`Too many ${requestType} requests, backing off until: ${this._disabledUntil(requestType)}`);

if (status === Status.Success) {
resolve({ status });
Expand Down
4 changes: 3 additions & 1 deletion packages/browser/src/transports/fetch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@ export class FetchTransport extends BaseTransport {
return Promise.reject({
event: originalPayload,
type: sentryRequest.type,
reason: `Transport locked till ${this._disabledUntil(sentryRequest.type)} due to too many requests.`,
reason: `Transport for ${sentryRequest.type} requests locked till ${this._disabledUntil(
sentryRequest.type,
)} due to too many requests.`,
status: 429,
});
}
Expand Down
4 changes: 3 additions & 1 deletion packages/browser/src/transports/xhr.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ export class XHRTransport extends BaseTransport {
return Promise.reject({
event: originalPayload,
type: sentryRequest.type,
reason: `Transport locked till ${this._disabledUntil(sentryRequest.type)} due to too many requests.`,
reason: `Transport for ${sentryRequest.type} requests locked till ${this._disabledUntil(
sentryRequest.type,
)} due to too many requests.`,
status: 429,
});
}
Expand Down
28 changes: 21 additions & 7 deletions packages/browser/test/unit/transports/fetch.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,9 @@ describe('FetchTransport', () => {
throw new Error('unreachable!');
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(`Transport locked till ${new Date(afterLimit)} due to too many requests.`);
expect(res.reason).equal(
`Transport for event requests locked till ${new Date(afterLimit)} due to too many requests.`,
);
expect(fetch.calledOnce).equal(true);
}

Expand Down Expand Up @@ -256,7 +258,9 @@ describe('FetchTransport', () => {
throw new Error('unreachable!');
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(`Transport locked till ${new Date(afterLimit)} due to too many requests.`);
expect(res.reason).equal(
`Transport for event requests locked till ${new Date(afterLimit)} due to too many requests.`,
);
expect(fetch.calledTwice).equal(true);
}

Expand Down Expand Up @@ -317,7 +321,9 @@ describe('FetchTransport', () => {
throw new Error('unreachable!');
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(`Transport locked till ${new Date(afterLimit)} due to too many requests.`);
expect(res.reason).equal(
`Transport for event requests locked till ${new Date(afterLimit)} due to too many requests.`,
);
expect(fetch.calledOnce).equal(true);
}

Expand All @@ -326,7 +332,9 @@ describe('FetchTransport', () => {
throw new Error('unreachable!');
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(`Transport locked till ${new Date(afterLimit)} due to too many requests.`);
expect(res.reason).equal(
`Transport for transaction requests locked till ${new Date(afterLimit)} due to too many requests.`,
);
expect(fetch.calledOnce).equal(true);
}

Expand Down Expand Up @@ -393,7 +401,9 @@ describe('FetchTransport', () => {
throw new Error('unreachable!');
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(`Transport locked till ${new Date(afterLimit)} due to too many requests.`);
expect(res.reason).equal(
`Transport for event requests locked till ${new Date(afterLimit)} due to too many requests.`,
);
expect(fetch.calledOnce).equal(true);
}

Expand All @@ -402,7 +412,9 @@ describe('FetchTransport', () => {
throw new Error('unreachable!');
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(`Transport locked till ${new Date(afterLimit)} due to too many requests.`);
expect(res.reason).equal(
`Transport for transaction requests locked till ${new Date(afterLimit)} due to too many requests.`,
);
expect(fetch.calledOnce).equal(true);
}

Expand Down Expand Up @@ -455,7 +467,9 @@ describe('FetchTransport', () => {
throw new Error('unreachable!');
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(`Transport locked till ${new Date(afterLimit)} due to too many requests.`);
expect(res.reason).equal(
`Transport for event requests locked till ${new Date(afterLimit)} due to too many requests.`,
);
expect(fetch.calledOnce).equal(true);
}

Expand Down
28 changes: 21 additions & 7 deletions packages/browser/test/unit/transports/xhr.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,9 @@ describe('XHRTransport', () => {
throw new Error('unreachable!');
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(`Transport locked till ${new Date(afterLimit)} due to too many requests.`);
expect(res.reason).equal(
`Transport for event requests locked till ${new Date(afterLimit)} due to too many requests.`,
);
expect(server.requests.length).equal(1);
}

Expand Down Expand Up @@ -190,7 +192,9 @@ describe('XHRTransport', () => {
throw new Error('unreachable!');
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(`Transport locked till ${new Date(afterLimit)} due to too many requests.`);
expect(res.reason).equal(
`Transport for event requests locked till ${new Date(afterLimit)} due to too many requests.`,
);
expect(server.requests.length).equal(2);
}

Expand Down Expand Up @@ -256,7 +260,9 @@ describe('XHRTransport', () => {
throw new Error('unreachable!');
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(`Transport locked till ${new Date(afterLimit)} due to too many requests.`);
expect(res.reason).equal(
`Transport for event requests locked till ${new Date(afterLimit)} due to too many requests.`,
);
expect(server.requests.length).equal(1);
}

Expand All @@ -265,7 +271,9 @@ describe('XHRTransport', () => {
throw new Error('unreachable!');
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(`Transport locked till ${new Date(afterLimit)} due to too many requests.`);
expect(res.reason).equal(
`Transport for transaction requests locked till ${new Date(afterLimit)} due to too many requests.`,
);
expect(server.requests.length).equal(1);
}

Expand Down Expand Up @@ -332,7 +340,9 @@ describe('XHRTransport', () => {
throw new Error('unreachable!');
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(`Transport locked till ${new Date(afterLimit)} due to too many requests.`);
expect(res.reason).equal(
`Transport for event requests locked till ${new Date(afterLimit)} due to too many requests.`,
);
expect(server.requests.length).equal(1);
}

Expand All @@ -341,7 +351,9 @@ describe('XHRTransport', () => {
throw new Error('unreachable!');
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(`Transport locked till ${new Date(afterLimit)} due to too many requests.`);
expect(res.reason).equal(
`Transport for transaction requests locked till ${new Date(afterLimit)} due to too many requests.`,
);
expect(server.requests.length).equal(1);
}

Expand Down Expand Up @@ -393,7 +405,9 @@ describe('XHRTransport', () => {
throw new Error('unreachable!');
} catch (res) {
expect(res.status).equal(429);
expect(res.reason).equal(`Transport locked till ${new Date(afterLimit)} due to too many requests.`);
expect(res.reason).equal(
`Transport for event requests locked till ${new Date(afterLimit)} due to too many requests.`,
);
expect(server.requests.length).equal(1);
}

Expand Down
19 changes: 12 additions & 7 deletions packages/node/src/transports/base/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,17 +182,19 @@ export abstract class BaseTransport implements Transport {

/** JSDoc */
protected async _send(
sentryReq: SentryRequest,
sentryRequest: SentryRequest,
originalPayload?: Event | Session | SessionAggregates,
): Promise<Response> {
if (!this.module) {
throw new SentryError('No module available');
}
if (originalPayload && this._isRateLimited(sentryReq.type)) {
if (originalPayload && this._isRateLimited(sentryRequest.type)) {
return Promise.reject({
payload: originalPayload,
type: sentryReq.type,
reason: `Transport locked till ${this._disabledUntil(sentryReq.type)} due to too many requests.`,
type: sentryRequest.type,
reason: `Transport for ${sentryRequest.type} requests locked till ${this._disabledUntil(
sentryRequest.type,
)} due to too many requests.`,
status: 429,
});
}
Expand All @@ -205,7 +207,7 @@ export abstract class BaseTransport implements Transport {
if (!this.module) {
throw new SentryError('No module available');
}
const options = this._getRequestOptions(this.urlParser(sentryReq.url));
const options = this._getRequestOptions(this.urlParser(sentryRequest.url));
const req = this.module.request(options, res => {
const statusCode = res.statusCode || 500;
const status = Status.fromHttpCode(statusCode);
Expand All @@ -228,7 +230,10 @@ export abstract class BaseTransport implements Transport {
};

const limited = this._handleRateLimit(headers);
if (limited) logger.warn(`Too many requests, backing off until: ${this._disabledUntil(sentryReq.type)}`);
if (limited)
logger.warn(
`Too many ${sentryRequest.type} requests, backing off until: ${this._disabledUntil(sentryRequest.type)}`,
);

if (status === Status.Success) {
resolve({ status });
Expand All @@ -249,7 +254,7 @@ export abstract class BaseTransport implements Transport {
});
});
req.on('error', reject);
req.end(sentryReq.body);
req.end(sentryRequest.body);
}),
);
}
Expand Down
44 changes: 33 additions & 11 deletions packages/node/test/transports/http.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,9 @@ describe('HTTPTransport', () => {
} catch (e) {
expect(e.status).toEqual(429);
expect(e.reason).toEqual(
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
`Transport for event requests locked till ${new Date(
now + retryAfterSeconds * 1000,
)} due to too many requests.`,
);
expect(e.payload.message).toEqual('test');
expect(e.type).toEqual('event');
Expand Down Expand Up @@ -257,7 +259,9 @@ describe('HTTPTransport', () => {
} catch (e) {
expect(e.status).toEqual(429);
expect(e.reason).toEqual(
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
`Transport for event requests locked till ${new Date(
now + retryAfterSeconds * 1000,
)} due to too many requests.`,
);
expect(e.payload).toEqual(eventPayload);
expect(e.type).toEqual('event');
Expand All @@ -268,7 +272,9 @@ describe('HTTPTransport', () => {
} catch (e) {
expect(e.status).toEqual(429);
expect(e.reason).toEqual(
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
`Transport for transaction requests locked till ${new Date(
now + retryAfterSeconds * 1000,
)} due to too many requests.`,
);
expect(e.payload).toEqual(transactionPayload);
expect(e.type).toEqual('transaction');
Expand Down Expand Up @@ -335,7 +341,9 @@ describe('HTTPTransport', () => {
} catch (e) {
expect(e.status).toEqual(429);
expect(e.reason).toEqual(
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
`Transport for event requests locked till ${new Date(
now + retryAfterSeconds * 1000,
)} due to too many requests.`,
);
expect(e.payload).toEqual(eventPayload);
expect(e.type).toEqual('event');
Expand Down Expand Up @@ -391,7 +399,9 @@ describe('HTTPTransport', () => {
} catch (e) {
expect(e.status).toEqual(429);
expect(e.reason).toEqual(
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
`Transport for event requests locked till ${new Date(
now + retryAfterSeconds * 1000,
)} due to too many requests.`,
);
expect(e.payload).toEqual(eventPayload);
expect(e.type).toEqual('event');
Expand All @@ -402,7 +412,9 @@ describe('HTTPTransport', () => {
} catch (e) {
expect(e.status).toEqual(429);
expect(e.reason).toEqual(
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
`Transport for session requests locked till ${new Date(
now + retryAfterSeconds * 1000,
)} due to too many requests.`,
);
expect(e.payload.environment).toEqual(sessionPayload.environment);
expect(e.payload.release).toEqual(sessionPayload.release);
Expand All @@ -415,7 +427,9 @@ describe('HTTPTransport', () => {
} catch (e) {
expect(e.status).toEqual(429);
expect(e.reason).toEqual(
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
`Transport for transaction requests locked till ${new Date(
now + retryAfterSeconds * 1000,
)} due to too many requests.`,
);
expect(e.payload).toEqual(transactionPayload);
expect(e.type).toEqual('transaction');
Expand Down Expand Up @@ -474,7 +488,9 @@ describe('HTTPTransport', () => {
} catch (e) {
expect(e.status).toEqual(429);
expect(e.reason).toEqual(
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
`Transport for event requests locked till ${new Date(
now + retryAfterSeconds * 1000,
)} due to too many requests.`,
);
expect(e.payload).toEqual(eventPayload);
expect(e.type).toEqual('event');
Expand All @@ -485,7 +501,9 @@ describe('HTTPTransport', () => {
} catch (e) {
expect(e.status).toEqual(429);
expect(e.reason).toEqual(
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
`Transport for transaction requests locked till ${new Date(
now + retryAfterSeconds * 1000,
)} due to too many requests.`,
);
expect(e.payload).toEqual(transactionPayload);
expect(e.type).toEqual('transaction');
Expand Down Expand Up @@ -539,7 +557,9 @@ describe('HTTPTransport', () => {
} catch (e) {
expect(e.status).toEqual(429);
expect(e.reason).toEqual(
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
`Transport for event requests locked till ${new Date(
now + retryAfterSeconds * 1000,
)} due to too many requests.`,
);
expect(e.payload).toEqual(eventPayload);
expect(e.type).toEqual('event');
Expand Down Expand Up @@ -586,7 +606,9 @@ describe('HTTPTransport', () => {
} catch (e) {
expect(e.status).toEqual(429);
expect(e.reason).toEqual(
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
`Transport for event requests locked till ${new Date(
now + retryAfterSeconds * 1000,
)} due to too many requests.`,
);
expect(e.payload).toEqual(eventPayload);
expect(e.type).toEqual('event');
Expand Down
4 changes: 3 additions & 1 deletion packages/node/test/transports/https.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,9 @@ describe('HTTPSTransport', () => {
} catch (e) {
expect(e.status).toEqual(429);
expect(e.reason).toEqual(
`Transport locked till ${new Date(now + retryAfterSeconds * 1000)} due to too many requests.`,
`Transport for event requests locked till ${new Date(
now + retryAfterSeconds * 1000,
)} due to too many requests.`,
);
expect(e.payload.message).toEqual('test');
expect(e.type).toEqual('event');
Expand Down