allthingssecurity commented on code in PR #27985:
URL: https://github.com/apache/camel/pull/27985#discussion_r4301872799
##########
components/camel-aws/camel-aws2-sqs/src/main/java/org/apache/camel/component/aws2/sqs/Sqs2Consumer.java:
##########
@@ -180,12 +186,26 @@ public String toString() {
timeoutExtender.entries.clear();
}
throw e;
+ } catch (RuntimeException e) {
Review Comment:
Fixed in c61f73733042: the `catch (Error e)` branch now uses the local
`extender` too, so a `doStop()` that clears the field in between can no longer
turn the original `Error` into an NPE. camel-aws2-sqs tests: 77 run, 0
failures. There is no test for this one, as it needs `doStop()` to run between
the check and the use.
_Claude Code on behalf of allthingssecurity_
##########
components/camel-aws/camel-aws2-sqs/src/main/java/org/apache/camel/component/aws2/sqs/Sqs2Consumer.java:
##########
@@ -180,12 +186,26 @@ public String toString() {
timeoutExtender.entries.clear();
}
throw e;
+ } catch (RuntimeException e) {
+ stopExtending(extender, exchanges);
+ throw e;
}
}
+ // the messages that are not routed (the consumer is stopping or
suspending) must become visible again
Review Comment:
Right, thanks. Fixed in c61f73733042. I used your wording for the comment,
and the javadoc of `TimeoutExtender.remove` now says the same: the message is
not made visible, it becomes visible again when its current (possibly just
extended) visibility timeout runs out.
_Claude Code on behalf of allthingssecurity_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
