davsclaus commented on code in PR #27985:
URL: https://github.com/apache/camel/pull/27985#discussion_r4301785893
##########
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:
Nit: the `catch (Error e)` branch just above still reads
`this.timeoutExtender` twice. If `doStop()` sets the field to null between the
check and the use, the handler throws an NPE that hides the original `Error`.
Using the local `extender` there too, like this new branch does, would make the
method consistent and close that gap.
##########
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:
Nit: removing the messages does not make them visible right away. It stops
extending them, so they become visible when their current (possibly just
extended) visibility timeout runs out. The PR description says this; the
comment could too:
```suggestion
// the messages that are not routed (the consumer is stopping or
suspending) are no longer extended, so they
// become visible again when their current visibility timeout runs
out
```
--
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]
