-
Notifications
You must be signed in to change notification settings - Fork 8
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
Fix consumer #62
Fix consumer #62
Conversation
ddf9c43
to
92a572d
Compare
Refactor it!This comment was generated by todo based on a
|
8b57d9b
to
d978cf2
Compare
} catch (e: Exception) { | ||
logger.error { "Can't send nack for deliveryTag: $deliveryTag" } | ||
} finally { | ||
throw RuntimeException(errorMessage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need some specific exception here for user to be able to implement some recover plan.
try { | ||
AMQPChannel.basicAck(deliveryTag, false) | ||
} catch (e: Exception) { | ||
val errorMessage = "Can't ack a message with deliveryTag: $deliveryTag" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it truth that if we've got exception then it's ack? What about ACK + IO exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use try-catch only for basicAck. A user is responsible for handling exceptions which can be thrown from the handler.
} catch (e: Exception) { | ||
val errorMessage = "Can't ack a message with deliveryTag: $deliveryTag" | ||
logger.error { errorMessage } | ||
throw RuntimeException(errorMessage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should throw some concrete exception here for user to be able to recover from error somehow.
ba263c3
to
7aa85d4
Compare
7aa85d4
to
22fdeae
Compare
Do we need coroutineScope here?This comment was generated by todo based on a
|
closes: #39
closes: #38