feat: Throw by default from failed SQS publish (#1194)

Currently, sending an SQS message using `SQSService` does not throw an
error by default. This has caught us out a couple of times where we were
expecting a failure. The `failureMode` parameter was added to opt in to
throwing errors, to avoid introducing a breaking change in v1.

In v2 we'll throw errors by default, and you'll have to opt _out_ of
this behaviour.

BREAKING CHANGE: `SQSService#publish` throws by default instead of
handling errors. To maintain old behaviour, you can pass `"catch"` in
the `failureMode` parameter.

Author: seb-cr

docs/migration/v2.md

@@ -7,6 +7,7 @@ This doc summarises the breaking changes introduced in v2 and what you need to d
 - [Dependency injection](#dependency-injection)
 - [Services](#services)
   - [RequestService](#requestservice)
+  - [SQSService](#sqsservice)
 - [Models](#models)
   - [StatusModel](#statusmodel)
   - [SQSMessageModel](#sqsmessagemodel)
@@ -161,6 +162,17 @@ There are some small breaking changes to the built-in services. We've tried to m
 
 Header names returned by `getAllHeaders` are now lowercased. This is consistent with many other libraries (e.g. `http`, `axios`) and makes it easier to work with HTTP headers. In some cases it may be easier to change existing code to use `getHeader`, which has provided case-insensitive access to headers since [v1.2.0](https://github.com/comicrelief/lambda-wrapper/releases/tag/v1.2.0).
 
+### `SQSService`
+
+In v1, the default behaviour of `publish` was to catch any error thrown while sending the message to SQS. In v2, this has been changed and it will now throw an error by default. In most scenarios this is the more intuitive mode, as it ensures that the caller is made aware of any SQS-related failure. To maintain the old behaviour, you can pass `"catch"` in the `failureMode` parameter.
+
+```js
+// v1
+sqs.publish(queue, message);
+// v2 equivalent
+sqs.publish(queue, message, null, 'catch');
+```
+
 ## Models
 
 The `Model` base class has been removed. It's hard to make it type-safe (it tries to dynamically call setter methods) and we do modelling and validation differently now, using our [data-models](https://github.com/comicrelief/data-models) repo which is based around [Yup](https://github.com/jquense/yup).

src/services/SQSService.ts

@@ -429,10 +429,11 @@ export default class SQSService<
    * @param messageObject  object
    * @param messageGroupId string
    * @param failureMode Choose how failures are handled:
-   *   - `catch`: errors will be caught and logged. This is the default.
-   *   - `throw`: errors will be thrown, causing promise to reject.
+   *   - `throw`: errors will be thrown, causing promise to reject. (default)
+   *   - `catch`: errors will be caught and logged. Useful for non-critical
+   *     messages.
    */
-  async publish(queue: QueueName<TConfig>, messageObject: object, messageGroupId = null, failureMode: 'catch' | 'throw' = SQS_PUBLISH_FAILURE_MODES.CATCH) {
+  async publish(queue: QueueName<TConfig>, messageObject: object, messageGroupId = null, failureMode: 'catch' | 'throw' = SQS_PUBLISH_FAILURE_MODES.THROW) {
     if (!Object.values(SQS_PUBLISH_FAILURE_MODES).includes(failureMode)) {
       throw new Error(`Invalid value for 'failureMode': ${failureMode}`);
     }

tests/unit/services/SQSService.spec.ts

@@ -318,22 +318,22 @@ describe('unit.services.SQSService', () => {
     });
 
     describe('failure modes', () => {
-      it(`catches the error if publish fails with failureMode === ${SQS_PUBLISH_FAILURE_MODES.CATCH}`, async () => {
+      it('throws the error if publish fails with failureMode omitted', async () => {
         const service = getService({
           sendMessage: new Error('SQS is down!'),
         }, false);
 
-        const promise = service.publish(TEST_QUEUE, { test: 1 }, null, SQS_PUBLISH_FAILURE_MODES.CATCH);
+        const promise = service.publish(TEST_QUEUE, { test: 1 }, null);
 
-        await expect(promise).resolves.toEqual(null);
+        await expect(promise).rejects.toThrowError('SQS is down!');
       });
 
-      it('catches the error if publish fails with failureMode omitted', async () => {
+      it(`catches the error if publish fails with failureMode === ${SQS_PUBLISH_FAILURE_MODES.CATCH}`, async () => {
         const service = getService({
           sendMessage: new Error('SQS is down!'),
         }, false);
 
-        const promise = service.publish(TEST_QUEUE, { test: 1 }, null);
+        const promise = service.publish(TEST_QUEUE, { test: 1 }, null, SQS_PUBLISH_FAILURE_MODES.CATCH);
 
         await expect(promise).resolves.toEqual(null);
       });