feat: limit retries to 5xx and 429 (#500)

* feat: limit PutRumEvents retry to 5xx

* feat: add 429

* cleanup

* cleanup

* docs

* doc

* fix: fmt

Author: williazz

src/dispatch/RetryHttpHandler.ts

@@ -1,4 +1,5 @@
 import { HttpHandler, HttpRequest, HttpResponse } from '@aws-sdk/protocol-http';
+import { is2xx, is429, is5xx } from '../plugins/utils/http-utils';
 
 export type BackoffFunction = (retry: number) => number;
 
@@ -30,14 +31,21 @@ export class RetryHttpHandler implements HttpHandler {
         while (true) {
             try {
                 const response = await this.handler.handle(request);
-                if (this.isStatusCode2xx(response.response.statusCode)) {
+                if (is2xx(response.response.statusCode)) {
                     return response;
                 }
-                throw new Error(`${response.response.statusCode}`);
+                throw response.response.statusCode;
             } catch (e) {
-                if (!retriesLeft) {
+                if (typeof e === 'number' && !is429(e) && !is5xx(e)) {
+                    // Fail immediately on client errors because they will never succeed.
+                    // Only retry when request is throttled (429) or received server error (5xx).
+                    throw new Error(`${e}`);
+                }
+
+                if (retriesLeft <= 0) {
                     throw e;
                 }
+
                 retriesLeft--;
                 await this.sleep(this.backoff(this.retries - retriesLeft));
             }
@@ -49,8 +57,4 @@ export class RetryHttpHandler implements HttpHandler {
             setTimeout(resolve, milliseconds)
         );
     }
-
-    private isStatusCode2xx = (statusCode: number): boolean => {
-        return statusCode >= 200 && statusCode < 300;
-    };
 }

src/dispatch/__tests__/RetryHttpHandler.test.ts

@@ -6,7 +6,7 @@ import { RetryHttpHandler } from '../RetryHttpHandler';
 import { FetchHttpHandler } from '@aws-sdk/fetch-http-handler';
 
 const fetchHandler = jest.fn<Promise<Record<string, unknown>>, []>(() =>
-    Promise.resolve({})
+    Promise.resolve({ response: { statusCode: 500 } })
 );
 jest.mock('@aws-sdk/fetch-http-handler', () => ({
     FetchHttpHandler: jest
@@ -93,9 +93,9 @@ describe('RetryHttpHandler tests', () => {
         expect(response).resolves.toBe(success);
     });
 
-    test('when status code is not 2xx then request fails', async () => {
+    test('when status code is 4xx then request fails without retrying', async () => {
         // Init
-        const success = { response: { statusCode: 500 } };
+        const success = { response: { statusCode: 400 } };
         fetchHandler.mockReturnValue(Promise.resolve(success));
 
         const retries = 0;
@@ -118,14 +118,15 @@ describe('RetryHttpHandler tests', () => {
             await client.sendFetch(Utils.PUT_RUM_EVENTS_REQUEST);
         } catch (e) {
             // Assert
-            expect(e.message).toEqual('500');
+            expect(e.message).toEqual('400');
             return;
         }
+        expect(fetchHandler).toHaveBeenCalledTimes(1);
 
         fail('Request should fail');
     });
 
-    test('when status code is not 2xx then request retries', async () => {
+    test('when status code is 5xx then request retries', async () => {
         // Init
         const badStatus = { response: { statusCode: 500 } };
         const okStatus = { response: { statusCode: 200 } };
@@ -157,6 +158,38 @@ describe('RetryHttpHandler tests', () => {
         expect(response).resolves.toBe(okStatus);
     });
 
+    test('when status code is 429 then request retries', async () => {
+        // Init
+        const badStatus = { response: { statusCode: 429 } };
+        const okStatus = { response: { statusCode: 200 } };
+        fetchHandler
+            .mockReturnValueOnce(Promise.resolve(badStatus))
+            .mockReturnValue(Promise.resolve(okStatus));
+
+        const retries = 1;
+        const retryHandler = new RetryHttpHandler(
+            new FetchHttpHandler(),
+            retries,
+            mockBackoff
+        );
+
+        const client: DataPlaneClient = new DataPlaneClient({
+            fetchRequestHandler: retryHandler,
+            beaconRequestHandler: undefined,
+            endpoint: Utils.AWS_RUM_ENDPOINT,
+            region: Utils.AWS_RUM_REGION,
+            credentials: Utils.createAwsCredentials()
+        });
+
+        // Run
+        const response: Promise<{ response: HttpResponse }> = client.sendFetch(
+            Utils.PUT_RUM_EVENTS_REQUEST
+        );
+
+        // Assert
+        expect(response).resolves.toBe(okStatus);
+    });
+
     test('when request fails then retry succeeds after exponential backoff', async () => {
         // Init
         jest.useFakeTimers({ legacyFakeTimers: true });

src/plugins/utils/http-utils.ts

@@ -59,6 +59,8 @@ export const defaultConfig: HttpPluginConfig = {
     addXRayTraceIdHeader: false
 };
 
+export const is2xx = (status: number) => 200 <= status && status < 300;
+
 export const is4xx = (status: number) => {
     return Math.floor(status / 100) === 4;
 };