Automatically retrying on 503 errors (#556)

* Automatically retrying on 503 errors

* Using explicit RetryConfig for tests

* Added tests for defaultRetryConfig()

* Using defaultRetryConfig() explicitly for clarity

Author: hiranya911

CHANGELOG.md

@@ -4,6 +4,7 @@
   apps in a Firebase project
 - [added] `admin.projectManagement().setDisplayName()` method to update the display name of a
   Firebase project
+- [fixed] The SDK now automatically retries HTTP calls failing due to 503 errors.
 
 # v8.0.0
 

src/utils/api-request.ts

@@ -191,13 +191,19 @@ export interface RetryConfig {
 }
 
 /**
- * Default retry configuration for HTTP requests. Retries once on connection reset and timeout errors.
+ * Default retry configuration for HTTP requests. Retries up to 4 times on connection reset and timeout errors
+ * as well as HTTP 503 errors. Exposed as a function to ensure that every HttpClient gets its own RetryConfig
+ * instance.
  */
-const DEFAULT_RETRY_CONFIG: RetryConfig = {
-  maxRetries: 1,
-  ioErrorCodes: ['ECONNRESET', 'ETIMEDOUT'],
-  maxDelayInMillis: 60 * 1000,
-};
+export function defaultRetryConfig(): RetryConfig {
+  return {
+    maxRetries: 4,
+    statusCodes: [503],
+    ioErrorCodes: ['ECONNRESET', 'ETIMEDOUT'],
+    backOffFactor: 0.5,
+    maxDelayInMillis: 60 * 1000,
+  };
+}
 
 /**
  * Ensures that the given RetryConfig object is valid.
@@ -233,7 +239,7 @@ function validateRetryConfig(retry: RetryConfig) {
 
 export class HttpClient {
 
-  constructor(private readonly retry: RetryConfig = DEFAULT_RETRY_CONFIG) {
+  constructor(private readonly retry: RetryConfig = defaultRetryConfig()) {
     if (this.retry) {
       validateRetryConfig(this.retry);
     }

test/unit/messaging/messaging.spec.ts

@@ -299,6 +299,9 @@ function mockTopicSubscriptionRequestWithError(
     });
 }
 
+function disableRetries(messaging: Messaging) {
+  (messaging as any).messagingRequestHandler.httpClient.retry = null;
+}
 
 describe('Messaging', () => {
   let mockApp: FirebaseApp;
@@ -1167,6 +1170,7 @@ describe('Messaging', () => {
     _.forEach(STATUS_CODE_TO_ERROR_MAP, (expectedError, statusCode) => {
       it(`should be rejected given a ${ statusCode } text server response`, () => {
         mockedRequests.push(mockSendRequestWithError(parseInt(statusCode, 10), 'text'));
+        disableRetries(messaging);
 
         return messaging.sendToDevice(
           mocks.messaging.registrationToken,
@@ -1460,6 +1464,7 @@ describe('Messaging', () => {
     _.forEach(STATUS_CODE_TO_ERROR_MAP, (expectedError, statusCode) => {
       it(`should be rejected given a ${ statusCode } text server response`, () => {
         mockedRequests.push(mockSendRequestWithError(parseInt(statusCode, 10), 'text'));
+        disableRetries(messaging);
 
         return messaging.sendToDeviceGroup(
         mocks.messaging.notificationKey,
@@ -1710,6 +1715,7 @@ describe('Messaging', () => {
     _.forEach(STATUS_CODE_TO_ERROR_MAP, (expectedError, statusCode) => {
       it(`should be rejected given a ${ statusCode } text server response`, () => {
         mockedRequests.push(mockSendRequestWithError(parseInt(statusCode, 10), 'text'));
+        disableRetries(messaging);
 
         return messaging.sendToTopic(
         mocks.messaging.topic,
@@ -1928,6 +1934,7 @@ describe('Messaging', () => {
     _.forEach(STATUS_CODE_TO_ERROR_MAP, (expectedError, statusCode) => {
       it(`should be rejected given a ${ statusCode } text server response`, () => {
         mockedRequests.push(mockSendRequestWithError(parseInt(statusCode, 10), 'text'));
+        disableRetries(messaging);
 
         return messaging.sendToCondition(
         mocks.messaging.condition,
@@ -3398,6 +3405,7 @@ describe('Messaging', () => {
     _.forEach(STATUS_CODE_TO_ERROR_MAP, (expectedError, statusCode) => {
       it(`should be rejected given a ${ statusCode } text server response`, () => {
         mockedRequests.push(mockTopicSubscriptionRequestWithError(methodName, parseInt(statusCode, 10), 'text'));
+        disableRetries(messaging);
 
         return messagingService[methodName](
           mocks.messaging.registrationToken,