fix: Simplify promises and callbacks (#1203)

Replaces some of the harder-to-understand use of callback-style async
and `new Promise` with simpler async-await. As a bonus, we can remove
the `async` dependency!

This also fixes a bug in `SQSService#checkStatus`, where a failed
`listQueues` call can result in a `TypeError` if the `data` parameter of
the callback is undefined or null. Previously the unit tests ignored
this by passing an empty object (see the deleted `createCallbackMock`
function).

No specific Jira ticket, but removal of `async` completes [ENG-3212]

(Note at time of merge: a substantial amount of the callback-style
async calls to AWS APIs were already replaced with Promise chains
when we migrated to AWS SDK v3 in #1201. I've preserved the original
commit message, but it is no longer completely accurate!)

[ENG-3212]: https://comicrelief.atlassian.net/browse/ENG-3212

Author: seb-cr

package.json

@@ -53,7 +53,6 @@
     "@smithy/node-http-handler": "^2.5.0",
     "@types/aws-lambda": "^8.10.134",
     "alai": "1.0.3",
-    "async": "^3.2.5",
     "axios": "^1.6.7",
     "useragent": "2.3.0",
     "uuid": "^9.0.1",

src/services/RequestService.ts

@@ -259,18 +259,15 @@ export default class RequestService extends DependencyAwareClass {
   validateAgainstConstraints(constraints: object): Promise<void> {
     const logger = this.di.get(LoggerService);
 
-    return new Promise((resolve, reject) => {
-      const validation = validate(this.getAll(), constraints);
-
-      if (typeof validation === 'undefined') {
-        resolve();
-      } else {
-        logger.label('request-validation-failed');
-        const validationErrorResponse = ERROR_TYPES.VALIDATION_ERROR;
-        validationErrorResponse.setBodyVariable('validation_errors', validation);
-        reject(validationErrorResponse);
-      }
-    });
+    const validation = validate(this.getAll(), constraints);
+    if (typeof validation === 'undefined') {
+      return Promise.resolve();
+    }
+
+    logger.label('request-validation-failed');
+    const validationErrorResponse = ERROR_TYPES.VALIDATION_ERROR;
+    validationErrorResponse.setBodyVariable('validation_errors', validation);
+    return Promise.reject(validationErrorResponse);
   }
 
   /**

src/services/SQSService.ts

@@ -13,7 +13,6 @@ import {
 } from '@aws-sdk/client-sqs';
 import { NodeHttpHandler } from '@smithy/node-http-handler';
 import alai from 'alai';
-import { each } from 'async';
 import { v4 as uuid } from 'uuid';
 
 import DependencyAwareClass from '../core/DependencyAwareClass';
@@ -316,111 +315,92 @@ export default class SQSService<
    * @param queue
    * @param messageModels
    */
-  batchDelete(queue: QueueName<TConfig>, messageModels: SQSMessageModel[]): Promise<void> {
+  async batchDelete(queue: QueueName<TConfig>, messageModels: SQSMessageModel[]): Promise<void> {
     const queueUrl = this.queueUrls[queue];
     const logger = this.di.get(LoggerService);
     const timer = this.di.get(TimerService);
+
     const timerId = `sqs-batch-delete-${uuid()} - Queue: '${queueUrl}'`;
+    timer.start(timerId);
 
-    return new Promise<void>((resolve) => {
-      const messagesForDeletion: { Id: string; ReceiptHandle: string }[] = [];
-
-      timer.start(timerId);
-      // assuming openFiles is an array of file names
-      each(
-        messageModels,
-        (messageModel, callback) => {
-          if (messageModel instanceof SQSMessageModel && messageModel.isForDeletion() === true) {
-            messagesForDeletion.push({
-              Id: messageModel.getMessageId(),
-              ReceiptHandle: messageModel.getReceiptHandle(),
-            });
-          }
-          callback();
-        },
-        (loopError) => {
-          if (loopError) {
-            logger.error(loopError);
-            resolve();
-          }
-
-          this.sqs.send(new DeleteMessageBatchCommand({
-            Entries: messagesForDeletion,
-            QueueUrl: queueUrl,
-          })).finally(() => {
-            timer.stop(timerId);
-          }).catch((error) => {
-            logger.error(error);
-          }).then(() => {
-            resolve();
-          });
-        },
-      );
-    });
+    const messagesForDeletion = messageModels
+      .filter((messageModel) => (
+        messageModel instanceof SQSMessageModel
+        && messageModel.isForDeletion()
+      ))
+      .map((messageModel) => ({
+        Id: messageModel.getMessageId(),
+        ReceiptHandle: messageModel.getReceiptHandle(),
+      }));
+
+    try {
+      await this.sqs.send(new DeleteMessageBatchCommand({
+        Entries: messagesForDeletion,
+        QueueUrl: queueUrl,
+      }));
+    } catch (error) {
+      logger.error(error);
+    } finally {
+      timer.stop(timerId);
+    }
   }
 
   /**
    * Check SQS status.
    */
-  checkStatus(): Promise<ServiceStatus> {
+  async checkStatus(): Promise<ServiceStatus> {
     const logger = this.di.get(LoggerService);
     const timer = this.di.get(TimerService);
+
     const timerId = `sqs-list-queues-${uuid()}`;
+    timer.start(timerId);
 
-    return new Promise((resolve) => {
-      timer.start(timerId);
-
-      let status: Status = 'OK';
-
-      this.sqs.send(new ListQueuesCommand())
-        .finally(() => {
-          timer.stop(timerId);
-        })
-        .then((data) => {
-          if (typeof data.QueueUrls === 'undefined' || data.QueueUrls.length === 0) {
-            status = 'APPLICATION_FAILURE';
-          }
-        })
-        .catch((error) => {
-          logger.error(error);
-          status = 'APPLICATION_FAILURE';
-        })
-        .then(() => {
-          resolve({
-            service: 'SQS',
-            status,
-          });
-        });
-    });
+    let status: Status = 'OK';
+    try {
+      const result = await this.sqs.send(new ListQueuesCommand());
+      if (typeof result.QueueUrls === 'undefined' || result.QueueUrls.length === 0) {
+        status = 'APPLICATION_FAILURE';
+      }
+    } catch (error) {
+      logger.error(error);
+      status = 'APPLICATION_FAILURE';
+    } finally {
+      timer.stop(timerId);
+    }
+
+    return {
+      service: 'SQS',
+      status,
+    };
   }
 
   /**
    * Get the approximate number of messages in a queue.
    *
    * @param queue
    */
-  getMessageCount(queue: QueueName<TConfig>): Promise<number> {
+  async getMessageCount(queue: QueueName<TConfig>): Promise<number> {
     const queueUrl = this.queueUrls[queue];
     const logger = this.di.get(LoggerService);
     const timer = this.di.get(TimerService);
-    const timerId = `sqs-get-queue-attributes-${uuid()} - Queue: '${queueUrl}'`;
 
-    return new Promise((resolve) => {
-      timer.start(timerId);
+    const timerId = `sqs-get-queue-attributes-${uuid()} - Queue: '${queueUrl}'`;
+    timer.start(timerId);
 
-      this.sqs.send(new GetQueueAttributesCommand({
+    try {
+      const result = await this.sqs.send(new GetQueueAttributesCommand({
         AttributeNames: ['ApproximateNumberOfMessages'],
         QueueUrl: queueUrl,
-      })).finally(() => {
-        timer.stop(timerId);
-      }).then((data) => {
-        const messageCount = data.Attributes?.ApproximateNumberOfMessages || '0';
-        resolve(Number.parseInt(messageCount, 10));
-      }).catch((error) => {
-        logger.error(error);
-        resolve(0);
-      });
-    });
+      }));
+
+      const messageCount = result.Attributes?.ApproximateNumberOfMessages || '0';
+      return Number.parseInt(messageCount, 10);
+    } catch (error) {
+      logger.error(error);
+      return 0;
+    } finally {
+      timer.stop(timerId);
+    }
   }
 
   /**
@@ -530,31 +510,31 @@ export default class SQSService<
    * @param queue string
    * @param timeout number
    */
-  receive(queue: QueueName<TConfig>, timeout = 15): Promise<SQSMessageModel[]> {
+  async receive(queue: QueueName<TConfig>, timeout = 15): Promise<SQSMessageModel[]> {
     const queueUrl = this.queueUrls[queue];
     const logger = this.di.get(LoggerService);
     const timer = this.di.get(TimerService);
-    const timerId = `sqs-receive-message-${uuid()} - Queue: '${queueUrl}'`;
 
-    return new Promise((resolve, reject) => {
-      timer.start(timerId);
+    const timerId = `sqs-receive-message-${uuid()} - Queue: '${queueUrl}'`;
+    timer.start(timerId);
 
-      this.sqs.send(new ReceiveMessageCommand({
+    try {
+      const result = await this.sqs.send(new ReceiveMessageCommand({
         QueueUrl: queueUrl,
         VisibilityTimeout: timeout,
         MaxNumberOfMessages: 10,
-      })).finally(() => {
-        timer.stop(timerId);
-      }).then((data) => {
-        if (typeof data.Messages === 'undefined') {
-          resolve([]);
-        } else {
-          resolve(data.Messages.map((message) => new SQSMessageModel(message)));
-        }
-      }).catch((error) => {
-        logger.error(error);
-        reject(error);
-      });
-    });
+      }));
+
+      if (typeof result.Messages === 'undefined') {
+        return [];
+      }
+
+      return result.Messages.map((message) => new SQSMessageModel(message));
+    } catch (error) {
+      logger.error(error);
+      throw error;
+    } finally {
+      timer.stop(timerId);
+    }
   }
 }

yarn.lock

@@ -2885,11 +2885,6 @@ async@^3.2.3:
   resolved "https://registry.yarnpkg.com/async/-/async-3.2.4.tgz#2d22e00f8cddeb5fde5dd33522b56d1cf569a81c"
   integrity sha512-iAB+JbDEGXhyIUavoDl9WP/Jj106Kz9DEn1DPgYw5ruDn0e3Wgi3sKFm55sASdGBNOQB8F59d9qQ7deqrHA8wQ==
 
-async@^3.2.5:
-  version "3.2.5"
-  resolved "https://registry.yarnpkg.com/async/-/async-3.2.5.tgz#ebd52a8fdaf7a2289a24df399f8d8485c8a46b66"
-  integrity sha512-baNZyqaaLhyLVKm/DlvdW051MSgO6b8eVfIezl9E5PqWxFgzLm/wQntEW4zOytVburDEr0JlALEpdOFwvErLsg==
-
 asynckit@^0.4.0:
   version "0.4.0"
   resolved "https://registry.yarnpkg.com/asynckit/-/asynckit-0.4.0.tgz#c79ed97f7f34cb8f2ba1bc9790bcc366474b4b79"