Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow custom retry config and retry disabling in Firebase App options #1739

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 11 additions & 0 deletions etc/firebase-admin.api.md
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 @@ -470,6 +472,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
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
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;

/**

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably add more details on the meaning of the fields in RetryConfig? E.g. how to interpret backoffFactor

* 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
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
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
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
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
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
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
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
Expand Up @@ -54,11 +54,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 @@ -67,19 +75,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
30 changes: 28 additions & 2 deletions test/unit/utils/api-request.spec.ts
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(null, true);
const err = 'Error while making request: connection reset 1';
return client.send({
method: 'GET',
Expand Down