Skip to content

Commit

Permalink
[sitecore-jss] Optimize initial Retry delay time (#1733)
Browse files Browse the repository at this point in the history
* update delay time to start from 1ms

* add tests for DefaultRetryStrategy class

* change test description

* update changelog

* update pretierignore

* remove only

* add more unit tests, make defaultStrategy class constructor an object

* add test for default case
  • Loading branch information
addy-pathania committed Feb 13, 2024
1 parent 2958e65 commit 030b028
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 9 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ This project does NOT adhere to [Semantic Versioning](https://semver.org/spec/v2

### 🎉 New Features & Improvements

* `[sitecore-jss]` Retry policy to handle transient network errors. Users can pass `retryStrategy` to configure custom retry config to the services. They can customize the error codes and the number of retries. It consist of two functions shouldRetry and getDelay. ([#1731](https://github.com/Sitecore/jss/pull/1731))
* `[sitecore-jss]` Retry policy to handle transient network errors. Users can pass `retryStrategy` to configure custom retry config to the services. They can customize the error codes and the number of retries. It consist of two functions shouldRetry and getDelay. To determine the back-off time, we employ an exponential strategy with a default factor of 2.([#1731](https://github.com/Sitecore/jss/pull/1731)) ([#1733](https://github.com/Sitecore/jss/pull/1733))

## 20.2.3

Expand Down
55 changes: 54 additions & 1 deletion packages/sitecore-jss/src/graphql-request-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import { expect, use, spy } from 'chai';
import sinon from 'sinon';
import spies from 'chai-spies';
import nock from 'nock';
import { GraphQLRequestClient } from './graphql-request-client';
import { GraphQLRequestClient, DefaultRetryStrategy } from './graphql-request-client';
import { ClientError } from 'graphql-request';
import debugApi from 'debug';
import debug from './debug';

Expand Down Expand Up @@ -391,4 +392,56 @@ describe('GraphQLRequestClient', () => {
}
});
});

describe('DefaultRetryStrategy', () => {
const mockClientError = new ClientError(
{
data: undefined,
errors: [{ message: 'GaphqlError' }],
extensions: undefined,
status: 429,
},
{
query: 'query',
}
);
it('should return true from shouldRetry and use default values from constructor', () => {
const retryStrategy = new DefaultRetryStrategy();

const shouldRetry = retryStrategy.shouldRetry(mockClientError, 1, 3);
expect(shouldRetry).to.equal(true);
});

it('should return false when attempt exceeds retries', () => {
const retryStrategy = new DefaultRetryStrategy({ statusCodes: [503] });
mockClientError.response.status = 503;

const shouldRetry = retryStrategy.shouldRetry(mockClientError, 2, 1);
expect(shouldRetry).to.equal(false);
});

it('should return false when retries is 0', () => {
const retryStrategy = new DefaultRetryStrategy({ statusCodes: [503] });
mockClientError.response.status = 503;

const shouldRetry = retryStrategy.shouldRetry(mockClientError, 1, 0);
expect(shouldRetry).to.equal(false);
});

it('should return delay using exponential backoff when Retry-After header is not present', () => {
const retryStrategy = new DefaultRetryStrategy();
const delay = retryStrategy.getDelay(mockClientError, 3);
const expectedDelay = Math.pow(retryStrategy['factor'], 3 - 1) * 1000;
expect(delay).to.equal(expectedDelay);
});

it('should use custom exponential factor', () => {
const customFactor = 3;
const retryStrategy = new DefaultRetryStrategy({ statusCodes: [429], factor: customFactor });

const delay = retryStrategy.getDelay(mockClientError, 3);
const expectedDelay = Math.pow(customFactor, 3 - 1) * 1000;
expect(delay).to.equal(expectedDelay);
});
});
});
15 changes: 8 additions & 7 deletions packages/sitecore-jss/src/graphql-request-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,13 @@ export class DefaultRetryStrategy implements RetryStrategy {
private factor: number;

/**
* @param {number[]} statusCodes HTTP status codes to trigger retries on
* @param {number} factor Factor by which the delay increases with each retry attempt
* @param {Object} options Configurable options for retry mechanism.
* @param {number[]} options.statusCodes HTTP status codes to trigger retries on
* @param {number} options.factor Factor by which the delay increases with each retry attempt
*/
constructor(statusCodes?: number[], factor?: number) {
this.statusCodes = statusCodes || [429];
this.factor = factor || 2;
constructor(options: { statusCodes?: number[]; factor?: number } = {}) {
this.statusCodes = options.statusCodes || [429];
this.factor = options.factor || 2;
}

shouldRetry(error: ClientError, attempt: number, retries: number): boolean {
Expand All @@ -98,7 +99,7 @@ export class DefaultRetryStrategy implements RetryStrategy {
const rawHeaders = error.response?.headers;
const delaySeconds = rawHeaders?.get('Retry-After')
? Number.parseInt(rawHeaders?.get('Retry-After'), 10)
: Math.pow(this.factor, attempt);
: Math.pow(this.factor, attempt - 1);

return delaySeconds * 1000;
}
Expand Down Expand Up @@ -136,7 +137,7 @@ export class GraphQLRequestClient implements GraphQLClient {
this.retries = clientConfig.retries || 0;
this.retryStrategy =
clientConfig.retryStrategy ||
new DefaultRetryStrategy([429, 502, 503, 504, 520, 521, 522, 523, 524]);
new DefaultRetryStrategy({ statusCodes: [429, 502, 503, 504, 520, 521, 522, 523, 524] });
this.client = new Client(endpoint, {
headers: this.headers,
fetch: clientConfig.fetch,
Expand Down

0 comments on commit 030b028

Please sign in to comment.