Merge pull request #57 from JupiterOne/clean-up-rety-after

Improve waiting logic after encountering 429 errors

CHANGELOG.md

@@ -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

package.json

@@ -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",

src/crowdstrike/FalconAPIClient.test.ts

@@ -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)
@@ -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',
@@ -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)

src/crowdstrike/FalconAPIClient.ts

@@ -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(
           {
@@ -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!`);