From 6c290dffc2d8e2c9b964608c0eef76f00aad0ec7 Mon Sep 17 00:00:00 2001 From: Addy Pathania Date: Wed, 7 Feb 2024 22:57:07 -0500 Subject: [PATCH 01/11] added retry policy for all error codes --- .../src/lib/dictionary-service-factory.ts | 60 ++++++++++++------- .../src/graphql-request-client.ts | 42 ++++++------- 2 files changed, 57 insertions(+), 45 deletions(-) diff --git a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts index 963f814194..6ca766c982 100644 --- a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts +++ b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts @@ -17,34 +17,50 @@ export class DictionaryServiceFactory { */ create(siteName: string): DictionaryService { return process.env.FETCH_WITH === constants.FETCH_WITH.GRAPHQL - ? new GraphQLDictionaryService({ - siteName, - clientFactory, - /* - The Dictionary Service needs a root item ID in order to fetch dictionary phrases for the current app. - When not provided, the service will attempt to figure out the root item for the current JSS App using GraphQL and app name. - For SXA site(s) and multisite setup there's no need to specify it - it will be autoresolved. - Otherwise, if your Sitecore instance only has 1 JSS App (i.e. in a Sitecore XP setup), you can specify the root item ID here. - rootItemId: '{GUID}' - */ - /* - GraphQL endpoint may reach its rate limit with the amount of Layout and Dictionary requests it receives and throw a rate limit error. - GraphQL Dictionary and Layout Services can handle rate limit errors from server and attempt a retry on requests. - For this, specify the number of retries the GraphQL client will attempt. - It will only try the request once by default. - retries: 'number' - */ - retries: - (process.env.GRAPH_QL_SERVICE_RETRIES && - parseInt(process.env.GRAPH_QL_SERVICE_RETRIES, 10)) || - 0, - }) + ? this.createWithRetryConfig(siteName) : new RestDictionaryService({ apiHost: config.sitecoreApiHost, apiKey: config.sitecoreApiKey, siteName, }); } + + /** + * @param {string} siteName site name + * @param {object} customRetryConfig custom retry configuration + * @returns {DictionaryService} service instance with custom retry configuration + */ + createWithRetryConfig( + siteName: string, + customRetryConfig?: Record + ): DictionaryService { + const defaultRetryConfig = { + retries: + (process.env.GRAPH_QL_SERVICE_RETRIES && + parseInt(process.env.GRAPH_QL_SERVICE_RETRIES, 10)) || + 3, + minimumTimeout: process.env.GRAPH_QL_SERVICE_RETRIES || 1000, + }; + + // Add or remove other default error codes as needed + const retryConfig = customRetryConfig || { + 429: defaultRetryConfig, + 502: defaultRetryConfig, + 503: defaultRetryConfig, + 504: defaultRetryConfig, + 520: defaultRetryConfig, + 521: defaultRetryConfig, + 522: defaultRetryConfig, + 523: defaultRetryConfig, + 524: defaultRetryConfig, + }; + + return new GraphQLDictionaryService({ + siteName, + clientFactory, + retryConfig, + }); + } } /** DictionaryServiceFactory singleton */ diff --git a/packages/sitecore-jss/src/graphql-request-client.ts b/packages/sitecore-jss/src/graphql-request-client.ts index 80da5232cf..8292828ddc 100644 --- a/packages/sitecore-jss/src/graphql-request-client.ts +++ b/packages/sitecore-jss/src/graphql-request-client.ts @@ -39,7 +39,7 @@ export type GraphQLRequestClientConfig = { /** * Number of retries for client. Will be used if endpoint responds with 429 (rate limit reached) error */ - retries?: number; + retryConfig?: Record; }; /** @@ -65,9 +65,9 @@ export class GraphQLRequestClient implements GraphQLClient { private client: Client; private headers: Record = {}; private debug: Debugger; - private retries: number; private abortTimeout?: TimeoutPromise; private timeout?: number; + private retryConfig: Record; /** * Provides ability to execute graphql query using given `endpoint` @@ -86,7 +86,7 @@ export class GraphQLRequestClient implements GraphQLClient { } this.timeout = clientConfig.timeout; - this.retries = clientConfig.retries || 0; + this.retryConfig = clientConfig.retryConfig || {}; this.client = new Client(endpoint, { headers: this.headers, fetch: clientConfig.fetch, @@ -117,45 +117,41 @@ export class GraphQLRequestClient implements GraphQLClient { query: string | DocumentNode, variables?: { [key: string]: unknown } ): Promise { - let retriesLeft = this.retries; + const statusCodes = Object.keys(this.retryConfig).map(Number); const retryer = async (): Promise => { - // Note we don't have access to raw request/response with graphql-request - // (or nice hooks like we have with Axios), but we should log whatever we have. - this.debug('request: %o', { - url: this.endpoint, - headers: this.headers, - query, - variables, - }); const startTimestamp = Date.now(); const fetchWithOptionalTimeout = [this.client.request(query, variables)]; if (this.timeout) { this.abortTimeout = new TimeoutPromise(this.timeout); fetchWithOptionalTimeout.push(this.abortTimeout.start); } + return Promise.race(fetchWithOptionalTimeout).then( (data: T) => { this.abortTimeout?.clear(); this.debug('response in %dms: %o', Date.now() - startTimestamp, data); return Promise.resolve(data); }, - (error: ClientError) => { + async (error: ClientError) => { this.abortTimeout?.clear(); - this.debug('response error: %o', error.response || error.message || error); - if (error.response?.status === 429 && retriesLeft > 0) { - const rawHeaders = (error as ClientError)?.response?.headers; + const status = error.response?.status; + if (statusCodes.includes(status) && this.retryConfig[status].retries > 0) { + const config = this.retryConfig[status]; + // factor by which the timeout increases with each retry. + const factor = 2; const delaySeconds = - rawHeaders && rawHeaders.get('Retry-After') - ? Number.parseInt(rawHeaders.get('Retry-After'), 10) - : 1; + error.response.headers.get('Retry-After') || + config.minimumTimeout * Math.pow(factor, config.retries); this.debug( - 'Error: Rate limit reached for GraphQL endpoint. Retrying in %ds. Retries left: %d', + 'Error: %d. Retrying in %ds. Retries left: %d', + status, delaySeconds, - retriesLeft + config.retries ); - retriesLeft--; - return new Promise((resolve) => setTimeout(resolve, delaySeconds * 1000)).then(retryer); + config.retries--; + await new Promise((resolve) => setTimeout(resolve, delaySeconds * 1000)); + return retryer(); } else { return Promise.reject(error); } From 7cd4275075596c19d233d06c75733c43cfcdf7bd Mon Sep 17 00:00:00 2001 From: Addy Pathania Date: Wed, 7 Feb 2024 23:03:26 -0500 Subject: [PATCH 02/11] remove seconds multiplier --- packages/sitecore-jss/src/graphql-request-client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sitecore-jss/src/graphql-request-client.ts b/packages/sitecore-jss/src/graphql-request-client.ts index 8292828ddc..732a2f6c47 100644 --- a/packages/sitecore-jss/src/graphql-request-client.ts +++ b/packages/sitecore-jss/src/graphql-request-client.ts @@ -150,7 +150,7 @@ export class GraphQLRequestClient implements GraphQLClient { config.retries ); config.retries--; - await new Promise((resolve) => setTimeout(resolve, delaySeconds * 1000)); + await new Promise((resolve) => setTimeout(resolve, delaySeconds)); return retryer(); } else { return Promise.reject(error); From 8b9bdcc491fd68e951d3b7fcd3ebda1ff7f67634 Mon Sep 17 00:00:00 2001 From: Addy Pathania Date: Fri, 9 Feb 2024 07:28:03 -0500 Subject: [PATCH 03/11] refactor solution, add unit tests --- .../src/lib/dictionary-service-factory.ts | 60 ++-- packages/sitecore-jss/package.json | 2 + .../src/graphql-request-client.test.ts | 283 ++++++++++++++---- .../src/graphql-request-client.ts | 83 ++++- yarn.lock | 63 +++- 5 files changed, 372 insertions(+), 119 deletions(-) diff --git a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts index 6ca766c982..963f814194 100644 --- a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts +++ b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts @@ -17,50 +17,34 @@ export class DictionaryServiceFactory { */ create(siteName: string): DictionaryService { return process.env.FETCH_WITH === constants.FETCH_WITH.GRAPHQL - ? this.createWithRetryConfig(siteName) + ? new GraphQLDictionaryService({ + siteName, + clientFactory, + /* + The Dictionary Service needs a root item ID in order to fetch dictionary phrases for the current app. + When not provided, the service will attempt to figure out the root item for the current JSS App using GraphQL and app name. + For SXA site(s) and multisite setup there's no need to specify it - it will be autoresolved. + Otherwise, if your Sitecore instance only has 1 JSS App (i.e. in a Sitecore XP setup), you can specify the root item ID here. + rootItemId: '{GUID}' + */ + /* + GraphQL endpoint may reach its rate limit with the amount of Layout and Dictionary requests it receives and throw a rate limit error. + GraphQL Dictionary and Layout Services can handle rate limit errors from server and attempt a retry on requests. + For this, specify the number of retries the GraphQL client will attempt. + It will only try the request once by default. + retries: 'number' + */ + retries: + (process.env.GRAPH_QL_SERVICE_RETRIES && + parseInt(process.env.GRAPH_QL_SERVICE_RETRIES, 10)) || + 0, + }) : new RestDictionaryService({ apiHost: config.sitecoreApiHost, apiKey: config.sitecoreApiKey, siteName, }); } - - /** - * @param {string} siteName site name - * @param {object} customRetryConfig custom retry configuration - * @returns {DictionaryService} service instance with custom retry configuration - */ - createWithRetryConfig( - siteName: string, - customRetryConfig?: Record - ): DictionaryService { - const defaultRetryConfig = { - retries: - (process.env.GRAPH_QL_SERVICE_RETRIES && - parseInt(process.env.GRAPH_QL_SERVICE_RETRIES, 10)) || - 3, - minimumTimeout: process.env.GRAPH_QL_SERVICE_RETRIES || 1000, - }; - - // Add or remove other default error codes as needed - const retryConfig = customRetryConfig || { - 429: defaultRetryConfig, - 502: defaultRetryConfig, - 503: defaultRetryConfig, - 504: defaultRetryConfig, - 520: defaultRetryConfig, - 521: defaultRetryConfig, - 522: defaultRetryConfig, - 523: defaultRetryConfig, - 524: defaultRetryConfig, - }; - - return new GraphQLDictionaryService({ - siteName, - clientFactory, - retryConfig, - }); - } } /** DictionaryServiceFactory singleton */ diff --git a/packages/sitecore-jss/package.json b/packages/sitecore-jss/package.json index b577904d71..e02540ea42 100644 --- a/packages/sitecore-jss/package.json +++ b/packages/sitecore-jss/package.json @@ -52,6 +52,7 @@ "typescript": "~4.3.5" }, "dependencies": { + "@types/sinon": "^17.0.3", "axios": "^0.21.1", "chalk": "^4.1.0", "debug": "^4.3.1", @@ -59,6 +60,7 @@ "graphql-request": "^4.2.0", "lodash.unescape": "^4.0.1", "memory-cache": "^0.2.0", + "sinon": "^17.0.1", "url-parse": "^1.5.9" }, "description": "", diff --git a/packages/sitecore-jss/src/graphql-request-client.test.ts b/packages/sitecore-jss/src/graphql-request-client.test.ts index ccb785a0ab..2448ad5ac2 100644 --- a/packages/sitecore-jss/src/graphql-request-client.test.ts +++ b/packages/sitecore-jss/src/graphql-request-client.test.ts @@ -1,6 +1,7 @@ /* eslint-disable no-unused-expressions */ /* eslint-disable dot-notation */ 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'; @@ -118,7 +119,6 @@ describe('GraphQLRequestClient', () => { ); } }); - it('should throw error when request is aborted with default timeout value', async () => { nock('http://jssnextweb') .post('/graphql') @@ -136,7 +136,7 @@ describe('GraphQLRequestClient', () => { }); it('should use retry and throw error when retries specified', async function() { - this.timeout(6000); + this.timeout(8000); nock('http://jssnextweb') .post('/graphql') .reply(429) @@ -144,7 +144,10 @@ describe('GraphQLRequestClient', () => { .reply(429) .post('/graphql') .reply(429); - const graphQLClient = new GraphQLRequestClient(endpoint, { retries: 2 }); + + const graphQLClient = new GraphQLRequestClient(endpoint, { + retries: 2, + }); spy.on(graphQLClient['client'], 'request'); await graphQLClient.request('test').catch((error) => { expect(error).to.not.be.undefined; @@ -153,67 +156,68 @@ describe('GraphQLRequestClient', () => { }); }); - it('should use retry and resolve if one of the requests resolves', async function() { - this.timeout(6000); - nock('http://jssnextweb') - .post('/graphql') - .reply(429) - .post('/graphql') - .reply(429) - .post('/graphql') - .reply(200, { - data: { - result: 'Hello world...', - }, - }); - const graphQLClient = new GraphQLRequestClient(endpoint, { retries: 3 }); - spy.on(graphQLClient['client'], 'request'); - - const data = await graphQLClient.request('test'); - - expect(data).to.not.be.null; - expect(graphQLClient['client'].request).to.be.called.exactly(3); - spy.restore(graphQLClient); - }); - - it('should use [retry-after] header value when response is 429', async function() { - this.timeout(6000); - nock('http://jssnextweb') - .post('/graphql') - .reply(429, {}, { 'Retry-After': '2' }); - const graphQLClient = new GraphQLRequestClient(endpoint, { retries: 1 }); - spy.on(graphQLClient, 'debug'); - - await graphQLClient.request('test').catch(() => { - expect(graphQLClient['debug']).to.have.been.called.with( - 'Error: Rate limit reached for GraphQL endpoint. Retrying in %ds. Retries left: %d', - 2, - 1 - ); - spy.restore(graphQLClient); - }); - }); - - it('should throw error when request is aborted with default timeout value after retry', async () => { - nock('http://jssnextweb') - .post('/graphql') - .reply(429) - .post('/graphql') - .delay(100) - .reply(200, { - data: { - result: 'Hello world...', - }, - }); - - const graphQLClient = new GraphQLRequestClient(endpoint, { retries: 2 }); - spy.on(graphQLClient['client'], 'request'); - await graphQLClient.request('test').catch((error) => { - expect(graphQLClient['client'].request).to.be.called.exactly(2); - expect(error.name).to.equal('AbortError'); - spy.restore(graphQLClient); - }); - }); + // it('should use retry and resolve if one of the requests resolves', async function() { + // this.timeout(8000); + // nock('http://jssnextweb') + // .post('/graphql') + // .reply(429) + // .post('/graphql') + // .reply(429) + // .post('/graphql') + // .reply(200, { + // data: { + // result: 'Hello world...', + // }, + // }); + // const graphQLClient = new GraphQLRequestClient(endpoint, { retries: 2 }); + // spy.on(graphQLClient['client'], 'request'); + + // const data = await graphQLClient.request('test'); + + // expect(data).to.not.be.null; + // expect(graphQLClient['client'].request).to.be.called.exactly(3); + // spy.restore(graphQLClient); + // }); + + // it.only('should use [retry-after] header value when response is 429', async function() { + // this.timeout(6000); + // nock('http://jssnextweb') + // .post('/graphql') + // .reply(429, {}, { 'Retry-After': '2' }); + // const graphQLClient = new GraphQLRequestClient(endpoint, { retries: 1 }); + // spy.on(graphQLClient, 'debug'); + + // await graphQLClient.request('test').catch(() => { + // expect(graphQLClient['debug']).to.have.been.called.with( + // 'Error: %d. Rate limit reached for GraphQL endpoint. Retrying in %ds. This was your %d attempt.', + // 429, + // 2, + // 1 + // ); + // spy.restore(graphQLClient); + // }); + // }); + + // it.only('should throw error when request is aborted with default timeout value after retry', async () => { + // nock('http://jssnextweb') + // .post('/graphql') + // .reply(429) + // .post('/graphql') + // .delay(100) + // .reply(200, { + // data: { + // result: 'Hello world...', + // }, + // }); + + // const graphQLClient = new GraphQLRequestClient(endpoint, { retries: 2 }); + // spy.on(graphQLClient['client'], 'request'); + // await graphQLClient.request('test').catch((error) => { + // expect(graphQLClient['client'].request).to.be.called.exactly(3); + // expect(error.name).to.equal('AbortError'); + // spy.restore(graphQLClient); + // }); + // }); it('should throw error upon request timeout using provided timeout value', async () => { nock('http://jssnextweb') @@ -245,4 +249,155 @@ describe('GraphQLRequestClient', () => { expect(client['timeout']).to.equal(300); }); }); + + describe('Retrayable status codes', () => { + const retryableStatusCodeThrowError = async (statusCode: number) => { + nock('http://jssnextweb') + .post('/graphql') + .reply(statusCode) + .post('/graphql') + .reply(statusCode) + .post('/graphql') + .reply(statusCode); + + const graphQLClient = new GraphQLRequestClient(endpoint, { + retries: 2, + }); + + spy.on(graphQLClient['client'], 'request'); + + try { + await graphQLClient.request('test'); + } catch (error) { + expect(error).to.not.be.undefined; + expect(graphQLClient['client'].request).to.have.been.called.exactly(3); + spy.restore(graphQLClient); + } + }; + + // Test cases for each retryable status code + for (const statusCode of [502, 503, 504, 520, 521, 522, 523, 524]) { + it(`should retry and throw error for ${statusCode} when retries specified`, async function() { + this.timeout(8000); + await retryableStatusCodeThrowError(statusCode); + }); + } + + const retryableStatusCodeResolve = async (statusCode: number) => { + nock('http://jssnextweb') + .post('/graphql') + .reply(statusCode) + .post('/graphql') + .reply(statusCode) + .post('/graphql') + .reply(200, { + data: { + result: 'Hello world...', + }, + }); + + const graphQLClient = new GraphQLRequestClient(endpoint, { + retries: 3, + }); + + spy.on(graphQLClient['client'], 'request'); + + const data = await graphQLClient.request('test'); + + try { + await graphQLClient.request('test'); + expect(data).to.not.be.null; + } catch (error) { + expect(graphQLClient['client'].request).to.have.been.called.exactly(4); + spy.restore(graphQLClient); + } + }; + + // Test cases for each retryable status code + for (const statusCode of [429]) { + it(`should retry and resolve for ${statusCode} if one of the request resolves`, async function() { + this.timeout(16000); + await retryableStatusCodeResolve(statusCode); + }); + } + + it('should use custom retryStrategy and retry accordingly', async function() { + this.timeout(8000); + + nock('http://jssnextweb') + .post('/graphql') + .reply(502, { + data: { + result: 'Hello world...', + }, + }); + + const customRetryStrategy = { + shouldRetry: (_: any, attempt: number) => attempt <= 3, + getDelay: () => 1000, + }; + + const graphQLClient = new GraphQLRequestClient(endpoint, { + retries: 3, + retryStrategy: customRetryStrategy, + }); + + // Spy on the client's request method + spy.on(graphQLClient['client'], 'request'); + + try { + await graphQLClient.request('test'); + } catch (error) { + expect(error).to.not.be.undefined; + expect(graphQLClient['client'].request).to.be.called.exactly(4); + spy.restore(graphQLClient); + } + }); + + it('should delay before retrying based on exponential backoff', async function() { + this.timeout(32000); + + nock('http://jssnextweb') + .post('/graphql') + .reply(429) + .post('/graphql') + .reply(429) + .post('/graphql') + .reply(429) + .post('/graphql') + .reply(429); + + const graphQLClient = new GraphQLRequestClient(endpoint, { + retries: 4, + }); + + spy.on(graphQLClient['client'], 'request'); + + try { + await graphQLClient.request('test'); + + expect(graphQLClient['client'].request).to.have.been.called.exactly(1); + + const clock = sinon.useFakeTimers(); + clock.tick(1000); + + await graphQLClient.request('test'); + expect(graphQLClient['client'].request).to.have.been.called.exactly(2); + + clock.tick(2000); + + await graphQLClient.request('test'); + expect(graphQLClient['client'].request).to.have.been.called.exactly(3); + + clock.tick(4000); + + await graphQLClient.request('test'); + expect(graphQLClient['client'].request).to.have.been.called.exactly(4); + + clock.restore(); + } catch (error) { + console.log('error'); + } + }); + }); }); diff --git a/packages/sitecore-jss/src/graphql-request-client.ts b/packages/sitecore-jss/src/graphql-request-client.ts index 732a2f6c47..56d171bf5b 100644 --- a/packages/sitecore-jss/src/graphql-request-client.ts +++ b/packages/sitecore-jss/src/graphql-request-client.ts @@ -15,6 +15,25 @@ export interface GraphQLClient { */ request(query: string | DocumentNode, variables?: { [key: string]: unknown }): Promise; } +/** + * Defines the strategy for retrying GraphQL requests based on errors and attempts. + */ +export interface RetryStrategy { + /** + * Determines whether a request should be retried based on the given error and attempt count. + * @param error - The error received from the GraphQL request. + * @param attempt - The current attempt number. + * @returns A boolean indicating whether to retry the request. + */ + shouldRetry(error: ClientError, attempt: number): boolean; + /** + * Calculates the delay (in milliseconds) before the next retry based on the given error and attempt count. + * @param error - The error received from the GraphQL request. + * @param attempt - The current attempt number. + * @returns The delay in milliseconds before the next retry. + */ + getDelay(error: ClientError, attempt: number): number; +} /** * Minimum configuration options for classes that implement @see GraphQLClient @@ -39,7 +58,11 @@ export type GraphQLRequestClientConfig = { /** * Number of retries for client. Will be used if endpoint responds with 429 (rate limit reached) error */ - retryConfig?: Record; + retries?: number; + /** + * Number of retries for client. Will be used if endpoint responds with 429 (rate limit reached) error + */ + retryStrategy?: RetryStrategy; }; /** @@ -67,7 +90,28 @@ export class GraphQLRequestClient implements GraphQLClient { private debug: Debugger; private abortTimeout?: TimeoutPromise; private timeout?: number; - private retryConfig: Record; + private retries: number; + private retryStrategy: RetryStrategy; + private defaultRetryStrategy: RetryStrategy = { + shouldRetry: (error: ClientError, attempt: number) => { + const retryableStatusCodes = [429, 502, 503, 504, 520, 521, 522, 523, 524]; + return ( + this.retries > 0 && + attempt <= this.retries && + retryableStatusCodes.includes(error.response.status) + ); + }, + getDelay: (error: ClientError, attempt: number) => { + console.log('attmept', attempt); + const factor = 2; + const rawHeaders = error.response.headers; + const delaySeconds = rawHeaders?.get('Retry-After') + ? Number.parseInt(rawHeaders?.get('Retry-After'), 10) + : Math.pow(factor, attempt); + + return delaySeconds * 1000; + }, + }; /** * Provides ability to execute graphql query using given `endpoint` @@ -86,7 +130,8 @@ export class GraphQLRequestClient implements GraphQLClient { } this.timeout = clientConfig.timeout; - this.retryConfig = clientConfig.retryConfig || {}; + this.retries = clientConfig.retries || 0; + this.retryStrategy = clientConfig.retryStrategy || this.defaultRetryStrategy; this.client = new Client(endpoint, { headers: this.headers, fetch: clientConfig.fetch, @@ -117,9 +162,17 @@ export class GraphQLRequestClient implements GraphQLClient { query: string | DocumentNode, variables?: { [key: string]: unknown } ): Promise { - const statusCodes = Object.keys(this.retryConfig).map(Number); + let attempt = 1; const retryer = async (): Promise => { + // Note we don't have access to raw request/response with graphql-request + // (or nice hooks like we have with Axios), but we should log whatever we have. + this.debug('request: %o', { + url: this.endpoint, + headers: this.headers, + query, + variables, + }); const startTimestamp = Date.now(); const fetchWithOptionalTimeout = [this.client.request(query, variables)]; if (this.timeout) { @@ -135,23 +188,21 @@ export class GraphQLRequestClient implements GraphQLClient { }, async (error: ClientError) => { this.abortTimeout?.clear(); + this.debug('response error: %o', error.response || error.message || error); const status = error.response?.status; - if (statusCodes.includes(status) && this.retryConfig[status].retries > 0) { - const config = this.retryConfig[status]; - // factor by which the timeout increases with each retry. - const factor = 2; - const delaySeconds = - error.response.headers.get('Retry-After') || - config.minimumTimeout * Math.pow(factor, config.retries); + const shouldRetry = this.retryStrategy.shouldRetry(error, attempt); + + if (shouldRetry) { + const delaySeconds = this.retryStrategy.getDelay(error, attempt); this.debug( - 'Error: %d. Retrying in %ds. Retries left: %d', + 'Error: %d. Rate limit reached for GraphQL endpoint. Retrying in %ds. This was your %d attempt.', status, delaySeconds, - config.retries + attempt ); - config.retries--; - await new Promise((resolve) => setTimeout(resolve, delaySeconds)); - return retryer(); + + attempt++; + return new Promise((resolve) => setTimeout(resolve, delaySeconds)).then(retryer); } else { return Promise.reject(error); } diff --git a/yarn.lock b/yarn.lock index 9b7dedf388..1bf7975080 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5256,6 +5256,15 @@ __metadata: languageName: node linkType: hard +"@sinonjs/fake-timers@npm:^11.2.2": + version: 11.2.2 + resolution: "@sinonjs/fake-timers@npm:11.2.2" + dependencies: + "@sinonjs/commons": ^3.0.0 + checksum: 68c29b0e1856fdc280df03ddbf57c726420b78e9f943a241b471edc018fb14ff36fdc1daafd6026cba08c3c7f50c976fb7ae11b88ff44cd7f609692ca7d25158 + languageName: node + linkType: hard + "@sinonjs/fake-timers@npm:^7.1.0": version: 7.1.2 resolution: "@sinonjs/fake-timers@npm:7.1.2" @@ -5276,7 +5285,7 @@ __metadata: languageName: node linkType: hard -"@sinonjs/text-encoding@npm:^0.7.1": +"@sinonjs/text-encoding@npm:^0.7.1, @sinonjs/text-encoding@npm:^0.7.2": version: 0.7.2 resolution: "@sinonjs/text-encoding@npm:0.7.2" checksum: fe690002a32ba06906cf87e2e8fe84d1590294586f2a7fd180a65355b53660c155c3273d8011a5f2b77209b819aa7306678ae6e4aea0df014bd7ffd4bbbcf1ab @@ -5756,6 +5765,7 @@ __metadata: "@types/memory-cache": ^0.2.1 "@types/mocha": ^9.0.0 "@types/node": ^16.11.6 + "@types/sinon": ^17.0.3 "@types/url-parse": 1.4.8 axios: ^0.21.1 chai: ^4.2.0 @@ -5772,6 +5782,7 @@ __metadata: mocha: ^10.2.0 nock: ^13.0.5 nyc: ^15.1.0 + sinon: ^17.0.1 ts-node: ^8.4.1 tslib: ^1.10.0 typescript: ~4.3.5 @@ -6729,6 +6740,15 @@ __metadata: languageName: node linkType: hard +"@types/sinon@npm:^17.0.3": + version: 17.0.3 + resolution: "@types/sinon@npm:17.0.3" + dependencies: + "@types/sinonjs__fake-timers": "*" + checksum: c8e9956d9c90fe1ec1cc43085ae48897f93f9ea86e909ab47f255ea71f5229651faa070393950fb6923aef426c84e92b375503f9f8886ef44668b82a8ee49e9a + languageName: node + linkType: hard + "@types/sinonjs__fake-timers@npm:*": version: 8.1.5 resolution: "@types/sinonjs__fake-timers@npm:8.1.5" @@ -17856,6 +17876,13 @@ __metadata: languageName: node linkType: hard +"just-extend@npm:^6.2.0": + version: 6.2.0 + resolution: "just-extend@npm:6.2.0" + checksum: 022024d6f687c807963b97a24728a378799f7e4af7357d1c1f90dedb402943d5c12be99a5136654bed8362c37a358b1793feaad3366896f239a44e17c5032d86 + languageName: node + linkType: hard + "karma-chrome-launcher@npm:^3.1.0": version: 3.2.0 resolution: "karma-chrome-launcher@npm:3.2.0" @@ -19888,6 +19915,19 @@ __metadata: languageName: node linkType: hard +"nise@npm:^5.1.5": + version: 5.1.9 + resolution: "nise@npm:5.1.9" + dependencies: + "@sinonjs/commons": ^3.0.0 + "@sinonjs/fake-timers": ^11.2.2 + "@sinonjs/text-encoding": ^0.7.2 + just-extend: ^6.2.0 + path-to-regexp: ^6.2.1 + checksum: ab9fd6eabc98170f18aef6c9567983145c1dc62c7aef46eda0fea754083316c1f0f9b2c32e9b4bfdd25122276d670293596ed672b54dd1ffa8eb58b56a30ea95 + languageName: node + linkType: hard + "nocache@npm:^2.1.0": version: 2.1.0 resolution: "nocache@npm:2.1.0" @@ -21338,6 +21378,13 @@ __metadata: languageName: node linkType: hard +"path-to-regexp@npm:^6.2.1": + version: 6.2.1 + resolution: "path-to-regexp@npm:6.2.1" + checksum: f0227af8284ea13300f4293ba111e3635142f976d4197f14d5ad1f124aebd9118783dd2e5f1fe16f7273743cc3dbeddfb7493f237bb27c10fdae07020cc9b698 + languageName: node + linkType: hard + "path-type@npm:^3.0.0": version: 3.0.0 resolution: "path-type@npm:3.0.0" @@ -23654,6 +23701,20 @@ __metadata: languageName: node linkType: hard +"sinon@npm:^17.0.1": + version: 17.0.1 + resolution: "sinon@npm:17.0.1" + dependencies: + "@sinonjs/commons": ^3.0.0 + "@sinonjs/fake-timers": ^11.2.2 + "@sinonjs/samsam": ^8.0.0 + diff: ^5.1.0 + nise: ^5.1.5 + supports-color: ^7.2.0 + checksum: a807c2997d6eabdcaa4409df9fd9816a3e839f96d7e5d76610a33f5e1b60cf37616c6288f0f580262da17ea4ee626c6d1600325bf423e30c5a7f0d9a203e26c0 + languageName: node + linkType: hard + "sisteransi@npm:^1.0.5": version: 1.0.5 resolution: "sisteransi@npm:1.0.5" From 4e05e3745f747811ba1803732b9de9f7e5b7b80d Mon Sep 17 00:00:00 2001 From: Addy Pathania Date: Fri, 9 Feb 2024 07:48:10 -0500 Subject: [PATCH 04/11] modify doc comment for services --- .../src/templates/nextjs/src/lib/dictionary-service-factory.ts | 3 ++- .../src/templates/nextjs/src/lib/layout-service-factory.ts | 1 + packages/sitecore-jss/src/graphql-request-client.ts | 1 - 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts index 963f814194..65b1899155 100644 --- a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts +++ b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts @@ -32,7 +32,8 @@ export class DictionaryServiceFactory { GraphQL Dictionary and Layout Services can handle rate limit errors from server and attempt a retry on requests. For this, specify the number of retries the GraphQL client will attempt. It will only try the request once by default. - retries: 'number' + retries: 'number' + Additionally, you can customize the retry strategy by providing your own RetryStrategy object. */ retries: (process.env.GRAPH_QL_SERVICE_RETRIES && diff --git a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/layout-service-factory.ts b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/layout-service-factory.ts index c175488bd7..34025ce00d 100644 --- a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/layout-service-factory.ts +++ b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/layout-service-factory.ts @@ -26,6 +26,7 @@ export class LayoutServiceFactory { For this, specify the number of retries the GraphQL client will attempt. It will only try the request once by default. retries: 'number' + Additionally, you can customize the retry strategy by providing your own RetryStrategy object. */ retries: (process.env.GRAPH_QL_SERVICE_RETRIES && diff --git a/packages/sitecore-jss/src/graphql-request-client.ts b/packages/sitecore-jss/src/graphql-request-client.ts index 56d171bf5b..fb6803b401 100644 --- a/packages/sitecore-jss/src/graphql-request-client.ts +++ b/packages/sitecore-jss/src/graphql-request-client.ts @@ -102,7 +102,6 @@ export class GraphQLRequestClient implements GraphQLClient { ); }, getDelay: (error: ClientError, attempt: number) => { - console.log('attmept', attempt); const factor = 2; const rawHeaders = error.response.headers; const delaySeconds = rawHeaders?.get('Retry-After') From 0e6b900561919b88b34ea82df2696d957ae631c8 Mon Sep 17 00:00:00 2001 From: Addy Pathania Date: Fri, 9 Feb 2024 08:18:15 -0500 Subject: [PATCH 05/11] update changelog --- CHANGELOG.md | 6 +++ .../src/graphql-request-client.test.ts | 51 ++++++++++--------- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 27244d6513..c1f8424d7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -611,6 +611,12 @@ Our versioning strategy is as follows: * `import { editingDataService } from '@sitecore-jss/sitecore-jss-nextjs/editing';` * `import { EditingRenderMiddleware } from '@sitecore-jss/sitecore-jss-nextjs/editing';` +## 20.3.0 + +### 🎉 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)) + ## 20.2.3 ### 🐛 Bug Fixes diff --git a/packages/sitecore-jss/src/graphql-request-client.test.ts b/packages/sitecore-jss/src/graphql-request-client.test.ts index 2448ad5ac2..5cc7a37d17 100644 --- a/packages/sitecore-jss/src/graphql-request-client.test.ts +++ b/packages/sitecore-jss/src/graphql-request-client.test.ts @@ -156,28 +156,28 @@ describe('GraphQLRequestClient', () => { }); }); - // it('should use retry and resolve if one of the requests resolves', async function() { - // this.timeout(8000); - // nock('http://jssnextweb') - // .post('/graphql') - // .reply(429) - // .post('/graphql') - // .reply(429) - // .post('/graphql') - // .reply(200, { - // data: { - // result: 'Hello world...', - // }, - // }); - // const graphQLClient = new GraphQLRequestClient(endpoint, { retries: 2 }); - // spy.on(graphQLClient['client'], 'request'); + it('should use retry and resolve if one of the requests resolves', async function() { + this.timeout(8000); + nock('http://jssnextweb') + .post('/graphql') + .reply(429) + .post('/graphql') + .reply(429) + .post('/graphql') + .reply(200, { + data: { + result: 'Hello world...', + }, + }); + const graphQLClient = new GraphQLRequestClient(endpoint, { retries: 2 }); + spy.on(graphQLClient['client'], 'request'); - // const data = await graphQLClient.request('test'); + const data = await graphQLClient.request('test'); - // expect(data).to.not.be.null; - // expect(graphQLClient['client'].request).to.be.called.exactly(3); - // spy.restore(graphQLClient); - // }); + expect(data).to.not.be.null; + expect(graphQLClient['client'].request).to.be.called.exactly(3); + spy.restore(graphQLClient); + }); // it.only('should use [retry-after] header value when response is 429', async function() { // this.timeout(6000); @@ -199,6 +199,7 @@ describe('GraphQLRequestClient', () => { // }); // it.only('should throw error when request is aborted with default timeout value after retry', async () => { + // this.timeout(5000); // nock('http://jssnextweb') // .post('/graphql') // .reply(429) @@ -212,11 +213,16 @@ describe('GraphQLRequestClient', () => { // const graphQLClient = new GraphQLRequestClient(endpoint, { retries: 2 }); // spy.on(graphQLClient['client'], 'request'); - // await graphQLClient.request('test').catch((error) => { + // try { + // await graphQLClient.request('test'); + // // If the request does not throw an error, fail the test + // expect.fail('Expected request to throw an error'); + // } catch (error) { // expect(graphQLClient['client'].request).to.be.called.exactly(3); // expect(error.name).to.equal('AbortError'); + // } finally { // spy.restore(graphQLClient); - // }); + // } // }); it('should throw error upon request timeout using provided timeout value', async () => { @@ -342,7 +348,6 @@ describe('GraphQLRequestClient', () => { retryStrategy: customRetryStrategy, }); - // Spy on the client's request method spy.on(graphQLClient['client'], 'request'); try { From 67cf404a5e496b85fe0a0ecae819f00c8baafcbb Mon Sep 17 00:00:00 2001 From: Addy Pathania Date: Fri, 9 Feb 2024 11:24:39 -0500 Subject: [PATCH 06/11] address review comments --- .../sitecore-jss-angular/src/public_api.ts | 2 + .../sitecore-jss-nextjs/src/graphql/index.ts | 2 + packages/sitecore-jss-react/src/index.ts | 1 + packages/sitecore-jss-vue/src/index.ts | 1 + packages/sitecore-jss/package.json | 4 +- .../src/graphql-request-client.test.ts | 8 +-- .../src/graphql-request-client.ts | 58 +++++++++++-------- packages/sitecore-jss/src/graphql/index.ts | 2 + .../src/i18n/graphql-dictionary-service.ts | 2 +- packages/sitecore-jss/src/index.ts | 2 + .../src/layout/graphql-layout-service.ts | 3 +- 11 files changed, 52 insertions(+), 33 deletions(-) diff --git a/packages/sitecore-jss-angular/src/public_api.ts b/packages/sitecore-jss-angular/src/public_api.ts index 251d0f7704..6fad33809c 100644 --- a/packages/sitecore-jss-angular/src/public_api.ts +++ b/packages/sitecore-jss-angular/src/public_api.ts @@ -1,3 +1,4 @@ +import { RetryStrategy } from './../../sitecore-jss/src/graphql-request-client'; export { FileDirective } from './components/file.directive'; export { ImageDirective } from './components/image.directive'; export { LinkDirective } from './components/link.directive'; @@ -56,6 +57,7 @@ export { ComponentFields, ComponentParams, } from '@sitecore-jss/sitecore-jss/layout'; +export { RetryStrategy, DefaultRetryStrategy } from '@sitecore-jss/sitecore-jss/graphql'; export { constants, HttpDataFetcher, HttpResponse, enableDebug } from '@sitecore-jss/sitecore-jss'; export { isServer, diff --git a/packages/sitecore-jss-nextjs/src/graphql/index.ts b/packages/sitecore-jss-nextjs/src/graphql/index.ts index 157e4bed60..fa83a58dd7 100644 --- a/packages/sitecore-jss-nextjs/src/graphql/index.ts +++ b/packages/sitecore-jss-nextjs/src/graphql/index.ts @@ -1,4 +1,6 @@ export { + RetryStrategy, + DefaultRetryStrategy, GraphQLRequestClient, GraphQLRequestClientFactory, GraphQLRequestClientFactoryConfig, diff --git a/packages/sitecore-jss-react/src/index.ts b/packages/sitecore-jss-react/src/index.ts index f48049b761..309a2532af 100644 --- a/packages/sitecore-jss-react/src/index.ts +++ b/packages/sitecore-jss-react/src/index.ts @@ -44,6 +44,7 @@ export { GraphQLDictionaryService, RestDictionaryService, } from '@sitecore-jss/sitecore-jss/i18n'; +export { RetryStrategy, DefaultRetryStrategy } from '@sitecore-jss/sitecore-jss/graphql'; export { mediaApi } from '@sitecore-jss/sitecore-jss/media'; export { getFEAASLibraryStylesheetLinks } from '@sitecore-jss/sitecore-jss/feaas'; export { ComponentFactory } from './components/sharedTypes'; diff --git a/packages/sitecore-jss-vue/src/index.ts b/packages/sitecore-jss-vue/src/index.ts index d9d9cc3b5b..b5915b0a9a 100644 --- a/packages/sitecore-jss-vue/src/index.ts +++ b/packages/sitecore-jss-vue/src/index.ts @@ -39,6 +39,7 @@ export { GraphQLDictionaryService, RestDictionaryService, } from '@sitecore-jss/sitecore-jss/i18n'; +export { RetryStrategy, DefaultRetryStrategy } from '@sitecore-jss/sitecore-jss/graphql'; export { mediaApi } from '@sitecore-jss/sitecore-jss/media'; export { EditFrame } from './components/EditFrame'; export { Placeholder } from './components/Placeholder'; diff --git a/packages/sitecore-jss/package.json b/packages/sitecore-jss/package.json index e02540ea42..4f7ff194e0 100644 --- a/packages/sitecore-jss/package.json +++ b/packages/sitecore-jss/package.json @@ -38,6 +38,7 @@ "@types/memory-cache": "^0.2.1", "@types/mocha": "^9.0.0", "@types/node": "^16.11.6", + "@types/sinon": "^17.0.3", "@types/url-parse": "1.4.8", "chai": "^4.2.0", "chai-spies": "^1.0.0", @@ -47,12 +48,12 @@ "mocha": "^10.2.0", "nock": "^13.0.5", "nyc": "^15.1.0", + "sinon": "^17.0.1", "ts-node": "^8.4.1", "tslib": "^1.10.0", "typescript": "~4.3.5" }, "dependencies": { - "@types/sinon": "^17.0.3", "axios": "^0.21.1", "chalk": "^4.1.0", "debug": "^4.3.1", @@ -60,7 +61,6 @@ "graphql-request": "^4.2.0", "lodash.unescape": "^4.0.1", "memory-cache": "^0.2.0", - "sinon": "^17.0.1", "url-parse": "^1.5.9" }, "description": "", diff --git a/packages/sitecore-jss/src/graphql-request-client.test.ts b/packages/sitecore-jss/src/graphql-request-client.test.ts index 5cc7a37d17..28db3bc370 100644 --- a/packages/sitecore-jss/src/graphql-request-client.test.ts +++ b/packages/sitecore-jss/src/graphql-request-client.test.ts @@ -282,7 +282,7 @@ describe('GraphQLRequestClient', () => { }; // Test cases for each retryable status code - for (const statusCode of [502, 503, 504, 520, 521, 522, 523, 524]) { + for (const statusCode of [429, 502, 503, 504, 520, 521, 522, 523, 524]) { it(`should retry and throw error for ${statusCode} when retries specified`, async function() { this.timeout(8000); await retryableStatusCodeThrowError(statusCode); @@ -320,14 +320,14 @@ describe('GraphQLRequestClient', () => { }; // Test cases for each retryable status code - for (const statusCode of [429]) { + for (const statusCode of [429, 502, 503, 504, 520, 521, 522, 523, 524]) { it(`should retry and resolve for ${statusCode} if one of the request resolves`, async function() { this.timeout(16000); await retryableStatusCodeResolve(statusCode); }); } - it('should use custom retryStrategy and retry accordingly', async function() { + it.only('should retry based on custom retryStrategy', async function() { this.timeout(8000); nock('http://jssnextweb') @@ -344,7 +344,7 @@ describe('GraphQLRequestClient', () => { }; const graphQLClient = new GraphQLRequestClient(endpoint, { - retries: 3, + retries: 4, retryStrategy: customRetryStrategy, }); diff --git a/packages/sitecore-jss/src/graphql-request-client.ts b/packages/sitecore-jss/src/graphql-request-client.ts index fb6803b401..01ae11bab3 100644 --- a/packages/sitecore-jss/src/graphql-request-client.ts +++ b/packages/sitecore-jss/src/graphql-request-client.ts @@ -23,9 +23,10 @@ export interface RetryStrategy { * Determines whether a request should be retried based on the given error and attempt count. * @param error - The error received from the GraphQL request. * @param attempt - The current attempt number. + * @param retries - The number of retries configured. * @returns A boolean indicating whether to retry the request. */ - shouldRetry(error: ClientError, attempt: number): boolean; + shouldRetry(error: ClientError, attempt: number, retries: number): boolean; /** * Calculates the delay (in milliseconds) before the next retry based on the given error and attempt count. * @param error - The error received from the GraphQL request. @@ -56,11 +57,12 @@ export type GraphQLRequestClientConfig = { */ timeout?: number; /** - * Number of retries for client. Will be used if endpoint responds with 429 (rate limit reached) error + * Number of retries for client. Number of retries for client. Will use the specified `retryStrategy`. */ retries?: number; /** - * Number of retries for client. Will be used if endpoint responds with 429 (rate limit reached) error + * Retry strategy for the client. Default will use an exponential back-off strategy for + * codes 429, 502, 503, 504, 520, 521, 522, 523, 524. */ retryStrategy?: RetryStrategy; }; @@ -80,6 +82,29 @@ export type GraphQLRequestClientFactory = ( */ export type GraphQLRequestClientFactoryConfig = { endpoint: string; apiKey?: string }; +export class DefaultRetryStrategy implements RetryStrategy { + private statusCodes: number[]; + private factor: number; + + constructor(statusCodes?: number[], factor?: number) { + this.statusCodes = statusCodes || [429]; + this.factor = factor || 2; + } + + shouldRetry(error: ClientError, attempt: number, retries: number): boolean { + return retries > 0 && attempt <= retries && this.statusCodes.includes(error.response.status); + } + + getDelay(error: ClientError, attempt: number): number { + const rawHeaders = error.response.headers; + const delaySeconds = rawHeaders?.get('Retry-After') + ? Number.parseInt(rawHeaders?.get('Retry-After'), 10) + : Math.pow(this.factor, attempt); + + return delaySeconds * 1000; + } +} + /** * A GraphQL client for Sitecore APIs that uses the 'graphql-request' library. * https://github.com/prisma-labs/graphql-request @@ -92,25 +117,6 @@ export class GraphQLRequestClient implements GraphQLClient { private timeout?: number; private retries: number; private retryStrategy: RetryStrategy; - private defaultRetryStrategy: RetryStrategy = { - shouldRetry: (error: ClientError, attempt: number) => { - const retryableStatusCodes = [429, 502, 503, 504, 520, 521, 522, 523, 524]; - return ( - this.retries > 0 && - attempt <= this.retries && - retryableStatusCodes.includes(error.response.status) - ); - }, - getDelay: (error: ClientError, attempt: number) => { - const factor = 2; - const rawHeaders = error.response.headers; - const delaySeconds = rawHeaders?.get('Retry-After') - ? Number.parseInt(rawHeaders?.get('Retry-After'), 10) - : Math.pow(factor, attempt); - - return delaySeconds * 1000; - }, - }; /** * Provides ability to execute graphql query using given `endpoint` @@ -130,7 +136,9 @@ export class GraphQLRequestClient implements GraphQLClient { this.timeout = clientConfig.timeout; this.retries = clientConfig.retries || 0; - this.retryStrategy = clientConfig.retryStrategy || this.defaultRetryStrategy; + this.retryStrategy = + clientConfig.retryStrategy || + new DefaultRetryStrategy([429, 502, 503, 504, 520, 521, 522, 523, 524]); this.client = new Client(endpoint, { headers: this.headers, fetch: clientConfig.fetch, @@ -189,12 +197,12 @@ export class GraphQLRequestClient implements GraphQLClient { this.abortTimeout?.clear(); this.debug('response error: %o', error.response || error.message || error); const status = error.response?.status; - const shouldRetry = this.retryStrategy.shouldRetry(error, attempt); + const shouldRetry = this.retryStrategy.shouldRetry(error, attempt, this.retries); if (shouldRetry) { const delaySeconds = this.retryStrategy.getDelay(error, attempt); this.debug( - 'Error: %d. Rate limit reached for GraphQL endpoint. Retrying in %ds. This was your %d attempt.', + 'Error: %d. Rate limit reached for GraphQL endpoint. Retrying in %ds (attempt %d).', status, delaySeconds, attempt diff --git a/packages/sitecore-jss/src/graphql/index.ts b/packages/sitecore-jss/src/graphql/index.ts index 9476d2f47d..1a69f55b7f 100644 --- a/packages/sitecore-jss/src/graphql/index.ts +++ b/packages/sitecore-jss/src/graphql/index.ts @@ -1,5 +1,7 @@ export { getAppRootId, AppRootQueryResult } from './app-root-query'; export { + RetryStrategy, + DefaultRetryStrategy, GraphQLClient, GraphQLRequestClient, GraphQLRequestClientConfig, diff --git a/packages/sitecore-jss/src/i18n/graphql-dictionary-service.ts b/packages/sitecore-jss/src/i18n/graphql-dictionary-service.ts index 88a865d26c..dd924d82df 100644 --- a/packages/sitecore-jss/src/i18n/graphql-dictionary-service.ts +++ b/packages/sitecore-jss/src/i18n/graphql-dictionary-service.ts @@ -56,7 +56,7 @@ const query = /* GraphQL */ ` export interface GraphQLDictionaryServiceConfig extends SearchServiceConfig, CacheOptions, - Pick { + Pick { /** * The URL of the graphQL endpoint. * @deprecated use @param clientFactory property instead diff --git a/packages/sitecore-jss/src/index.ts b/packages/sitecore-jss/src/index.ts index bf1a5a8791..6debc3aa2a 100644 --- a/packages/sitecore-jss/src/index.ts +++ b/packages/sitecore-jss/src/index.ts @@ -5,6 +5,8 @@ import * as constants from './constants'; export { default as debug, Debugger, enableDebug } from './debug'; export { HttpDataFetcher, HttpResponse, fetchData } from './data-fetcher'; export { + RetryStrategy, + DefaultRetryStrategy, GraphQLClient, GraphQLRequestClient, GraphQLRequestClientConfig, diff --git a/packages/sitecore-jss/src/layout/graphql-layout-service.ts b/packages/sitecore-jss/src/layout/graphql-layout-service.ts index 7ad50aa41a..2fade8bfae 100644 --- a/packages/sitecore-jss/src/layout/graphql-layout-service.ts +++ b/packages/sitecore-jss/src/layout/graphql-layout-service.ts @@ -8,7 +8,8 @@ import { } from '../graphql-request-client'; import debug from '../debug'; -export interface GraphQLLayoutServiceConfig extends Pick { +export interface GraphQLLayoutServiceConfig + extends Pick { /** * Your Graphql endpoint * @deprecated use @param clientFactory property instead From 05b057957003673ddb0927e28d884b01ebcfd416 Mon Sep 17 00:00:00 2001 From: Addy Pathania Date: Fri, 9 Feb 2024 13:19:49 -0500 Subject: [PATCH 07/11] fix tests, add comments --- .../src/lib/dictionary-service-factory.ts | 11 ++- .../nextjs/src/lib/layout-service-factory.ts | 15 ++- .../sitecore-jss-angular/src/public_api.ts | 1 - .../src/graphql-request-client.test.ts | 92 +++++++++---------- .../src/graphql-request-client.ts | 10 +- 5 files changed, 73 insertions(+), 56 deletions(-) diff --git a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts index 65b1899155..53f8213726 100644 --- a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts +++ b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts @@ -33,7 +33,16 @@ export class DictionaryServiceFactory { For this, specify the number of retries the GraphQL client will attempt. It will only try the request once by default. retries: 'number' - Additionally, you can customize the retry strategy by providing your own RetryStrategy object. + Additionally, you have the flexibility to customize the retry strategy by passing your customized RetryStrategy + object using the DefaultRetryStrategy class. The DefaultRetryStrategy class, which can be imported from the @sitecore-jss-nextjs + package, provides two essential methods: 'shouldRetry' which returns a boolean indicating whether a retry should + be attempted, and 'getDelay' which calculates the delay (in milliseconds) before the subsequent retry based on + the encountered error and the current attempt. + Example: + retryStrategy: new DefaultRetryStrategy({ + statusCodes: 'number[]', + factor: 'number' (The exponential factor to calculate backoff-time), + }), */ retries: (process.env.GRAPH_QL_SERVICE_RETRIES && diff --git a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/layout-service-factory.ts b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/layout-service-factory.ts index 34025ce00d..0a82d39b7b 100644 --- a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/layout-service-factory.ts +++ b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/layout-service-factory.ts @@ -23,10 +23,19 @@ export class LayoutServiceFactory { /* GraphQL endpoint may reach its rate limit with the amount of Layout and Dictionary requests it receives and throw a rate limit error. GraphQL Dictionary and Layout Services can handle rate limit errors from server and attempt a retry on requests. - For this, specify the number of retries the GraphQL client will attempt. + For this, specify the number of retries the GraphQL client will attempt. It will only try the request once by default. - retries: 'number' - Additionally, you can customize the retry strategy by providing your own RetryStrategy object. + retries: 'number' + Additionally, you have the flexibility to customize the retry strategy by passing your customized RetryStrategy + object using the DefaultRetryStrategy class. The DefaultRetryStrategy class, which can be imported from the @sitecore-jss-nextjs + package, provides two essential methods: 'shouldRetry' which returns a boolean indicating whether a retry should + be attempted, and 'getDelay' which calculates the delay (in milliseconds) before the subsequent retry based on + the encountered error and the current attempt. + Example: + retryStrategy: new DefaultRetryStrategy({ + statusCodes: 'number[]', + factor: 'number' (The exponential factor to calculate backoff-time), + }), */ retries: (process.env.GRAPH_QL_SERVICE_RETRIES && diff --git a/packages/sitecore-jss-angular/src/public_api.ts b/packages/sitecore-jss-angular/src/public_api.ts index 6fad33809c..6ab405679a 100644 --- a/packages/sitecore-jss-angular/src/public_api.ts +++ b/packages/sitecore-jss-angular/src/public_api.ts @@ -1,4 +1,3 @@ -import { RetryStrategy } from './../../sitecore-jss/src/graphql-request-client'; export { FileDirective } from './components/file.directive'; export { ImageDirective } from './components/image.directive'; export { LinkDirective } from './components/link.directive'; diff --git a/packages/sitecore-jss/src/graphql-request-client.test.ts b/packages/sitecore-jss/src/graphql-request-client.test.ts index 28db3bc370..72ba8ed9f9 100644 --- a/packages/sitecore-jss/src/graphql-request-client.test.ts +++ b/packages/sitecore-jss/src/graphql-request-client.test.ts @@ -179,51 +179,51 @@ describe('GraphQLRequestClient', () => { spy.restore(graphQLClient); }); - // it.only('should use [retry-after] header value when response is 429', async function() { - // this.timeout(6000); - // nock('http://jssnextweb') - // .post('/graphql') - // .reply(429, {}, { 'Retry-After': '2' }); - // const graphQLClient = new GraphQLRequestClient(endpoint, { retries: 1 }); - // spy.on(graphQLClient, 'debug'); - - // await graphQLClient.request('test').catch(() => { - // expect(graphQLClient['debug']).to.have.been.called.with( - // 'Error: %d. Rate limit reached for GraphQL endpoint. Retrying in %ds. This was your %d attempt.', - // 429, - // 2, - // 1 - // ); - // spy.restore(graphQLClient); - // }); - // }); - - // it.only('should throw error when request is aborted with default timeout value after retry', async () => { - // this.timeout(5000); - // nock('http://jssnextweb') - // .post('/graphql') - // .reply(429) - // .post('/graphql') - // .delay(100) - // .reply(200, { - // data: { - // result: 'Hello world...', - // }, - // }); - - // const graphQLClient = new GraphQLRequestClient(endpoint, { retries: 2 }); - // spy.on(graphQLClient['client'], 'request'); - // try { - // await graphQLClient.request('test'); - // // If the request does not throw an error, fail the test - // expect.fail('Expected request to throw an error'); - // } catch (error) { - // expect(graphQLClient['client'].request).to.be.called.exactly(3); - // expect(error.name).to.equal('AbortError'); - // } finally { - // spy.restore(graphQLClient); - // } - // }); + it('should use [retry-after] header value when response is 429', async function() { + this.timeout(7000); + nock('http://jssnextweb') + .post('/graphql') + .reply(429, {}, { 'Retry-After': '2' }); + const graphQLClient = new GraphQLRequestClient(endpoint, { retries: 1 }); + spy.on(graphQLClient, 'debug'); + + await graphQLClient.request('test').catch(() => { + expect(graphQLClient['debug']).to.have.been.called.with( + 'Error: %d. Rate limit reached for GraphQL endpoint. Retrying in %dms (attempt %d).', + 429, + 2000, + 1 + ); + spy.restore(graphQLClient); + }); + }); + + it('should throw error when request is aborted value after retry', async function() { + this.timeout(3000); + nock('http://jssnextweb') + .post('/graphql') + .reply(429) + .post('/graphql') + .delay(100) + .reply(200, { + data: { + result: 'Hello world...', + }, + }); + + const graphQLClient = new GraphQLRequestClient(endpoint, { retries: 1, timeout: 50 }); + spy.on(graphQLClient['client'], 'request'); + try { + await graphQLClient.request('test'); + // If the request does not throw an error, fail the test + expect.fail('Expected request to throw an error'); + } catch (error) { + expect(graphQLClient['client'].request).to.be.called.exactly(2); + expect(error.name).to.equal('AbortError'); + } finally { + spy.restore(graphQLClient); + } + }); it('should throw error upon request timeout using provided timeout value', async () => { nock('http://jssnextweb') @@ -327,7 +327,7 @@ describe('GraphQLRequestClient', () => { }); } - it.only('should retry based on custom retryStrategy', async function() { + it('should retry based on custom retryStrategy', async function() { this.timeout(8000); nock('http://jssnextweb') diff --git a/packages/sitecore-jss/src/graphql-request-client.ts b/packages/sitecore-jss/src/graphql-request-client.ts index 01ae11bab3..6a92821cc9 100644 --- a/packages/sitecore-jss/src/graphql-request-client.ts +++ b/packages/sitecore-jss/src/graphql-request-client.ts @@ -53,7 +53,7 @@ export type GraphQLRequestClientConfig = { */ fetch?: typeof fetch; /** - * GraphQLClient request timeout + * GraphQLClient request timeout (in milliseconds). */ timeout?: number; /** @@ -200,16 +200,16 @@ export class GraphQLRequestClient implements GraphQLClient { const shouldRetry = this.retryStrategy.shouldRetry(error, attempt, this.retries); if (shouldRetry) { - const delaySeconds = this.retryStrategy.getDelay(error, attempt); + const delayMs = this.retryStrategy.getDelay(error, attempt); this.debug( - 'Error: %d. Rate limit reached for GraphQL endpoint. Retrying in %ds (attempt %d).', + 'Error: %d. Rate limit reached for GraphQL endpoint. Retrying in %dms (attempt %d).', status, - delaySeconds, + delayMs, attempt ); attempt++; - return new Promise((resolve) => setTimeout(resolve, delaySeconds)).then(retryer); + return new Promise((resolve) => setTimeout(resolve, delayMs)).then(retryer); } else { return Promise.reject(error); } From 2c22dd53dc4b5a5ccc78d9f4fcedc7906a64d15a Mon Sep 17 00:00:00 2001 From: Addy Pathania <89087450+sc-addypathania@users.noreply.github.com> Date: Fri, 9 Feb 2024 13:56:01 -0500 Subject: [PATCH 08/11] Update packages/sitecore-jss/src/graphql-request-client.ts Co-authored-by: Adam Brauer <400763+ambrauer@users.noreply.github.com> --- packages/sitecore-jss/src/graphql-request-client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sitecore-jss/src/graphql-request-client.ts b/packages/sitecore-jss/src/graphql-request-client.ts index 6a92821cc9..d589fadac5 100644 --- a/packages/sitecore-jss/src/graphql-request-client.ts +++ b/packages/sitecore-jss/src/graphql-request-client.ts @@ -57,7 +57,7 @@ export type GraphQLRequestClientConfig = { */ timeout?: number; /** - * Number of retries for client. Number of retries for client. Will use the specified `retryStrategy`. + * Number of retries for client. Will use the specified `retryStrategy`. */ retries?: number; /** From d17beae0d65da04ccf6fa8df94c6223c7fbca030 Mon Sep 17 00:00:00 2001 From: Addy Pathania <89087450+sc-addypathania@users.noreply.github.com> Date: Fri, 9 Feb 2024 13:56:07 -0500 Subject: [PATCH 09/11] Update packages/sitecore-jss/src/graphql-request-client.ts Co-authored-by: Adam Brauer <400763+ambrauer@users.noreply.github.com> --- packages/sitecore-jss/src/graphql-request-client.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/sitecore-jss/src/graphql-request-client.ts b/packages/sitecore-jss/src/graphql-request-client.ts index d589fadac5..36859a8bc1 100644 --- a/packages/sitecore-jss/src/graphql-request-client.ts +++ b/packages/sitecore-jss/src/graphql-request-client.ts @@ -61,8 +61,8 @@ export type GraphQLRequestClientConfig = { */ retries?: number; /** - * Retry strategy for the client. Default will use an exponential back-off strategy for - * codes 429, 502, 503, 504, 520, 521, 522, 523, 524. + * Retry strategy for the client. Uses `DefaultRetryStrategy` by default with exponential + * back-off factor of 2 for codes 429, 502, 503, 504, 520, 521, 522, 523, 524. */ retryStrategy?: RetryStrategy; }; From bcf3bfec0b2903b0ba5a0c36ac69a2019bed1187 Mon Sep 17 00:00:00 2001 From: Addy Pathania Date: Fri, 9 Feb 2024 14:04:39 -0500 Subject: [PATCH 10/11] add jsdoc --- packages/sitecore-jss/src/graphql-request-client.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/sitecore-jss/src/graphql-request-client.ts b/packages/sitecore-jss/src/graphql-request-client.ts index 6a92821cc9..01193e2c8c 100644 --- a/packages/sitecore-jss/src/graphql-request-client.ts +++ b/packages/sitecore-jss/src/graphql-request-client.ts @@ -82,10 +82,19 @@ export type GraphQLRequestClientFactory = ( */ export type GraphQLRequestClientFactoryConfig = { endpoint: string; apiKey?: string }; +/** + * Represents a default retry strategy for handling retry attempts in case of specific HTTP status codes. + * This class implements the RetryStrategy interface and provides methods to determine whether a request + * should be retried and calculates the delay before the next retry attempt. + */ export class DefaultRetryStrategy implements RetryStrategy { private statusCodes: number[]; 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 + */ constructor(statusCodes?: number[], factor?: number) { this.statusCodes = statusCodes || [429]; this.factor = factor || 2; From fafc75ef19b3a6bc411ccf2e3c8e2ebe46fd951d Mon Sep 17 00:00:00 2001 From: Addy Pathania Date: Fri, 9 Feb 2024 14:32:06 -0500 Subject: [PATCH 11/11] rephrase services comment --- .../src/lib/dictionary-service-factory.ts | 19 ++++++------------- .../nextjs/src/lib/layout-service-factory.ts | 19 ++++++------------- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts index 53f8213726..3c0fd28bc5 100644 --- a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts +++ b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/dictionary-service-factory.ts @@ -28,21 +28,14 @@ export class DictionaryServiceFactory { rootItemId: '{GUID}' */ /* - GraphQL endpoint may reach its rate limit with the amount of Layout and Dictionary requests it receives and throw a rate limit error. + GraphQL endpoint may reach its rate limit with the amount of requests it receives and throw a rate limit error. GraphQL Dictionary and Layout Services can handle rate limit errors from server and attempt a retry on requests. - For this, specify the number of retries the GraphQL client will attempt. + For this, specify the number of 'retries' the GraphQL client will attempt. It will only try the request once by default. - retries: 'number' - Additionally, you have the flexibility to customize the retry strategy by passing your customized RetryStrategy - object using the DefaultRetryStrategy class. The DefaultRetryStrategy class, which can be imported from the @sitecore-jss-nextjs - package, provides two essential methods: 'shouldRetry' which returns a boolean indicating whether a retry should - be attempted, and 'getDelay' which calculates the delay (in milliseconds) before the subsequent retry based on - the encountered error and the current attempt. - Example: - retryStrategy: new DefaultRetryStrategy({ - statusCodes: 'number[]', - factor: 'number' (The exponential factor to calculate backoff-time), - }), + + Additionally, you have the flexibility to customize the retry strategy by passing a 'retryStrategy'. + By default it uses the `DefaultRetryStrategy` with exponential back-off factor of 2 for error codes 429, + 502, 503, 504, 520, 521, 522, 523, and 524. You can use this class or your own implementation of `RetryStrategy`. */ retries: (process.env.GRAPH_QL_SERVICE_RETRIES && diff --git a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/layout-service-factory.ts b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/layout-service-factory.ts index 0a82d39b7b..1de97adc20 100644 --- a/packages/create-sitecore-jss/src/templates/nextjs/src/lib/layout-service-factory.ts +++ b/packages/create-sitecore-jss/src/templates/nextjs/src/lib/layout-service-factory.ts @@ -21,21 +21,14 @@ export class LayoutServiceFactory { siteName, clientFactory, /* - GraphQL endpoint may reach its rate limit with the amount of Layout and Dictionary requests it receives and throw a rate limit error. + GraphQL endpoint may reach its rate limit with the amount of requests it receives and throw a rate limit error. GraphQL Dictionary and Layout Services can handle rate limit errors from server and attempt a retry on requests. - For this, specify the number of retries the GraphQL client will attempt. + For this, specify the number of 'retries' the GraphQL client will attempt. It will only try the request once by default. - retries: 'number' - Additionally, you have the flexibility to customize the retry strategy by passing your customized RetryStrategy - object using the DefaultRetryStrategy class. The DefaultRetryStrategy class, which can be imported from the @sitecore-jss-nextjs - package, provides two essential methods: 'shouldRetry' which returns a boolean indicating whether a retry should - be attempted, and 'getDelay' which calculates the delay (in milliseconds) before the subsequent retry based on - the encountered error and the current attempt. - Example: - retryStrategy: new DefaultRetryStrategy({ - statusCodes: 'number[]', - factor: 'number' (The exponential factor to calculate backoff-time), - }), + + Additionally, you have the flexibility to customize the retry strategy by passing a 'retryStrategy'. + By default it uses the `DefaultRetryStrategy` with exponential back-off factor of 2 for error codes 429, + 502, 503, 504, 520, 521, 522, 523, and 524. You can use this class or your own implementation of `RetryStrategy`. */ retries: (process.env.GRAPH_QL_SERVICE_RETRIES &&