gnodet-bot commented on code in PR #29388:
URL: https://github.com/apache/camel/pull/29388#discussion_r4503539304
##########
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:
🔍 **Still open — overly broad exception catch without logging:** The `catch
(RuntimeException e)` in `checkIn()` has no `LOG.debug`, while the companion
`catch (RuntimeException e)` in `reach()` (from the base branch) now logs via
`LOG.debug("Cannot read what the routes reach...")`. A `NullPointerException`
or `ClassCastException` from the `RouteReach` traversal will be silently
swallowed here, making bugs invisible.
```suggestion
} catch (ConcurrentModificationException e) {
// the routes changed while they were read (such as a reload)
return null;
}
```
Alternatively, if `RuntimeException` is intentionally broad, add the same
`LOG.debug` as `reach()`:
```
} catch (RuntimeException e) {
LOG.debug("Cannot determine what {} reaches for the chaos report
(the routes may have changed): {}",
rule.in, e.getMessage(), e);
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]
