bbc / sqs-consumer

Build Amazon Simple Queue Service (SQS) based applications without the boilerplate
https://bbc.github.io/sqs-consumer/
Other
1.72k stars 331 forks source link

feat: sqs-consumer should have the option to abort requests on stop #346

Closed nicholasgriffintn closed 1 year ago

nicholasgriffintn commented 1 year ago

Problem

There have been a number of requests that sqs-consumer should abort any existing requests when .stop() is called, as in some implementations this is expected behaviour.

Originally posted in https://github.com/bbc/sqs-consumer/issues/234 From @jwalton: > When you call start(), it calls into [poll() immediately and synchronously](https://github.com/bbc/sqs-consumer/blob/961e842c0f9091a944ee4dc4d18f0e241940ec26/src/consumer.ts#L112), so this.stopped will be false when we call into poll(). That test case validates that [handleMessage was called once](https://github.com/bbc/sqs-consumer/blob/961e842c0f9091a944ee4dc4d18f0e241940ec26/test/consumer.test.ts#L992), and it should be called not at all. > > Even if the handler were not called again, there is other undesirable behavior from the current state of affairs. If you write a program that calls start() and then stop(), the program will take 20 seconds to run, even though it isn't doing anything, because it will wait for that open network connection to timeout. Ideally calling [stop() should abort the current poll request](https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/clients/client-sqs/interfaces/abortcontroller.html), which would prevent us from receiving further messages, and would clean up the requests and let the process exit gracefully. And @rogerweb: > In regard to the stop behavior, I have the same understanding as @jwalton and the same pain. I'm able to perform a graceful shutdown within a second for all my dependencies except by the SQS consumer as it is waiting the 20 seconds in the worst case scenario. The way I see it, the pending request to SQS is a sqs-consumer implementation concern, while the exposed functionality would be "please shutdown ASAP, I'm not expecting any new messages, no matter what you are doing internally". If an abort() fits this use case better, I would be very happy with it too.

Solution

In order to support existing implementations, sqs-consumer should provide new functionality that also aborts existing requests when .stop() is called.

The suggested solution would be to add an option to the stop function that will allow users to tell us to abort any requests to AWS SQS, this would be implemented like so:

consumer.stop({abort: true})

To support backwards compatibility, this option would default to false.

If the option is true, we should implement and use the AbortController to abort SQS requests as discussed here:

https://aws.amazon.com/blogs/developer/abortcontroller-in-modular-aws-sdk-for-javascript/

More documentation is also available here:

https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/clients/client-sqs/interfaces/abortcontroller.html

I don't know if we will use the aws-sdk for this though, as it would require an additional dependency that people would have to add. We can potentially use the Node implementation instead:

https://nodejs.org/api/globals.html#globals_class_abortcontroller

nicholasgriffintn commented 1 year ago

The PR is still in for this and it is ready to go, if we drop Node 14 support or add a polyfill to this library.

At the moment, I'm leaning towards dropping support for Node 14, the security support for that version will end on 30 Apr 2023 so there are other reasons that we should do this, and this would be more valuable than spending time and future resource on a Polyfill.

I've raised an issue for this here: https://github.com/bbc/sqs-consumer/issues/362