[Bug]: abort: true raises errors when AWS SQS is polling for messages

[ypicard]

When calling consumer.stop({abort: true }), the following uncaught exception gets thrown:

.../node_modules/sqs-consumer/dist/errors.js:40
    const sqsError = new SQSError(message);
                     ^
SQSError: SQS receive message failed: Request aborted
    at toSQSError (.../node_modules/sqs-consumer/dist/errors.js:40:22)
    at Consumer.receiveMessage (.../node_modules/sqs-consumer/dist/consumer.js:159:43) {
  code: 'AbortError',
  statusCode: undefined,
  retryable: undefined,
  service: undefined,
  fault: undefined,
  time: 2023-04-18T17:41:50.041Z
}

Is this the expected behavior? Should it be caught in this library?

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

const uploadObject = async (file) => {
  ...
  const client = new S3Client(clientParams);
  try {
    await client.send(new PutObjectCommand(commandParams), { abortSignal });
  } catch(e) {
    if (e.name === "AbortError") {
      uploadProgress.textContent = 'Upload aborted: ' + e.message;
    }
    ...
  }
}

Package version

7.0.2

[nicholasgriffintn]

If any error happens within receive message then it is thrown yeah: https://github.com/bbc/sqs-consumer/blob/main/src/consumer.ts#L258

I would say it's for your app to handle those errors, I don't think we should prescribe if your app does this or that in terms of a response to this. Handling it within the library would simply hide it away, when really, you probably want to see if this has happened as it means an in flight request essentially failed.

[ypicard]

I partially agree with this. What about the error and aborted events, shouldn't be this the centralized place on where to handle such occurences? What is the purpose of the aborted event if we throw an error no matter what?

On top of that, the only way of catching this error is to do the following from what I understand:

 process.on('uncaughtException', (err) => {
        if ('code' in err && err.code === 'AbortError') {
          this.logger.info('SQS listener stopped while polling for messages');
          return;
        }
        throw err;
      });

It really doesn't seem right that using a library brings to writing code like this. What do you think? How do you suggest those exceptions be handled?

[ypicard]

After some additional testing, I am convinced there is another issue, perhaps not related, with the singleton instance created for the AbortController.

When creating 2 consumers, polling the same SQS queue, and calling .stop({abort:true}), only one consumer gets stopped and aborts its polling to SQS.

By adding a private abortController member to each consumer instance fixes this from what I have observed in some quick testing.

[nicholasgriffintn]

Yeah I'm just not sure if the library hiding errors is the best way as it requires users to listen to the emitted events in order to see them.

In terms of the second thing, I would say that's an entirely different thing but it's likely because you are reusing a consumer instantiation so yeah, the abort controller wouldn't be re assigned. Only way around that would be to allow users to provide their own abort controllers or to always create a new instance of the consumer.

[ypicard]

I don't think this actually hides errors. There is a clear path for the user to subscribe to all errors from this library, and I'd say I would not expect other errors to actually be thrown... What is the purpose of this error event if not this? If you could explain the difference here, I think it would help me understand :)

For number 2, I do not think I am re-using a consumer instantiation: I am instantiating 2 distinct consumers, polling the same queue. Is this an expected behavior? Am I misunderstand the underlying concepts here?

Thank you!

[nicholasgriffintn]

Sorry but I'm not going to go back and forth on this, if you'd want a discussion on it head over to the discussions tab, for now, again, I don't believe there's anything we need to do here and you should handle it in your application.

In terms of the second thing, that's a different thing, needs a different issue with a reproduction.

[github-actions]

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.