From 87e4cb4eee9810823de0458a4c8c6158a1732c08 Mon Sep 17 00:00:00 2001 From: Billy Date: Wed, 27 Mar 2024 11:57:25 -0700 Subject: [PATCH] feat: keep alive when dispatch fails (#524) * feat: disableOnFail * doc: add config doc * Revert "doc: add config doc" This reverts commit ec1f74faf92f3033020234fdbbfffb89e35c0083. * Revert "feat: disableOnFail" This reverts commit 67ed7b3d07a5bea26367e1005aeec3edf3c57367. * feat: only disable RUM when dispatch fails with 403 or 404 * chore: add unit tests * feat: add 401 --- src/dispatch/Dispatch.ts | 11 +- src/dispatch/__tests__/Dispatch.test.ts | 138 ++++++++++++++++++++++-- 2 files changed, 135 insertions(+), 14 deletions(-) diff --git a/src/dispatch/Dispatch.ts b/src/dispatch/Dispatch.ts index b1682367..7fbcc1ce 100644 --- a/src/dispatch/Dispatch.ts +++ b/src/dispatch/Dispatch.ts @@ -38,6 +38,7 @@ export class Dispatch { private dispatchTimerId: number | undefined; private buildClient: ClientBuilder; private config: Config; + private disableCodes = ['401', '403', '404']; constructor( region: string, @@ -232,10 +233,12 @@ export class Dispatch { } private handleReject = (e: any): { response: HttpResponse } => { - // The handler has run out of retries. We adhere to our convention to - // fail safe by disabling dispatch. This ensures that we will not - // continue to attempt requests when the problem is not recoverable. - this.disable(); + if (e instanceof Error && this.disableCodes.includes(e.message)) { + // RUM disables only when dispatch fails and we are certain + // that subsequent attempts will not succeed, such as when + // credentials are invalid or the app monitor does not exist. + this.disable(); + } throw e; }; diff --git a/src/dispatch/__tests__/Dispatch.test.ts b/src/dispatch/__tests__/Dispatch.test.ts index ca67ba97..9433d2a9 100644 --- a/src/dispatch/__tests__/Dispatch.test.ts +++ b/src/dispatch/__tests__/Dispatch.test.ts @@ -419,15 +419,41 @@ describe('Dispatch tests', () => { ); }); - test('when a fetch request is rejected then dispatch is disabled', async () => { + test('when a fetch request is rejected with 429 then dispatch is NOT disabled', async () => { // Init - const ERROR = 'Something went wrong.'; - const sendFetch = jest.fn(() => Promise.reject(ERROR)); - (DataPlaneClient as any).mockImplementation(() => { - return { - sendFetch - }; - }); + (DataPlaneClient as any).mockImplementationOnce(() => ({ + sendFetch: () => Promise.reject(new Error('429')) + })); + + const eventCache: EventCache = + Utils.createDefaultEventCacheWithEvents(); + + const dispatch = new Dispatch( + Utils.AWS_RUM_REGION, + Utils.AWS_RUM_ENDPOINT, + eventCache, + { + ...DEFAULT_CONFIG, + ...{ dispatchInterval: Utils.AUTO_DISPATCH_OFF, retries: 0 } + } + ); + dispatch.setAwsCredentials(Utils.createAwsCredentials()); + + // Run + eventCache.recordEvent('com.amazon.rum.event1', {}); + + // Assert + await expect(dispatch.dispatchFetch()).rejects.toEqual( + new Error('429') + ); + expect((dispatch as unknown as any).enabled).toBe(true); + }); + + test('when a fetch request is rejected with 500 then dispatch is NOT disabled', async () => { + // Init + (DataPlaneClient as any).mockImplementationOnce(() => ({ + sendFetch: () => Promise.reject(new Error('500')) + })); const eventCache: EventCache = Utils.createDefaultEventCacheWithEvents(); @@ -444,11 +470,103 @@ describe('Dispatch tests', () => { dispatch.setAwsCredentials(Utils.createAwsCredentials()); // Run - await expect(dispatch.dispatchFetch()).rejects.toEqual(ERROR); eventCache.recordEvent('com.amazon.rum.event1', {}); // Assert - await expect(dispatch.dispatchFetch()).resolves.toEqual(undefined); + await expect(dispatch.dispatchFetch()).rejects.toEqual( + new Error('500') + ); + expect((dispatch as unknown as any).enabled).toBe(true); + }); + + test('when a fetch request is rejected with 401 then dispatch is disabled', async () => { + // Init + (DataPlaneClient as any).mockImplementationOnce(() => ({ + sendFetch: () => Promise.reject(new Error('401')) + })); + + const eventCache: EventCache = + Utils.createDefaultEventCacheWithEvents(); + + const dispatch = new Dispatch( + Utils.AWS_RUM_REGION, + Utils.AWS_RUM_ENDPOINT, + eventCache, + { + ...DEFAULT_CONFIG, + ...{ dispatchInterval: Utils.AUTO_DISPATCH_OFF, retries: 0 } + } + ); + dispatch.setAwsCredentials(Utils.createAwsCredentials()); + + // Run + eventCache.recordEvent('com.amazon.rum.event1', {}); + + // Assert + await expect(dispatch.dispatchFetch()).rejects.toEqual( + new Error('401') + ); + expect((dispatch as unknown as any).enabled).toBe(false); + }); + + test('when a fetch request is rejected with 403 then dispatch is disabled', async () => { + // Init + (DataPlaneClient as any).mockImplementationOnce(() => ({ + sendFetch: () => Promise.reject(new Error('403')) + })); + + const eventCache: EventCache = + Utils.createDefaultEventCacheWithEvents(); + + const dispatch = new Dispatch( + Utils.AWS_RUM_REGION, + Utils.AWS_RUM_ENDPOINT, + eventCache, + { + ...DEFAULT_CONFIG, + ...{ dispatchInterval: Utils.AUTO_DISPATCH_OFF, retries: 0 } + } + ); + dispatch.setAwsCredentials(Utils.createAwsCredentials()); + + // Run + eventCache.recordEvent('com.amazon.rum.event1', {}); + + // Assert + await expect(dispatch.dispatchFetch()).rejects.toEqual( + new Error('403') + ); + expect((dispatch as unknown as any).enabled).toBe(false); + }); + + test('when a fetch request is rejected with 404 then dispatch is disabled', async () => { + // Init + (DataPlaneClient as any).mockImplementationOnce(() => ({ + sendFetch: () => Promise.reject(new Error('404')) + })); + + const eventCache: EventCache = + Utils.createDefaultEventCacheWithEvents(); + + const dispatch = new Dispatch( + Utils.AWS_RUM_REGION, + Utils.AWS_RUM_ENDPOINT, + eventCache, + { + ...DEFAULT_CONFIG, + ...{ dispatchInterval: Utils.AUTO_DISPATCH_OFF, retries: 0 } + } + ); + dispatch.setAwsCredentials(Utils.createAwsCredentials()); + + // Run + eventCache.recordEvent('com.amazon.rum.event1', {}); + + // Assert + await expect(dispatch.dispatchFetch()).rejects.toEqual( + new Error('404') + ); + expect((dispatch as unknown as any).enabled).toBe(false); }); test('when signing is disabled then credentials are not needed for dispatch', async () => {