allthingssecurity commented on code in PR #27990:
URL: https://github.com/apache/camel/pull/27990#discussion_r4301873321
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java:
##########
@@ -266,10 +266,32 @@ public String toString() {
// process the exchange
processExchange(exchange);
}
+ // the remaining exchanges are not routed as the consumer is stopping:
their mails must be consumed by a
+ // later poll (or by another consumer sharing the idempotent
repository)
+ releaseNotRouted(CastUtils.cast(exchanges));
Review Comment:
Thanks, fixed in d699316e2543. `processBatch` now routes the batch in a
`try` and gives back the rest in a `finally`, so the keys of the exchanges that
are not routed are also removed when `processExchange` throws.
New test
`MailIdempotentRepositoryPollErrorTest.keysOfTheRestOfTheBatchAreRemovedWhenRoutingAnExchangeThrows`:
5 mails, routing the second throws a `RejectedExecutionException`. Afterwards
the repository holds only the keys of the two mails that went into routing.
Without the change, all 5 keys stay. camel-mail tests: 244 run, 0 failures.
_Claude Code on behalf of allthingssecurity_
##########
components/camel-mail/src/main/java/org/apache/camel/component/mail/MailConsumer.java:
##########
@@ -330,7 +352,15 @@ private Queue<Exchange> retrieveMessages() throws
MessagingException {
if (getEndpoint().getConfiguration().isPeek()) {
peekMessage(message);
}
- Exchange exchange = createExchange(new KeyValueHolder<>(key,
message));
+ Exchange exchange;
+ try {
+ exchange = createExchange(new KeyValueHolder<>(key,
message));
+ } catch (Exception e) {
Review Comment:
Thanks, fixed in d699316e2543. The `try`/`catch` in `retrieveMessages` now
covers the whole loop body. The key of the current mail is removed only once
`isValidMessage` has added it, so a failing `add(key)` removes only the keys of
the mails before it. I also made removing a key robust: a repository that also
fails in `remove` is logged at WARN, and the other mails are still given back,
and the failure of the poll is not hidden.
New test
`MailIdempotentRepositoryPollErrorTest.keysOfTheMailsRetrievedSoFarAreRemovedWhenAddingTheKeyOfALaterMailFails`:
the repository fails the third `add`, the poll fails, and the repository is
empty afterwards. Without the change, the first two keys stay.
_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]
