Skip to content

Commit

Permalink
increase default retry delay
Browse files Browse the repository at this point in the history
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](#978) for more discussion.
  • Loading branch information
altano committed Oct 30, 2023
1 parent 607bf20 commit fe6557b
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
8 changes: 4 additions & 4 deletions src/middleware/options/RetryHandlerOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions test/common/middleware/RetryHandlerOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
Expand Down

0 comments on commit fe6557b

Please sign in to comment.