Bound external healthcheck probe with a timeout

AwsClient.healthcheck HeadBucket probe uses the shared client with
requestTimeout: 0, so an unreachable endpoint hangs clientCheck and
blocks cloudserver startup. Bound it with an AbortController and a new
externalBackendHealthCheckTimeout (5s) so a dead location fails fast.

Issue: ARSN-612

Author: maeldonn

lib/constants.ts

@@ -119,6 +119,8 @@ export const legacyLocations = ['sproxyd', 'legacy'];
 // for external backends, don't call unless at least 1 minute
 // (60,000 milliseconds) since last call
 export const externalBackendHealthCheckInterval = 60000;
+// bound each healthcheck probe so an unreachable endpoint can't hang startup
+export const externalBackendHealthCheckTimeout = 5000;
 // some of the available data backends  (if called directly rather
 // than through the multiple backend gateway) need a key provided
 // as a string as first parameter of the get/delete methods.

lib/storage/data/external/AwsClient.js

@@ -28,6 +28,7 @@ const getMetaHeaders = require('../../../s3middleware/userMetadata').getMetaHead
 const { prepareStream } = require('../../../s3middleware/prepareStream');
 const { createLogger, logHelper, removeQuotes, trimXMetaPrefix } = require('./utils');
 const jsutil = require('../../../jsutil');
+const { externalBackendHealthCheckTimeout } = require('../../../constants');
 const { callbackify } = require('util');
 
 const missingVerIdInternalError = errorInstances.InternalError.customizeDescription(
@@ -341,20 +342,28 @@ class AwsClient {
 
     healthcheck(location, callback) {
         const awsResp = {};
+        // bound the probe so an unreachable endpoint can't hang startup
+        const controller = new AbortController();
+        const timer = setTimeout(() => controller.abort(), externalBackendHealthCheckTimeout);
+        const done = (err, resp) => {
+            clearTimeout(timer);
+            return callback(err, resp);
+        };
         this._client
-            .send(new HeadBucketCommand({ Bucket: this._awsBucketName }))
+            .send(new HeadBucketCommand({ Bucket: this._awsBucketName }), { abortSignal: controller.signal })
             .then(() => {
                 if (!this._supportsVersioning) {
                     awsResp[location] = {
                         message: 'Congrats! You own the bucket',
                     };
-                    return callback(null, awsResp);
+                    return done(null, awsResp);
                 }
                 return this._client
                     .send(
                         new GetBucketVersioningCommand({
                             Bucket: this._awsBucketName,
                         }),
+                        { abortSignal: controller.signal },
                     )
                     .then(data => {
                         if (!data.Status || data.Status === 'Suspended') {
@@ -369,12 +378,12 @@ class AwsClient {
                                 message: 'Congrats! You own the bucket',
                             };
                         }
-                        return callback(null, awsResp);
+                        return done(null, awsResp);
                     });
             })
             .catch(err => {
                 awsResp[location] = { error: err, external: true };
-                return callback(null, awsResp);
+                return done(null, awsResp);
             });
     }
     createMPU(

tests/unit/storage/data/external/ExternalClients.spec.js

@@ -10,6 +10,7 @@ const DummyService = require('../DummyService');
 const { DummyRequestLogger } = require('../../../helpers');
 const BucketInfo = require('../../../../../lib/models/BucketInfo').default;
 const { HeadBucketCommand } = require('@aws-sdk/client-s3');
+const { externalBackendHealthCheckTimeout } = require('../../../../../lib/constants');
 
 const backendClients = [
     {
@@ -182,6 +183,36 @@ describe('external backend clients', () => {
             assert.strictEqual(entry.external, true);
         });
 
+        itIfAws(`${backend.name} healthcheck should pass an abort signal to bound the probe`, async () => {
+            const sendStub = sandbox.stub(testClient._client, 'send').resolves({});
+            await promisify(testClient.healthcheck.bind(testClient))(backend.config.dataStoreName);
+
+            const opts = sendStub.firstCall.args[1];
+            assert.ok(
+                opts && opts.abortSignal instanceof AbortSignal,
+                'expected healthcheck to pass an AbortSignal to send',
+            );
+        });
+
+        itIfAws(`${backend.name} healthcheck should abort a hanging probe and report an error`, async () => {
+            const clock = sandbox.useFakeTimers();
+            // unreachable endpoint: settles only when aborted
+            sandbox.stub(testClient._client, 'send').callsFake(
+                (_cmd, opts) =>
+                    new Promise((_resolve, reject) => {
+                        opts.abortSignal.addEventListener('abort', () => reject(new Error('aborted')));
+                    }),
+            );
+            const healthcheckPromise = promisify(testClient.healthcheck.bind(testClient))(backend.config.dataStoreName);
+
+            await clock.tickAsync(externalBackendHealthCheckTimeout);
+            const resp = await healthcheckPromise;
+
+            const entry = resp[backend.config.dataStoreName];
+            assert.ok(entry.error, 'expected the aborted probe to report an error');
+            assert.strictEqual(entry.external, true);
+        });
+
         it(`${backend.name} get() should stream a range of data`, async () => {
             const readable = await getAsync(
                 {