-
Notifications
You must be signed in to change notification settings - Fork 340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Bug]: abort: true
raises errors when AWS SQS is polling for messages
#389
Comments
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. |
I partially agree with this. What about the 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? |
After some additional testing, I am convinced there is another issue, perhaps not related, with the singleton instance created for the When creating 2 consumers, polling the same SQS queue, and calling By adding a private |
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. |
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 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! |
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. |
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. |
When calling
consumer.stop({abort: true })
, the following uncaught exception gets thrown: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/:
Package version
7.0.2
The text was updated successfully, but these errors were encountered: