gnodet-bot commented on code in PR #27563:
URL: https://github.com/apache/camel/pull/27563#discussion_r4218669718
##########
dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/JavaRouteReader.java:
##########
@@ -150,10 +151,20 @@ private void route(RouteDefinition r,
RouteTemplateDefinition template, boolean
for (ProcessorDefinition<?> p : r.getOutputs()) {
decisions(p, scope, decisions, visited, 0);
}
- routes.add(new Route(
+ Route route = new Route(
id, template != null ? "routeTemplate" : "route", description,
r.getGroup(), file, Math.max(1, line),
"java", partial, from, consumes, produces,
r.getOutputs().size(), -1, 0, null, null,
- logOnly(r, produces), note, decisions));
+ logOnly(r, produces), note, decisions);
+ // the route's own error handler, such as
errorHandler(noErrorHandler())
+ if (r.isErrorHandlerFactorySet()) {
+ ErrorHandlerFactory eh = r.getErrorHandlerFactory();
+ if (eh instanceof NoErrorHandlerDefinition) {
+ route = route.withErrorHandler("noErrorHandler");
+ } else if (eh instanceof DeadLetterChannelDefinition) {
+ route = route.withErrorHandler("deadLetterChannel");
+ }
Review Comment:
⚠️ **Missing else branch — `defaultErrorHandler()` and ref-based handlers
are silently ignored**
The current code only handles two subtypes of `ErrorHandlerFactory`. If a
route overrides with `errorHandler(defaultErrorHandler())` or
`errorHandlerRef("myHandler")`, the factory is set but the branch falls through
— leaving `route.errorHandler` as `null`, making it look like the route
inherits the global handler.
This means a route that explicitly resets to the default error handler
(overriding a global DLC, for example) will be incorrectly reported in the
overview.
Suggested fix:
```suggestion
// the route's own error handler, such as
errorHandler(noErrorHandler())
if (r.isErrorHandlerFactorySet()) {
ErrorHandlerFactory eh = r.getErrorHandlerFactory();
if (eh instanceof NoErrorHandlerDefinition) {
route = route.withErrorHandler("noErrorHandler");
} else if (eh instanceof DeadLetterChannelDefinition) {
route = route.withErrorHandler("deadLetterChannel");
} else {
route = route.withErrorHandler("defaultErrorHandler");
}
}
```
The `else` branch catches `DefaultErrorHandlerDefinition` and any ref-based
handler (both of which are semantically equivalent to "using the default" in
terms of the overview display).
--
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]