From fe6557b4fa72e0a8df52a01564f4a2b4433c154c Mon Sep 17 00:00:00 2001 From: Alan Norbauer Date: Sun, 29 Oct 2023 19:12:46 -0700 Subject: [PATCH] increase default retry delay Some of the graph APIs do not provide the `retry-after` header in HTTP 429 throttling responses. The default behavior of this SDK is to retry 3 times, with exponential backoff. The problem with this approach is that the max retry of 3 attempts (with a delay of ~8 seconds) just isn't enough for some APIs. For example, OneNote's API will potentially return 429s for 1 hour. To fix this, let us create some new defaults: | Thing | Old Value | New Value | Reasoning | | ------------------- | ----------- | -------------- | ------------------------------------------------------------------------------------------------------------------------- | | DEFAULT_MAX_RETRIES | 3 | 12 | 2^12 seconds is just over 1 hour, which should be enough for OneNote | | MAX_DELAY | 180 seconds | 3_600 (1 hour) | This makes it so that retry 12+ only wait 1 hour between retries instead of continuing to backoff the delay exponentially | | MAX_MAX_RETRIES | 10 | 64 | This needs to be higher than DEFAULT_MAX_RETRIES but the choice of 64 is arbitrary | See [this GitHub issue](https://github.com/microsoftgraph/msgraph-sdk-javascript/issues/978) for more discussion. --- src/middleware/options/RetryHandlerOptions.ts | 8 ++++---- test/common/middleware/RetryHandlerOptions.ts | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/middleware/options/RetryHandlerOptions.ts b/src/middleware/options/RetryHandlerOptions.ts index 8191d165c..6825266e3 100644 --- a/src/middleware/options/RetryHandlerOptions.ts +++ b/src/middleware/options/RetryHandlerOptions.ts @@ -37,21 +37,21 @@ export class RetryHandlerOptions implements MiddlewareOptions { * @static * A member holding default maxRetries value */ - private static DEFAULT_MAX_RETRIES = 3; + private static DEFAULT_MAX_RETRIES = 12; /** * @private * @static - * A member holding maximum delay value in seconds + * A member holding maximum delay value (1 hour) in seconds */ - private static MAX_DELAY = 180; + private static MAX_DELAY = 3_600; /** * @private * @static * A member holding maximum maxRetries value */ - private static MAX_MAX_RETRIES = 10; + private static MAX_MAX_RETRIES = 64; /** * @public diff --git a/test/common/middleware/RetryHandlerOptions.ts b/test/common/middleware/RetryHandlerOptions.ts index 7b62e9ad9..1bb75c3cc 100644 --- a/test/common/middleware/RetryHandlerOptions.ts +++ b/test/common/middleware/RetryHandlerOptions.ts @@ -21,7 +21,7 @@ describe("RetryHandlerOptions.ts", () => { it("Should throw error for both delay and maxRetries are higher than the limit", () => { try { // eslint-disable-next-line @typescript-eslint/no-unused-vars - const options = new RetryHandlerOptions(1000, 1000); + const options = new RetryHandlerOptions(100_000, 1000); throw new Error("Test Failed - Something wrong with the delay and maxRetries max limit validation"); } catch (error) { assert.equal(error.name, "MaxLimitExceeded"); @@ -31,7 +31,7 @@ describe("RetryHandlerOptions.ts", () => { it("Should throw error for delay is higher than the limit", () => { try { // eslint-disable-next-line @typescript-eslint/no-unused-vars - const options = new RetryHandlerOptions(1000, 2); + const options = new RetryHandlerOptions(100_000, 2); throw new Error("Test Failed - Test Failed - Something wrong with the delay max limit validation"); } catch (error) { assert.equal(error.name, "MaxLimitExceeded");