Merge pull request #1721 from ably/1717/fix-rest-fallback-behavior

[ECO-4721] Fix rest fallback behavior
ably · Apr 17, 2024 · bf2bfcc · bf2bfcc
2 parents 0c0021a + 5419ef3
commit bf2bfcc
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 2 deletions.
diff --git a/src/common/lib/util/defaults.ts b/src/common/lib/util/defaults.ts
@@ -23,6 +23,7 @@ type CompleteDefaults = IDefaults & {
     disconnectedRetryTimeout: number;
     suspendedRetryTimeout: number;
     httpRequestTimeout: number;
+    httpMaxRetryDuration: number;
     channelRetryTimeout: number;
     fallbackRetryTimeout: number;
     connectionStateTtl: number;
@@ -73,7 +74,8 @@ const Defaults = {
     disconnectedRetryTimeout: 15000,
     suspendedRetryTimeout: 30000,
     /* Undocumented, but part of the api and can be used by customers: */
-    httpRequestTimeout: 15000,
+    httpRequestTimeout: 10000,
+    httpMaxRetryDuration: 15000,
     channelRetryTimeout: 15000,
     fallbackRetryTimeout: 600000,
     /* For internal / test use only: */

diff --git a/src/common/types/http.ts b/src/common/types/http.ts
@@ -192,10 +192,24 @@ export class Http {
         return this.doUri(method, uriFromHost(hosts[0]), headers, body, params);
       }
 
+      let tryAHostStartedAt: Date | null = null;
       const tryAHost = async (candidateHosts: Array<string>, persistOnSuccess?: boolean): Promise<RequestResult> => {
         const host = candidateHosts.shift();
+        tryAHostStartedAt = tryAHostStartedAt ?? new Date();
         const result = await this.doUri(method, uriFromHost(host as string), headers, body, params);
         if (result.error && this.platformHttp.shouldFallback(result.error as ErrnoException) && candidateHosts.length) {
+          // TO3l6
+          const elapsedTime = Date.now() - tryAHostStartedAt.getTime();
+          if (elapsedTime > client.options.timeouts.httpMaxRetryDuration) {
+            return {
+              error: new ErrorInfo(
+                `Timeout for trying fallback hosts retries. Total elapsed time exceeded the ${client.options.timeouts.httpMaxRetryDuration}ms limit`,
+                50003,
+                500,
+              ),
+            };
+          }
+
           return tryAHost(candidateHosts, true);
         }
         if (persistOnSuccess) {

diff --git a/test/realtime/init.test.js b/test/realtime/init.test.js
@@ -238,6 +238,7 @@ define(['ably', 'shared_helper', 'chai'], function (Ably, helper, chai) {
           disconnectedRetryTimeout: 123,
           suspendedRetryTimeout: 456,
           httpRequestTimeout: 789,
+          httpMaxRetryDuration: 321,
         });
         /* Note: uses internal knowledge of connectionManager */
         try {
@@ -251,7 +252,11 @@ define(['ably', 'shared_helper', 'chai'], function (Ably, helper, chai) {
           );
           expect(realtime.connection.connectionManager.options.timeouts.httpRequestTimeout).to.equal(
             789,
-            'Verify suspended retry frequency is settable',
+            'Verify http request timeout is settable',
+          );
+          expect(realtime.connection.connectionManager.options.timeouts.httpMaxRetryDuration).to.equal(
+            321,
+            'Verify http max retry duration is settable',
           );
         } catch (err) {
           closeAndFinish(done, realtime, err);

diff --git a/test/rest/fallbacks.test.js b/test/rest/fallbacks.test.js
@@ -50,5 +50,58 @@ define(['shared_helper', 'async', 'chai'], function (helper, async, chai) {
       expect(currentFallback && currentFallback.host).to.equal(goodHost, 'Check good host set again');
       expect(currentFallback.validUntil > now, 'Check validUntil has been re-set').to.be.ok;
     });
+
+    // TO3l6
+    describe('Max elapsed time for host retries', function () {
+      it('can timeout after default host', async function () {
+        const httpRequestTimeout = 1000;
+        // set httpMaxRetryDuration lower than httpRequestTimeout so it would timeout after default host attempt
+        const httpMaxRetryDuration = Math.floor(httpRequestTimeout / 2);
+        const rest = helper.AblyRest({
+          restHost: helper.unroutableHost,
+          fallbackHosts: [helper.unroutableHost],
+          httpRequestTimeout,
+          httpMaxRetryDuration,
+        });
+
+        let thrownError = null;
+        try {
+          // we expect it to fail due to max elapsed time reached for host retries
+          await rest.time();
+        } catch (error) {
+          thrownError = error;
+        }
+
+        expect(thrownError).not.to.be.null;
+        expect(thrownError.message).to.equal(
+          `Timeout for trying fallback hosts retries. Total elapsed time exceeded the ${httpMaxRetryDuration}ms limit`,
+        );
+      });
+
+      it('can timeout after fallback host retries', async function () {
+        const httpRequestTimeout = 1000;
+        // set httpMaxRetryDuration higher than httpRequestTimeout and lower than 2*httpRequestTimeout so it would timeout after first fallback host retry attempt
+        const httpMaxRetryDuration = Math.floor(httpRequestTimeout * 1.5);
+        const rest = helper.AblyRest({
+          restHost: helper.unroutableHost,
+          fallbackHosts: [helper.unroutableHost, helper.unroutableHost],
+          httpRequestTimeout,
+          httpMaxRetryDuration,
+        });
+
+        let thrownError = null;
+        try {
+          // we expect it to fail due to max elapsed time reached for host retries
+          await rest.time();
+        } catch (error) {
+          thrownError = error;
+        }
+
+        expect(thrownError).not.to.be.null;
+        expect(thrownError.message).to.equal(
+          `Timeout for trying fallback hosts retries. Total elapsed time exceeded the ${httpMaxRetryDuration}ms limit`,
+        );
+      });
+    });
   });
 });