gnodet-bot commented on code in PR #29387:
URL: https://github.com/apache/camel/pull/29387#discussion_r4503202649
##########
components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosTargets.java:
##########
@@ -89,6 +106,167 @@ private static String checkEndpoints(CamelContext
camelContext, ChaosRule rule)
return null;
}
+ /**
+ * The hooks of the rule that can fire in the context now, for the list
and the report: of the hooks of an
+ * {@code endpoint=} or {@code component=} rule, {@code poll} only when a
route that consumes a matching endpoint
+ * polls on a schedule ({@link ScheduledPollConsumer}), and {@code send}
(and {@code reply}, the reply of a send)
+ * only when something sends to a matching endpoint. A hook where the rule
fired already can fire, whatever the
+ * context has now. The other hooks, and the hooks of the other targets,
are those of the rule.
+ */
+ static Set<Hook> hooksThatCanFire(Reach reach, ChaosRule rule) {
+ if (rule.target != ChaosRule.Target.ENDPOINT && rule.target !=
ChaosRule.Target.COMPONENT
+ || rule.lifecycle != null || reach == null) {
+ return rule.hooks;
+ }
+ Set<Hook> answer = EnumSet.noneOf(Hook.class);
+ Boolean sent = null;
+ for (Hook hook : rule.hooks) {
+ boolean can = switch (hook) {
+ case POLL -> reach.polled.stream().anyMatch(rule::matchesUri);
+ case SEND, REPLY -> {
+ if (sent == null) {
+ sent = reach.sentTo.stream().anyMatch(rule::matchesUri)
+ || reach.dynamic.stream().anyMatch(uri ->
mayMatch(rule, uri));
+ }
+ yield sent;
+ }
+ default -> true;
+ };
+ if (can || rule.firedAt(hook) > 0) {
+ answer.add(hook);
+ }
+ }
+ return answer;
+ }
+
+ /**
+ * What the routes of the context reach now, read once for all the rules
of a report: the URIs of the endpoints
+ * something sends to, the URIs of the sends that are known only when they
send (toD, wireTap, enrich,
+ * recipientList, routingSlip, dynamicRouter), and the URIs of the
endpoints that a route consumes with a scheduled
+ * poll. Null before the routes are there (the context did not start yet),
as then nothing is known.
+ */
+ static Reach reach(CamelContext camelContext) {
+ List<Route> routes = camelContext.getRoutes();
+ if (routes.isEmpty() && !camelContext.isStarted()) {
+ return null;
+ }
+ try {
+ return reach(camelContext, routes);
+ } catch (RuntimeException e) {
+ // the routes changed while they were read (such as a reload): the
hooks of the rules, as before the start
+ return null;
Review Comment:
⚠️ **Silent exception swallowing — no logging**
`ChaosTargets` has no `Logger`, so this `catch (RuntimeException e)` block
discards exceptions with zero trace. The comment says "routes changed while
they were read", but the catch also covers NPEs in the processor walk,
`ClassCastException` from model nodes, or any other programming error in
`reach(CamelContext, List<Route>)`.
The fallback (`return null`) is safe for display purposes, but bugs here
will be invisible in production. At minimum, add a `LOG.debug(...)` so
operators can distinguish "concurrent reload" from "bug in route introspection":
```java
private static final Logger LOG =
LoggerFactory.getLogger(ChaosTargets.class);
```
Then in the catch:
```suggestion
} catch (RuntimeException e) {
// the routes changed while they were read (such as a reload):
the hooks of the rules, as before the start
LOG.debug("Route introspection failed while computing hooks
(concurrent reload?): {}", e.toString());
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]
