Skip to content

Commit

Permalink
feat: Throw by default from failed SQS publish (#1194)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
seb-cr authored Jan 19, 2024
1 parent 7bac68d commit dca7620
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 8 deletions.
12 changes: 12 additions & 0 deletions docs/migration/v2.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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).
Expand Down
7 changes: 4 additions & 3 deletions src/services/SQSService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
}
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/services/SQSService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down

0 comments on commit dca7620

Please sign in to comment.