Automatically delay message visibility changes while processing a message

[mltsy]

Background

By default, SQS messages, once received, are hidden from additional consumers for 30 seconds and then become visible again (in case the consumer dies and cannot finish processing). This opens the possibility that a job that takes an unexpectedly long time to run might result in double-processing of the same message, if not properly handled.

The message-subscriber package offers a refreshInterval option according to its docs that simply delays the visibility change periodically, until the message handler returns or throws. This seems like a wise choice. If the handler takes longer than the visibility timeout, you won't accidentally double-process the message, but if the process dies for some reason, both the handler and the refreshing will stop, so the message will still be re-processed correctly.

Objectives

1. Periodically delay visibility changes for a received message until the message handler either returns or throws
2. Make the refresh interval configurable (perhaps default to 10 seconds?), and allow it to be disabled.

[nicholasgriffintn]

Hey, sorry if I am misunderstanding, but we have an option here that allows for the visibility to be extended: https://bbc.github.io/sqs-consumer/interfaces/ConsumerOptions.html#heartbeatInterval

Does this not resolve what you are talking about?

[mltsy]

Oh, yes it does! That's exactly it - thank you 🙏 I was missing that whole docs link (I didn't see that there were reference docs outside of the Readme). Sorry I missed that, and thanks for pointing me to them 😄

Though I still can't tell if there's a default heartbeatInterval, or if that's only enabled if I pass in the option... can you clarify?

[nicholasgriffintn]

Ahh that's maybe a good point, we should maybe link to those directly from the README, somewhere at the top.

I'll pop a ticket in for that. It doesn't currently default to anything no: https://github.com/bbc/sqs-consumer/blob/main/src/consumer.ts#L78, if we were to do that now, it would be a breaking change, so not sure if we would do that.

[mltsy]

Ah, Thank you so much! I think it's fine that it isn't enabled by default, I just wanted to verify.

It might be helpful to note that in the docs (what value indicates disabled, and that it is the default)