diff --git a/src/utils/api-request.ts b/src/utils/api-request.ts index c53aefcab6..06fc476b1a 100644 --- a/src/utils/api-request.ts +++ b/src/utils/api-request.ts @@ -166,8 +166,79 @@ export class HttpError extends Error { } } +/** + * Specifies how failing HTTP requests should be retried. + */ +export interface RetryConfig { + /** Maximum number of times to retry a given request. */ + maxRetries: number; + + /** HTTP status codes that should be retried. */ + statusCodes?: number[]; + + /** Low-level I/O error codes that should be retried. */ + ioErrorCodes?: string[]; + + /** + * The multiplier for exponential back off. The retry delay is calculated in seconds using the formula + * `(2^n) * backOffFactor`, where n is the number of retries performed so far. When the backOffFactor is set + * to 0, retries are not delayed. When the backOffFactor is 1, retry duration is doubled each iteration. + */ + backOffFactor?: number; + + /** Maximum duration to wait before initiating a retry. */ + maxDelayInMillis: number; +} + +/** + * Default retry configuration for HTTP requests. Retries once on connection reset and timeout errors. + */ +const DEFAULT_RETRY_CONFIG: RetryConfig = { + maxRetries: 1, + ioErrorCodes: ['ECONNRESET', 'ETIMEDOUT'], + maxDelayInMillis: 60 * 1000, +}; + +/** + * Ensures that the given RetryConfig object is valid. + * + * @param retry The configuration to be validated. + */ +function validateRetryConfig(retry: RetryConfig) { + if (!validator.isNumber(retry.maxRetries) || retry.maxRetries < 0) { + throw new FirebaseAppError( + AppErrorCodes.INVALID_ARGUMENT, 'maxRetries must be a non-negative integer'); + } + + if (typeof retry.backOffFactor !== 'undefined') { + if (!validator.isNumber(retry.backOffFactor) || retry.backOffFactor < 0) { + throw new FirebaseAppError( + AppErrorCodes.INVALID_ARGUMENT, 'backOffFactor must be a non-negative number'); + } + } + + if (!validator.isNumber(retry.maxDelayInMillis) || retry.maxDelayInMillis < 0) { + throw new FirebaseAppError( + AppErrorCodes.INVALID_ARGUMENT, 'maxDelayInMillis must be a non-negative integer'); + } + + if (typeof retry.statusCodes !== 'undefined' && !validator.isArray(retry.statusCodes)) { + throw new FirebaseAppError(AppErrorCodes.INVALID_ARGUMENT, 'statusCodes must be an array'); + } + + if (typeof retry.ioErrorCodes !== 'undefined' && !validator.isArray(retry.ioErrorCodes)) { + throw new FirebaseAppError(AppErrorCodes.INVALID_ARGUMENT, 'ioErrorCodes must be an array'); + } +} + export class HttpClient { + constructor(private readonly retry: RetryConfig = DEFAULT_RETRY_CONFIG) { + if (this.retry) { + validateRetryConfig(this.retry); + } + } + /** * Sends an HTTP request to a remote server. If the server responds with a successful response (2xx), the returned * promise resolves with an HttpResponse. If the server responds with an error (3xx, 4xx, 5xx), the promise rejects @@ -179,7 +250,7 @@ export class HttpClient { * header should be explicitly set by the caller. To send a JSON leaf value (e.g. "foo", 5), parse it into JSON, * and pass as a string or a Buffer along with the appropriate content-type header. * - * @param {HttpRequest} request HTTP request to be sent. + * @param {HttpRequest} config HTTP request to be sent. * @return {Promise} A promise that resolves with the response details. */ public send(config: HttpRequestConfig): Promise { @@ -187,20 +258,30 @@ export class HttpClient { } /** - * Sends an HTTP request, and retries it once in case of low-level network errors. + * Sends an HTTP request. In the event of an error, retries the HTTP request according to the + * RetryConfig set on the HttpClient. + * + * @param {HttpRequestConfig} config HTTP request to be sent. + * @param {number} retryAttempts Number of retries performed up to now. + * @return {Promise} A promise that resolves with the response details. */ - private sendWithRetry(config: HttpRequestConfig, attempts: number = 0): Promise { + private sendWithRetry(config: HttpRequestConfig, retryAttempts: number = 0): Promise { return AsyncHttpCall.invoke(config) .then((resp) => { return this.createHttpResponse(resp); - }).catch((err: LowLevelError) => { - const retryCodes = ['ECONNRESET', 'ETIMEDOUT']; - if (retryCodes.indexOf(err.code) !== -1 && attempts === 0) { - return this.sendWithRetry(config, attempts + 1); + }) + .catch((err: LowLevelError) => { + const [delayMillis, canRetry] = this.getRetryDelayMillis(retryAttempts, err); + if (canRetry && delayMillis <= this.retry.maxDelayInMillis) { + return this.waitForRetry(delayMillis).then(() => { + return this.sendWithRetry(config, retryAttempts + 1); + }); } + if (err.response) { throw new HttpError(this.createHttpResponse(err.response)); } + if (err.code === 'ETIMEDOUT') { throw new FirebaseAppError( AppErrorCodes.NETWORK_TIMEOUT, @@ -218,6 +299,85 @@ export class HttpClient { } return new DefaultHttpResponse(resp); } + + private waitForRetry(delayMillis: number): Promise { + if (delayMillis > 0) { + return new Promise((resolve) => { + setTimeout(resolve, delayMillis); + }); + } + return Promise.resolve(); + } + + /** + * Checks if a failed request is eligible for a retry, and if so returns the duration to wait before initiating + * the retry. + * + * @param {number} retryAttempts Number of retries completed up to now. + * @param {LowLevelError} err The last encountered error. + * @returns {[number, boolean]} A 2-tuple where the 1st element is the duration to wait before another retry, and the + * 2nd element is a boolean indicating whether the request is eligible for a retry or not. + */ + private getRetryDelayMillis(retryAttempts: number, err: LowLevelError): [number, boolean] { + if (!this.isRetryEligible(retryAttempts, err)) { + return [0, false]; + } + + const response = err.response; + if (response && response.headers['retry-after']) { + const delayMillis = this.parseRetryAfterIntoMillis(response.headers['retry-after']); + if (delayMillis > 0) { + return [delayMillis, true]; + } + } + + return [this.backOffDelayMillis(retryAttempts), true]; + } + + private isRetryEligible(retryAttempts: number, err: LowLevelError): boolean { + if (!this.retry) { + return false; + } + + if (retryAttempts >= this.retry.maxRetries) { + return false; + } + + if (err.response) { + const statusCodes = this.retry.statusCodes || []; + return statusCodes.indexOf(err.response.status) !== -1; + } + + const retryCodes = this.retry.ioErrorCodes || []; + return retryCodes.indexOf(err.code) !== -1; + } + + /** + * Parses the Retry-After HTTP header as a milliseconds value. Return value is negative if the Retry-After header + * contains an expired timestamp or otherwise malformed. + */ + private parseRetryAfterIntoMillis(retryAfter: string): number { + const delaySeconds: number = parseInt(retryAfter, 10); + if (!isNaN(delaySeconds)) { + return delaySeconds * 1000; + } + + const date = new Date(retryAfter); + if (!isNaN(date.getTime())) { + return date.getTime() - Date.now(); + } + return -1; + } + + private backOffDelayMillis(retryAttempts: number): number { + if (retryAttempts === 0) { + return 0; + } + + const backOffFactor = this.retry.backOffFactor || 0; + const delayInSeconds = (2 ** retryAttempts) * backOffFactor; + return Math.min(delayInSeconds * 1000, this.retry.maxDelayInMillis); + } } /** diff --git a/src/utils/error.ts b/src/utils/error.ts index 2f3fc2d3fe..88e5b72335 100755 --- a/src/utils/error.ts +++ b/src/utils/error.ts @@ -321,6 +321,7 @@ export class FirebaseProjectManagementError extends PrefixedFirebaseError { export class AppErrorCodes { public static APP_DELETED = 'app-deleted'; public static DUPLICATE_APP = 'duplicate-app'; + public static INVALID_ARGUMENT = 'invalid-argument'; public static INTERNAL_ERROR = 'internal-error'; public static INVALID_APP_NAME = 'invalid-app-name'; public static INVALID_APP_OPTIONS = 'invalid-app-options'; diff --git a/test/unit/utils/api-request.spec.ts b/test/unit/utils/api-request.spec.ts index 6ab5209345..503137a5f4 100644 --- a/test/unit/utils/api-request.spec.ts +++ b/test/unit/utils/api-request.spec.ts @@ -27,7 +27,8 @@ import * as mocks from '../../resources/mocks'; import {FirebaseApp} from '../../../src/firebase-app'; import { - ApiSettings, HttpClient, HttpError, AuthorizedHttpClient, ApiCallbackFunction, HttpRequestConfig, parseHttpResponse, + ApiSettings, HttpClient, HttpError, AuthorizedHttpClient, ApiCallbackFunction, HttpRequestConfig, + HttpResponse, parseHttpResponse, } from '../../../src/utils/api-request'; import { deepCopy } from '../../../src/utils/deep-copy'; import {Agent} from 'http'; @@ -94,6 +95,8 @@ function mockRequestWithError(err: any) { describe('HttpClient', () => { let mockedRequests: nock.Scope[] = []; let transportSpy: sinon.SinonSpy = null; + let delayStub: sinon.SinonStub = null; + let clock: sinon.SinonFakeTimers = null; const sampleMultipartData = '--boundary\r\n' + 'Content-type: application/json\r\n\r\n' @@ -110,6 +113,59 @@ describe('HttpClient', () => { transportSpy.restore(); transportSpy = null; } + if (delayStub) { + delayStub.restore(); + delayStub = null; + } + if (clock) { + clock.restore(); + clock = null; + } + }); + + const invalidNumbers: any[] = ['string', null, undefined, {}, [], true, false, NaN, -1]; + const invalidArrays: any[] = ['string', null, {}, true, false, NaN, 0, 1]; + + invalidNumbers.forEach((maxRetries: any) => { + it(`should throw when maxRetries is: ${maxRetries}`, () => { + expect(() => { + new HttpClient({maxRetries} as any); + }).to.throw('maxRetries must be a non-negative integer'); + }); + }); + + invalidNumbers.forEach((backOffFactor: any) => { + if (typeof backOffFactor !== 'undefined') { + it(`should throw when backOffFactor is: ${backOffFactor}`, () => { + expect(() => { + new HttpClient({maxRetries: 1, backOffFactor} as any); + }).to.throw('backOffFactor must be a non-negative number'); + }); + } + }); + + invalidNumbers.forEach((maxDelayInMillis: any) => { + it(`should throw when maxDelayInMillis is: ${maxDelayInMillis}`, () => { + expect(() => { + new HttpClient({maxRetries: 1, maxDelayInMillis} as any); + }).to.throw('maxDelayInMillis must be a non-negative integer'); + }); + }); + + invalidArrays.forEach((ioErrorCodes: any) => { + it(`should throw when ioErrorCodes is: ${ioErrorCodes}`, () => { + expect(() => { + new HttpClient({maxRetries: 1, maxDelayInMillis: 10000, ioErrorCodes} as any); + }).to.throw('ioErrorCodes must be an array'); + }); + }); + + invalidArrays.forEach((statusCodes: any) => { + it(`should throw when statusCodes is: ${statusCodes}`, () => { + expect(() => { + new HttpClient({maxRetries: 1, maxDelayInMillis: 10000, statusCodes} as any); + }).to.throw('statusCodes must be an array'); + }); }); it('should be fulfilled for a 2xx response with a json payload', () => { @@ -636,6 +692,395 @@ describe('HttpClient', () => { }); }); + it('should not retry when RetryConfig is not set', () => { + mockedRequests.push(mockRequestWithError({message: 'connection reset 1', code: 'ECONNRESET'})); + const client = new HttpClient(null); + const err = 'Error while making request: connection reset 1'; + return client.send({ + method: 'GET', + url: mockUrl, + }).should.eventually.be.rejectedWith(err).and.have.property('code', 'app/network-error'); + }); + + it('should not retry when maxRetries is set to 0', () => { + mockedRequests.push(mockRequestWithError({message: 'connection reset 1', code: 'ECONNRESET'})); + const client = new HttpClient({ + maxRetries: 0, + ioErrorCodes: ['ECONNRESET'], + maxDelayInMillis: 10000, + }); + const err = 'Error while making request: connection reset 1'; + return client.send({ + method: 'GET', + url: mockUrl, + }).should.eventually.be.rejectedWith(err).and.have.property('code', 'app/network-error'); + }); + + it('should not retry when error codes are not configured', () => { + mockedRequests.push(mockRequestWithError({message: 'connection reset 1', code: 'ECONNRESET'})); + const client = new HttpClient({ + maxRetries: 1, + maxDelayInMillis: 10000, + }); + const err = 'Error while making request: connection reset 1'; + return client.send({ + method: 'GET', + url: mockUrl, + }).should.eventually.be.rejectedWith(err).and.have.property('code', 'app/network-error'); + }); + + it('should succeed after a retry on a configured I/O error', () => { + mockedRequests.push(mockRequestWithError({message: 'connection reset 1', code: 'ETESTCODE'})); + const respData = {foo: 'bar'}; + const scope = nock('https://' + mockHost) + .get(mockPath) + .reply(200, respData, { + 'content-type': 'application/json', + }); + mockedRequests.push(scope); + const client = new HttpClient({ + maxRetries: 1, + maxDelayInMillis: 1000, + ioErrorCodes: ['ETESTCODE'], + }); + return client.send({ + method: 'GET', + url: mockUrl, + }).then((resp) => { + expect(resp.status).to.equal(200); + expect(resp.data).to.deep.equal(respData); + }); + }); + + it('should succeed after a retry on a configured HTTP error', () => { + const scope1 = nock('https://' + mockHost) + .get(mockPath) + .reply(503, {}, { + 'content-type': 'application/json', + }); + mockedRequests.push(scope1); + const respData = {foo: 'bar'}; + const scope2 = nock('https://' + mockHost) + .get(mockPath) + .reply(200, respData, { + 'content-type': 'application/json', + }); + mockedRequests.push(scope2); + const client = new HttpClient({ + maxRetries: 1, + maxDelayInMillis: 1000, + statusCodes: [503], + }); + return client.send({ + method: 'GET', + url: mockUrl, + }).then((resp) => { + expect(resp.status).to.equal(200); + expect(resp.data).to.deep.equal(respData); + }); + }); + + it('should not retry more than maxRetries', () => { + // simulate 2 low-level errors + mockedRequests.push(mockRequestWithError({message: 'connection reset 1', code: 'ECONNRESET'})); + mockedRequests.push(mockRequestWithError({message: 'connection reset 2', code: 'ECONNRESET'})); + + // followed by 3 HTTP errors + const scope = nock('https://' + mockHost) + .get(mockPath) + .times(3) + .reply(503, {}, { + 'content-type': 'application/json', + }); + mockedRequests.push(scope); + + const client = new HttpClient({ + maxRetries: 4, + maxDelayInMillis: 10 * 1000, + ioErrorCodes: ['ECONNRESET'], + statusCodes: [503], + }); + return client.send({ + method: 'GET', + url: mockUrl, + }).catch((err: HttpError) => { + expect(err.message).to.equal('Server responded with status 503.'); + const resp = err.response; + expect(resp.status).to.equal(503); + expect(resp.headers['content-type']).to.equal('application/json'); + expect(resp.data).to.deep.equal({}); + expect(resp.isJson()).to.be.true; + }); + }); + + it('should not retry when retry-after exceeds maxDelayInMillis', () => { + const scope = nock('https://' + mockHost) + .get(mockPath) + .reply(503, {}, { + 'content-type': 'application/json', + 'retry-after': '61', + }); + mockedRequests.push(scope); + const client = new HttpClient({ + maxRetries: 1, + maxDelayInMillis: 60 * 1000, + statusCodes: [503], + }); + return client.send({ + method: 'GET', + url: mockUrl, + }).catch((err: HttpError) => { + expect(err.message).to.equal('Server responded with status 503.'); + const resp = err.response; + expect(resp.status).to.equal(503); + expect(resp.headers['content-type']).to.equal('application/json'); + expect(resp.data).to.deep.equal({}); + expect(resp.isJson()).to.be.true; + }); + }); + + it('should retry with exponential back off', () => { + const scope = nock('https://' + mockHost) + .get(mockPath) + .times(5) + .reply(503, {}, { + 'content-type': 'application/json', + }); + mockedRequests.push(scope); + const client = new HttpClient({ + maxRetries: 4, + backOffFactor: 0.5, + maxDelayInMillis: 60 * 1000, + statusCodes: [503], + }); + delayStub = sinon.stub(client as any, 'waitForRetry').resolves(); + return client.send({ + method: 'GET', + url: mockUrl, + }).catch((err: HttpError) => { + expect(err.message).to.equal('Server responded with status 503.'); + const resp = err.response; + expect(resp.status).to.equal(503); + expect(resp.headers['content-type']).to.equal('application/json'); + expect(resp.data).to.deep.equal({}); + expect(resp.isJson()).to.be.true; + expect(delayStub.callCount).to.equal(4); + const delays = delayStub.args.map((args) => args[0]); + expect(delays).to.deep.equal([0, 1000, 2000, 4000]); + }); + }); + + it('delay should not exceed maxDelayInMillis', () => { + const scope = nock('https://' + mockHost) + .get(mockPath) + .times(5) + .reply(503, {}, { + 'content-type': 'application/json', + }); + mockedRequests.push(scope); + const client = new HttpClient({ + maxRetries: 4, + backOffFactor: 1, + maxDelayInMillis: 4 * 1000, + statusCodes: [503], + }); + delayStub = sinon.stub(client as any, 'waitForRetry').resolves(); + return client.send({ + method: 'GET', + url: mockUrl, + }).catch((err: HttpError) => { + expect(err.message).to.equal('Server responded with status 503.'); + const resp = err.response; + expect(resp.status).to.equal(503); + expect(resp.headers['content-type']).to.equal('application/json'); + expect(resp.data).to.deep.equal({}); + expect(resp.isJson()).to.be.true; + expect(delayStub.callCount).to.equal(4); + const delays = delayStub.args.map((args) => args[0]); + expect(delays).to.deep.equal([0, 2000, 4000, 4000]); + }); + }); + + it('should retry without delays when backOffFactor is not set', () => { + const scope = nock('https://' + mockHost) + .get(mockPath) + .times(5) + .reply(503, {}, { + 'content-type': 'application/json', + }); + mockedRequests.push(scope); + const client = new HttpClient({ + maxRetries: 4, + maxDelayInMillis: 60 * 1000, + statusCodes: [503], + }); + delayStub = sinon.stub(client as any, 'waitForRetry').resolves(); + return client.send({ + method: 'GET', + url: mockUrl, + }).catch((err: HttpError) => { + expect(err.message).to.equal('Server responded with status 503.'); + const resp = err.response; + expect(resp.status).to.equal(503); + expect(resp.headers['content-type']).to.equal('application/json'); + expect(resp.data).to.deep.equal({}); + expect(resp.isJson()).to.be.true; + expect(delayStub.callCount).to.equal(4); + const delays = delayStub.args.map((args) => args[0]); + expect(delays).to.deep.equal([0, 0, 0, 0]); + }); + }); + + it('should wait when retry-after expressed as seconds', () => { + const scope1 = nock('https://' + mockHost) + .get(mockPath) + .reply(503, {}, { + 'content-type': 'application/json', + 'retry-after': '30', + }); + mockedRequests.push(scope1); + const respData = {foo: 'bar'}; + const scope2 = nock('https://' + mockHost) + .get(mockPath) + .reply(200, respData, { + 'content-type': 'application/json', + }); + mockedRequests.push(scope2); + + const client = new HttpClient({ + maxRetries: 4, + backOffFactor: 0.5, + maxDelayInMillis: 60 * 1000, + statusCodes: [503], + }); + delayStub = sinon.stub(client as any, 'waitForRetry').resolves(); + return client.send({ + method: 'GET', + url: mockUrl, + }).then((resp: HttpResponse) => { + expect(resp.status).to.equal(200); + expect(resp.headers['content-type']).to.equal('application/json'); + expect(resp.data).to.deep.equal(respData); + expect(resp.isJson()).to.be.true; + expect(delayStub.callCount).to.equal(1); + expect(delayStub.args[0][0]).to.equal(30 * 1000); + }); + }); + + it('should wait when retry-after expressed as a timestamp', () => { + clock = sinon.useFakeTimers(); + clock.setSystemTime(1000); + const timestamp = new Date(clock.now + 30 * 1000); + + const scope1 = nock('https://' + mockHost) + .get(mockPath) + .reply(503, {}, { + 'content-type': 'application/json', + 'retry-after': timestamp.toUTCString(), + }); + mockedRequests.push(scope1); + const respData = {foo: 'bar'}; + const scope2 = nock('https://' + mockHost) + .get(mockPath) + .reply(200, respData, { + 'content-type': 'application/json', + }); + mockedRequests.push(scope2); + + const client = new HttpClient({ + maxRetries: 4, + backOffFactor: 0.5, + maxDelayInMillis: 60 * 1000, + statusCodes: [503], + }); + delayStub = sinon.stub(client as any, 'waitForRetry').resolves(); + return client.send({ + method: 'GET', + url: mockUrl, + }).then((resp: HttpResponse) => { + expect(resp.status).to.equal(200); + expect(resp.headers['content-type']).to.equal('application/json'); + expect(resp.data).to.deep.equal(respData); + expect(resp.isJson()).to.be.true; + expect(delayStub.callCount).to.equal(1); + expect(delayStub.args[0][0]).to.equal(30 * 1000); + }); + }); + + it('should not wait when retry-after timestamp is expired', () => { + const timestamp = new Date(Date.now() - 30 * 1000); + + const scope1 = nock('https://' + mockHost) + .get(mockPath) + .reply(503, {}, { + 'content-type': 'application/json', + 'retry-after': timestamp.toUTCString(), + }); + mockedRequests.push(scope1); + const respData = {foo: 'bar'}; + const scope2 = nock('https://' + mockHost) + .get(mockPath) + .reply(200, respData, { + 'content-type': 'application/json', + }); + mockedRequests.push(scope2); + + const client = new HttpClient({ + maxRetries: 4, + backOffFactor: 0.5, + maxDelayInMillis: 60 * 1000, + statusCodes: [503], + }); + delayStub = sinon.stub(client as any, 'waitForRetry').resolves(); + return client.send({ + method: 'GET', + url: mockUrl, + }).then((resp: HttpResponse) => { + expect(resp.status).to.equal(200); + expect(resp.headers['content-type']).to.equal('application/json'); + expect(resp.data).to.deep.equal(respData); + expect(resp.isJson()).to.be.true; + expect(delayStub.callCount).to.equal(1); + expect(delayStub.args[0][0]).to.equal(0); + }); + }); + + it('should not wait when retry-after is malformed', () => { + const scope1 = nock('https://' + mockHost) + .get(mockPath) + .reply(503, {}, { + 'content-type': 'application/json', + 'retry-after': 'invalid', + }); + mockedRequests.push(scope1); + const respData = {foo: 'bar'}; + const scope2 = nock('https://' + mockHost) + .get(mockPath) + .reply(200, respData, { + 'content-type': 'application/json', + }); + mockedRequests.push(scope2); + + const client = new HttpClient({ + maxRetries: 4, + backOffFactor: 0.5, + maxDelayInMillis: 60 * 1000, + statusCodes: [503], + }); + delayStub = sinon.stub(client as any, 'waitForRetry').resolves(); + return client.send({ + method: 'GET', + url: mockUrl, + }).then((resp: HttpResponse) => { + expect(resp.status).to.equal(200); + expect(resp.headers['content-type']).to.equal('application/json'); + expect(resp.data).to.deep.equal(respData); + expect(resp.isJson()).to.be.true; + expect(delayStub.callCount).to.equal(1); + expect(delayStub.args[0][0]).to.equal(0); + }); + }); + it('should reject if the request payload is invalid', () => { const client = new HttpClient(); const err = 'Error while making request: Request data must be a string, a Buffer '