Skip to content

Commit

Permalink
feat: add retry config customization in app init
Browse files Browse the repository at this point in the history
  • Loading branch information
thiagomorales committed Jun 4, 2022
1 parent a96729a commit cdb480b
Show file tree
Hide file tree
Showing 13 changed files with 94 additions and 16 deletions.
11 changes: 11 additions & 0 deletions etc/firebase-admin.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,10 @@ export interface AppOptions {
credential?: Credential;
databaseAuthVariableOverride?: object | null;
databaseURL?: string;
disableRetry?: boolean;
httpAgent?: Agent;
projectId?: string;
retryConfig?: RetryConfig;
serviceAccountId?: string;
storageBucket?: string;
}
Expand Down Expand Up @@ -462,6 +464,15 @@ export namespace remoteConfig {
export type Version = Version;
}

// @public
export interface RetryConfig {
backOffFactor?: number;
ioErrorCodes?: string[];
maxDelayInMillis: number;
maxRetries: number;
statusCodes?: number[];
}

// @public (undocumented)
export const SDK_VERSION: string;

Expand Down
11 changes: 11 additions & 0 deletions etc/firebase-admin.app.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ export interface AppOptions {
credential?: Credential;
databaseAuthVariableOverride?: object | null;
databaseURL?: string;
disableRetry?: boolean;
httpAgent?: Agent;
projectId?: string;
retryConfig?: RetryConfig;
serviceAccountId?: string;
storageBucket?: string;
}
Expand Down Expand Up @@ -73,6 +75,15 @@ export function initializeApp(options?: AppOptions, appName?: string): App;
// @public
export function refreshToken(refreshTokenPathOrObject: string | object, httpAgent?: Agent): Credential;

// @public
export interface RetryConfig {
backOffFactor?: number;
ioErrorCodes?: string[];
maxDelayInMillis: number;
maxRetries: number;
statusCodes?: number[];
}

// @public (undocumented)
export const SDK_VERSION: string;

Expand Down
11 changes: 11 additions & 0 deletions src/app/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import { Agent } from 'http';

import { Credential } from './credential';
import { RetryConfig } from '.'

/**
* Available options to pass to {@link firebase-admin.app#initializeApp}.
Expand Down Expand Up @@ -83,6 +84,16 @@ export interface AppOptions {
* specifying an HTTP Agent in the corresponding factory methods.
*/
httpAgent?: Agent;

/**
* The retry configuration that will be used in HttpClient for API Requests.
*/
retryConfig?: RetryConfig;

/**
* Completely disable the retry strategy in HttpClient.
*/
disableRetry?: boolean;
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,6 @@ export { initializeApp, getApp, getApps, deleteApp } from './lifecycle';

export { Credential, ServiceAccount, GoogleOAuthAccessToken } from './credential';
export { applicationDefault, cert, refreshToken } from './credential-factory';
export { RetryConfig } from '../utils/api-request'

export const SDK_VERSION = getSdkVersion();
2 changes: 1 addition & 1 deletion src/auth/auth-api-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const FIREBASE_AUTH_HEADER = {
'X-Client-Version': `Node/Admin/${utils.getSdkVersion()}`,
};
/** Firebase Auth request timeout duration in milliseconds. */
const FIREBASE_AUTH_TIMEOUT = 25000;
const FIREBASE_AUTH_TIMEOUT = parseInt(process.env.FIREBASE_AUTH_TIMEOUT || '25000', 10);


/** List of reserved claims which cannot be provided when creating a custom token. */
Expand Down
1 change: 1 addition & 0 deletions src/firebase-namespace-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ export { projectManagement } from './project-management/project-management-names
export { remoteConfig } from './remote-config/remote-config-namespace';
export { securityRules } from './security-rules/security-rules-namespace';
export { storage } from './storage/storage-namespace';
export { RetryConfig } from './utils/api-request'

// Declare other top-level members of the admin namespace below. Unfortunately, there's no
// compile-time mechanism to ensure that the FirebaseNamespace class actually provides these
Expand Down
4 changes: 2 additions & 2 deletions src/messaging/batch-request-internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
import { FirebaseAppError, AppErrorCodes } from '../utils/error';

const PART_BOUNDARY = '__END_OF_PART__';
const TEN_SECONDS_IN_MILLIS = 10000;
const FIREBASE_MESSAGING_BATCH_TIMEOUT = parseInt(process.env.FIREBASE_MESSAGING_BATCH_TIMEOUT || '10000', 10);

/**
* Represents a request that can be sent as part of an HTTP batch request.
Expand Down Expand Up @@ -72,7 +72,7 @@ export class BatchRequestClient {
url: this.batchUrl,
data: this.getMultipartPayload(requests),
headers: Object.assign({}, this.commonHeaders, requestHeaders),
timeout: TEN_SECONDS_IN_MILLIS,
timeout: FIREBASE_MESSAGING_BATCH_TIMEOUT,
};
return this.httpClient.send(request).then((response) => {
if (!response.multipart) {
Expand Down
2 changes: 1 addition & 1 deletion src/messaging/messaging-api-request-internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { SendResponse, BatchResponse } from './messaging-api';


// FCM backend constants
const FIREBASE_MESSAGING_TIMEOUT = 10000;
const FIREBASE_MESSAGING_TIMEOUT = parseInt(process.env.FIREBASE_MESSAGING_TIMEOUT || '10000', 10);
const FIREBASE_MESSAGING_BATCH_URL = 'https://fcm.googleapis.com/batch';
const FIREBASE_MESSAGING_HTTP_METHOD: HttpMethod = 'POST';
const FIREBASE_MESSAGING_HEADERS = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const PROJECT_MANAGEMENT_HEADERS = {
'X-Client-Version': `Node/Admin/${getSdkVersion()}`,
};
/** Project management request timeout duration in milliseconds. */
const PROJECT_MANAGEMENT_TIMEOUT_MILLIS = 10000;
const FIREBASE_PROJECT_MANAGEMENT_TIMEOUT = parseInt(process.env.FIREBASE_PROJECT_MANAGEMENT_TIMEOUT || '10000', 10);

const LIST_APPS_MAX_PAGE_SIZE = 100;

Expand Down Expand Up @@ -315,7 +315,7 @@ export class ProjectManagementRequestHandler {
url: `${baseUrlToUse}${path}`,
headers: PROJECT_MANAGEMENT_HEADERS,
data: requestData,
timeout: PROJECT_MANAGEMENT_TIMEOUT_MILLIS,
timeout: FIREBASE_PROJECT_MANAGEMENT_TIMEOUT,
};

return this.httpClient.send(request)
Expand Down
13 changes: 9 additions & 4 deletions src/utils/api-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,13 @@ function validateRetryConfig(retry: RetryConfig): void {

export class HttpClient {

constructor(private readonly retry: RetryConfig | null = defaultRetryConfig()) {
if (this.retry) {
constructor(
private readonly retry: RetryConfig | null = null,
private readonly disableRetry: boolean = false
) {
if (!this.disableRetry && !this.retry) {
this.retry = defaultRetryConfig();
} else if (!this.disableRetry && this.retry) {
validateRetryConfig(this.retry);
}
}
Expand Down Expand Up @@ -345,7 +350,7 @@ export class HttpClient {
}

private isRetryEligible(retryAttempts: number, err: LowLevelError): boolean {
if (!this.retry) {
if (this.disableRetry || !this.retry) {
return false;
}

Expand Down Expand Up @@ -811,7 +816,7 @@ class HttpRequestConfigImpl implements HttpRequestConfig {

export class AuthorizedHttpClient extends HttpClient {
constructor(private readonly app: FirebaseApp) {
super();
super(app.options.retryConfig, app.options.disableRetry);
}

public send(request: HttpRequestConfig): Promise<HttpResponse> {
Expand Down
12 changes: 12 additions & 0 deletions test/resources/mocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,19 @@ export const databaseAuthVariableOverride = { 'some#string': 'some#val' };
export const storageBucket = 'bucketName.appspot.com';

export const credential = cert(path.resolve(__dirname, './mock.key.json'));
export const retryConfig = {
maxRetries: 15,
statusCodes: [507],
ioErrorCodes: ['ECONNRESET', 'ETIMEDOUT'],
backOffFactor: 0.8,
maxDelayInMillis: 190000,
}

export const appOptions: AppOptions = {
credential,
databaseURL,
storageBucket,
retryConfig,
};

export const appOptionsWithOverride: AppOptions = {
Expand All @@ -66,19 +74,23 @@ export const appOptionsWithOverride: AppOptions = {
databaseURL,
storageBucket,
projectId,
retryConfig,
};

export const appOptionsNoAuth: AppOptions = {
databaseURL,
retryConfig,
};

export const appOptionsNoDatabaseUrl: AppOptions = {
credential,
retryConfig,
};

export const appOptionsAuthDB: AppOptions = {
credential,
databaseURL,
retryConfig,
};

export class MockCredential implements Credential {
Expand Down
8 changes: 4 additions & 4 deletions test/unit/machine-learning/machine-learning.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ describe('MachineLearning', () => {
stubs.push(stub);
return machineLearning.createModel(MODEL_OPTIONS_WITH_GCS)
.should.eventually.be.rejected.and.have.property(
'message', 'Cannot read property \'done\' of null');
'message', 'Cannot read properties of null (reading \'done\')');
});

it('should reject when API response does not contain a name', () => {
Expand Down Expand Up @@ -699,7 +699,7 @@ describe('MachineLearning', () => {
stubs.push(stub);
return machineLearning.updateModel(MODEL_ID, MODEL_OPTIONS_WITH_GCS)
.should.eventually.be.rejected.and.have.property(
'message', 'Cannot read property \'done\' of null');
'message', 'Cannot read properties of null (reading \'done\')');
});

it('should reject when API response does not contain a name', () => {
Expand Down Expand Up @@ -803,7 +803,7 @@ describe('MachineLearning', () => {
stubs.push(stub);
return machineLearning.publishModel(MODEL_ID)
.should.eventually.be.rejected.and.have.property(
'message', 'Cannot read property \'done\' of null');
'message', 'Cannot read properties of null (reading \'done\')');
});

it('should reject when API response does not contain a name', () => {
Expand Down Expand Up @@ -907,7 +907,7 @@ describe('MachineLearning', () => {
stubs.push(stub);
return machineLearning.unpublishModel(MODEL_ID)
.should.eventually.be.rejected.and.have.property(
'message', 'Cannot read property \'done\' of null');
'message', 'Cannot read properties of null (reading \'done\')');
});

it('should reject when API response does not contain a name', () => {
Expand Down
30 changes: 28 additions & 2 deletions test/unit/utils/api-request.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -786,9 +786,35 @@ describe('HttpClient', () => {
});
});

it('should not retry when RetryConfig is explicitly null', () => {
it('should succeed, with custom retry config after 1 retry, on a single internal server error response', () => {
mockedRequests.push(mockRequestWithHttpError(500));
const customRetryConfig = {
maxRetries: 4,
statusCodes: [500],
ioErrorCodes: [],
backOffFactor: 0.5,
maxDelayInMillis: 60 * 1000,
};
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(customRetryConfig);
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 when retry is explicitly disabled', () => {
mockedRequests.push(mockRequestWithError({ message: 'connection reset 1', code: 'ECONNRESET' }));
const client = new HttpClient(null);
const client = new HttpClient(mocks.retryConfig, true);
const err = 'Error while making request: connection reset 1';
return client.send({
method: 'GET',
Expand Down

0 comments on commit cdb480b

Please sign in to comment.