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

heartbeatInterval does not increment the visibilityTimeout value #313

Closed shayanc1985 closed 1 year ago

shayanc1985 commented 1 year ago

Describe the bug While using the visibilityTimeout, we see that the visibilityTimeout value does not get incremented with the value specified while creating the Consumer object.

To Reproduce Steps to reproduce the behaviour:

  1. set heartbeatInterval to 25
  2. set visibilityTimeout to 30
  3. When the startHeartbeat gets executed the first time, the 30s does not get added to 30s visibilityTimeout when the message was originally received.

Expected behaviour visibilityTimeout should be added to the timeout value being sent to the SQS.changeMessageVisibility()method

alexbaileyuk commented 1 year ago

I may be wrong but I don't think it should be added at all.

The code is:

...

  private async changeVisabilityTimeoutBatch(messages: SQSMessage[], timeout: number): Promise<PromiseResult<any, AWSError>> {
    const params = {
      QueueUrl: this.queueUrl,
      Entries: messages.map((message) => ({
        Id: message.MessageId,
        ReceiptHandle: message.ReceiptHandle,
        VisibilityTimeout: timeout
      }))
    };
    try {
      return this.sqs
        .changeMessageVisibilityBatch(params)
        .promise();
    } catch (err) {
      this.emit('error', err, messages);
    }
  }

...

this.changeVisabilityTimeoutBatch(messages, this.visibilityTimeout);

...

According to the AWS docs (https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-visibility-timeout.html#changing-message-visibility-timeout), this timeout will be set as a new 30 second timeout.

"For example, if the default timeout for a queue is 60 seconds, 15 seconds have elapsed since you received the message, and you send a ChangeMessageVisibility call with VisibilityTimeout set to 10 seconds, the 10 seconds begin to count from the time that you make the ChangeMessageVisibility call."

The main infor here is that the new visibility will "begin to count from the time that you make the ChangeMessageVisibility call"

In your example, after 25 seconds, the message will be given an additional 30 seconds to be processed. After another 25 seconds, that would be an additional 30 seconds.

nicholasgriffintn commented 1 year ago

I don't think it should either, from the AWS docs that were linked, it seems as though we have the correct behavior here.