gnodet-bot commented on code in PR #29388:
URL: https://github.com/apache/camel/pull/29388#discussion_r4503521257
##########
components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosTargets.java:
##########
@@ -71,11 +73,70 @@ static String check(CamelContext camelContext, ChaosRule
rule) {
if (rule.lifecycle != null) {
return null;
}
- return switch (rule.target) {
+ String answer = switch (rule.target) {
case ENDPOINT, COMPONENT -> checkEndpoints(camelContext, rule);
case ROUTE -> checkRoutes(camelContext, rule);
case STEP -> checkSteps(camelContext, rule);
};
+ return answer == null && rule.in != null ? checkIn(camelContext, rule)
: answer;
+ }
+
+ /**
+ * Why a rule with {@code in=} finds nothing to fire at in its routes: no
route matches {@code in=}, or its target
+ * matches nothing inside those routes (a send, a step, or the poll of the
consumer of the route). A send inside a
+ * route that one of them calls ({@code direct:}) is in that other route.
+ */
+ private static String checkIn(CamelContext camelContext, ChaosRule rule) {
+ List<Route> routes = new ArrayList<>();
+ TreeSet<String> ids = new TreeSet<>();
+ for (Route route : camelContext.getRoutes()) {
+ ids.add(route.getRouteId());
+ if (rule.matchesIn(route.getRouteId())) {
+ routes.add(route);
+ }
+ }
+ if (routes.isEmpty()) {
+ return "in=" + rule.in + " matches no route of the context now, so
the rule fires 0 times; the routes are: "
+ + some(ids);
+ }
+ String in = (routes.size() == 1 ? "route " : "routes ") + rule.in + "
(in=" + rule.in + ")";
+ if (rule.target == ChaosRule.Target.STEP) {
+ TreeSet<String> steps = new TreeSet<>();
+ for (Route route : routes) {
+ for (Processor processor : route.filter("*")) {
+ String id = processor instanceof IdAware idAware ?
idAware.getId() : null;
+ if (id != null) {
+ if (rule.matchesId(id)) {
+ return null;
+ }
+ steps.add(id);
+ }
+ }
+ }
+ return "step=" + rule.pattern + " matches no step of " + in + ",
so the rule fires 0 times; the steps there"
+ + " are: " + some(steps);
+ }
+ if (rule.hooks.contains(Hook.DLQ)) {
+ // the dead letter channel is the error handler's, not a send of
the route
+ return null;
+ }
+ Reach reach;
+ try {
+ reach = reach(camelContext, routes);
+ } catch (RuntimeException e) {
Review Comment:
🔍 **Overly broad exception catch:** `catch (RuntimeException e)` in
`checkIn()` will swallow `NullPointerException`, `ClassCastException`, and
other programming errors from the new per-route `RouteReach` traversal,
alongside the intended `ConcurrentModificationException` from route reload. The
top-level `reach()` method now has a `LOG.debug` on the same catch —
`checkIn()`'s catch has no such logging, making bugs here invisible.
Consider narrowing to `ConcurrentModificationException`, or at minimum
logging at debug level when the exception is not a
`ConcurrentModificationException`:
```suggestion
} catch (ConcurrentModificationException e) {
// the routes changed while they were read (such as a reload)
return null;
}
```
--
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]
