allthingssecurity opened a new pull request, #29785:
URL: https://github.com/apache/camel/pull/29785

   # Description
   
   [CAMEL-26881](https://issues.apache.org/jira/browse/CAMEL-26881)
   
   Follow-up of #29525 (CAMEL-26741, an `Error` of a processor fails the 
exchange): another place that catches only `Exception`, found while going 
through the follow-ups of that change.
   
   The completion timeout checker of the Aggregate EIP runs on 
`DefaultTimeoutMap`: a task scheduled with `scheduleWithFixedDelay` purges the 
expired entries and calls the listener of the aggregator 
(`AggregationTimeoutMap.onEviction`), which completes the group and calls 
`AggregationStrategy.timeout` and `onCompletion`. Three places caught only 
`Exception`: `onEviction`, `DefaultTimeoutMap.emitEvent` and the purge task. So 
an `Error` that is not a `VirtualMachineError` (such as an `AssertionError` or 
a `NoClassDefFoundError` of the strategy) escaped the purge task, and the 
executor did not run the task again (`scheduleWithFixedDelay` keeps the `Error` 
in the `ScheduledFuture`, which nothing reads, so nothing was logged): no group 
of that aggregator completed by its timeout again until the route was 
restarted; the other entries that expired in the same purge were already 
removed from the timeout map and were not completed either.
   
   Change:
   - `DefaultTimeoutMap` (camel-support): the purge task catches `Throwable`, 
and `emitEvent` also catches `Error`, where they caught only `Exception` (a 
`VirtualMachineError` is rethrown, and still ends the purge task, as before), 
so the other listeners and entries, and the next purges, go on; an `Error` of a 
listener is logged at WARN (an exception of a listener stays ignored silently, 
as before). This also covers the callers of `put`, `putIfAbsent`, `remove` and 
`drainAndEvictAll`, and the other users of `DefaultTimeoutMap` (the correlation 
timeouts of camel-jms and camel-sjms, the reply timeouts of camel-rocketmq, the 
correlation manager of camel-netty), none of which relies on an `Error` 
propagating;
   - `AggregationTimeoutMap.onEviction` (camel-core-processor): an `Error` is 
handled as an exception is since CAMEL-25709 (logged at WARN once, and tried 
again);
   - the completion interval task (`AggregationIntervalTask`, scheduled with 
`scheduleAtFixedRate`) and the recover task of the Aggregate EIP, which an 
`Error` (such as of `onCompletion`, which `onSubmitCompletion` throws on) 
stopped for good in the same way: they catch `Throwable` where they caught 
`Exception` (a `VirtualMachineError` is rethrown), and the recover task goes on 
with the other exchanges after an `Error` of one, as after an exception.
   
   Not changed: a group whose `timeout` throws is handled as for an exception 
today: it was removed from the repository before `timeout` is called, so with a 
repository that is not recoverable it is not delivered, and with a 
`RecoverableAggregationRepository` the recover task delivers it later.
   
   Sibling, for a follow-up JIRA that I will file and link here: 
`TimerListenerManager` (`scheduleAtFixedRate`) also catches only `Exception`, 
so an `Error` of a timer listener stops all of them.
   
   Tests:
   - new `DefaultTimeoutMapListenerThrowsErrorTest`: a listener throws a 
`NoClassDefFoundError` for one of two entries that (usually) expire in the same 
purge: the other is evicted too, and an entry added later is evicted as well;
   - new `AggregateTimeoutStrategyThrowsErrorTest`: the `timeout` of the 
strategy throws a `NoClassDefFoundError` for the first group, and a second 
group still completes by its completion timeout;
   - `AggregateTimeoutRepositoryExceptionTest` (CAMEL-25709): + 
`testTimeoutCompletesGroupAfterFailedGetWithError`, the read of a group by the 
timeout checker throws a `NoClassDefFoundError` once, and the group is tried 
again and completes, as for an exception (this one needs the `onEviction` 
change: with only the `DefaultTimeoutMap` change nothing tries the group again);
   - `AggregateIntervalTaskExceptionTest`: + `testIntervalTaskSurvivesError`, 
the repository throws a `NoClassDefFoundError` at the first interval tick, and 
a later tick completes the group. The recover task change has no test of its 
own (the same catch as the interval task).
   
   Without the change the four new tests fail on main (the purge task, or the 
interval task, does not run again: nothing completes by timeout or interval any 
more, and the group of the failed read is not tried again); the existing tests 
of these classes pass. With it the aggregator and timeout map tests pass (192 
run, 0 failures, 2 skipped), and the camel-core tests pass (9197 run, 0 
failures, 1 error in `ValidatorExternalResourceTest`, which needs the network, 
45 skipped).
   
   No documentation or upgrade guide change: nothing changes for an exception 
or a `VirtualMachineError`, and for another `Error` the only difference is that 
it is logged and the completion timeouts go on, where before they stopped 
without a log line.
   
   # Target
   
   - [x] I checked that the commit is targeting the correct branch (Camel 4 
uses the `main` branch)
   
   # Tracking
   - [x] If this is a large change, bug fix, or code improvement, I checked 
there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for 
the change (usually before you start working on it): 
[CAMEL-26881](https://issues.apache.org/jira/browse/CAMEL-26881).
   
   # Apache Camel coding standards and style
   
   - [x] I checked that each commit in the pull request has a meaningful 
subject line and body.
   - [ ] I have run `mvn clean install -DskipTests` locally from root folder 
and I have committed all auto-generated changes.
     (I built the core modules with the formatter and import-sort plugins and 
ran the camel-core tests. I did not run the full root build.)
   
   # AI-assisted contributions
   
   - [x] If this PR includes AI-generated code, commits have proper 
co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR 
description identifies the AI tool used.
     This PR was prepared with Claude Code (Claude Opus 5.5). The commit 
carries a `Co-Authored-By` trailer.
   
   _Claude Code on behalf of allthingssecurity_
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to