Skip to content
This repository has been archived by the owner on Dec 4, 2023. It is now read-only.

Improve waiting logic after encountering 429 errors #57

Merged
merged 2 commits into from
Oct 20, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ and this project adheres to

## [Unreleased]

## [2.0.8] - 2021-10-20

- Improved waiting logic after encountering 429 errors

## [2.0.7] - 2021-10-19

- Attempted waiting an additional 60s after the first 429 encountered
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@jupiterone/graph-crowdstrike",
"version": "2.0.7",
"version": "2.0.8",
"description": "A template for JupiterOne graph converters.",
"main": "src/index.js",
"types": "src/index.d.ts",
Expand Down
7 changes: 3 additions & 4 deletions src/crowdstrike/FalconAPIClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ describe('executeAPIRequest', () => {
requestTimesInMs.push(Date.now());
});

const retryAfterTimeInSeconds = Date.now() / 1000 + 1; // server responds with epoch time in seconds, Date.now() returns epoch time in ms
const retryAfterTimeInSeconds = Date.now() / 1000 - 60 + 1; // server responds with epoch time in seconds, Date.now() returns epoch time in ms
recording.server
.any()
.times(1)
Expand All @@ -181,8 +181,7 @@ describe('executeAPIRequest', () => {
expect(requestTimesInMs[1]).toBeGreaterThan(retryAfterTimeInSeconds * 1000);
});

test.skip('retries 429 response limited times', async () => {
// TODO remove temporary 60s wait in FalconAPIClient before re-enabling this test.
test('retries 429 response limited times', async () => {
recording = setupCrowdstrikeRecording({
directory: __dirname,
name: 'executeAPIRequest429limit',
Expand All @@ -193,7 +192,7 @@ describe('executeAPIRequest', () => {
requestTimesInMs.push(Date.now());
});

const retryAfterTimeInSeconds = Date.now() / 1000 - 10; // server responds with epoch time in seconds, Date.now() returns epoch time in ms
const retryAfterTimeInSeconds = Date.now() / 1000 - 60 - 10; // server responds with epoch time in seconds, Date.now() returns epoch time in ms
recording.server.any().intercept((_req, res) => {
res
.status(429)
Expand Down
17 changes: 12 additions & 5 deletions src/crowdstrike/FalconAPIClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -271,8 +271,19 @@ export class FalconAPIClient {

if (response.status === 429) {
const unixTimeNow = getUnixTimeNow();
/**
* We have seen in the wild that waiting until the
* `x-ratelimit-retryafter` unix timestamp before retrying requests
* does often still result in additional 429 errors. This may be caused
* by incorrect logic on the API server, out-of-sync clocks between
* client and server, or something else. However, we have seen that
* waiting an additional minute does result in successful invocations.
*
* `timeToSleepInSeconds` adds 60s to the `retryAfter` property, but
* may be reduced in the future.
*/
const timeToSleepInSeconds = rateLimitState.retryAfter
? rateLimitState.retryAfter - unixTimeNow
? rateLimitState.retryAfter + 60 - unixTimeNow
: 0;
this.logger.info(
{
Expand Down Expand Up @@ -309,10 +320,6 @@ export class FalconAPIClient {
},
'Encountered retryable status code from Crowdstrike API',
);
if (attempts === 2) {
// TODO remove additional 1m of waiting after second 429 encountered
await sleep(60 * 1000);
}
} while (attempts < this.rateLimitConfig.maxAttempts);

throw new Error(`Could not complete request within ${attempts} attempts!`);
Expand Down