[#3062] Add exception handler support for command, event, and query handling components#4520
Conversation
Introduces MessageHandlingExceptionHandler and EventHandlingExceptionHandler interfaces along with ErrorHandlingEventHandlingComponent to let callers intercept exceptions thrown by event handlers - either suppressing them or propagating a different error. Exposes withExceptionHandler() on EventHandlingComponentsConfigurer to apply a handler across all registered components. Also fixes @ExceptionHandler resolution to carry the exception through ProcessingContext instead of a ThreadLocal, making it safe for async/reactive dispatch paths, and adds a null-safety guard for missing interceptor chains in MessageHandlerInterceptorDefinition. Key changes: - New MessageHandlingExceptionHandler / EventHandlingExceptionHandler interfaces - New ErrorHandlingEventHandlingComponent decorator - withExceptionHandler() on EventHandlingComponentsConfigurer - ResultParameterResolverFactory switched from ThreadLocal to ProcessingContext-based result injection - Null-guard in MessageHandlerInterceptorDefinition - Tests enabled and extended, docs updated
…ts (#3062) Introduces CommandHandlingExceptionHandler and QueryHandlingExceptionHandler interfaces, along with ErrorHandlingCommandHandlingComponent and ErrorHandlingQueryHandlingComponent decorators that wrap handling components to intercept handler exceptions. Exception handlers are registered via withExceptionHandler() on the CommandHandlerPhase and QueryHandlerPhase configuration interfaces, and multiple handlers chain in registration order.
abuijze
requested review from
MateuszNaKodach,
hatzlj and
smcvb
and removed request for
a team
…ler branch Resolves conflicts in CommandHandlingModule, QueryHandlingModule, and their Simple* implementations by combining both interceptor and exception handler support. Interceptors are applied first (inner layer), exception handlers outermost, so exception handlers catch errors from the full chain.
- Enable component-message-intercepting and annotated-exception-handling in nav - Replace annotated-exception-handling.adoc WARNING placeholder with real content - Replace "not yet supported" section in migration guide with migration examples for @CommandHandlerInterceptor, @EventHandlerInterceptor, @QueryHandlerInterceptor, and @ExceptionHandler
…re/3062-exception-handler-interceptors
|
Resolves #3062 |
abuijze
changed the title
smcvb
added
Type: Feature
- Enable exceptionHandlerAnnotatedMethodsAreSupportedForCommandHandling- Components test; rework to given/when/then style in AnnotatedInterceptor- Handling and verify the exception handler is invoked - Add exceptionHandlerWithChainParamIsRejected tests for command and query handler components (equivalent to the event handler test) - Fix the entity-constructor NOTE in component-message-intercepting.adoc to reflect that AF5 uses static creation handlers, not constructors - Fix CommandMessage<?> to CommandMessage (no generic) in the @ExceptionHandler migration guide prose
Remove the @ExceptionHandler test, documentation, and migration guide content that belongs in the @ExceptionHandler PR (#3062/#4520), not in the annotated interceptors PR (#3485/#4515). Also add missing @NullMarked package-info.java files for the three new annotation packages introduced for annotated interceptors.
…xception-handler-interceptors Preserve @ExceptionHandler documentation that was erroneously removed on the 3485 branch: - Restored annotated-exception-handling.adoc - Kept @ExceptionHandler section in component-message-intercepting.adoc - Kept @ExceptionHandler migration section in interceptors.adoc - Kept active nav entry for annotated-exception-handling - Kept null-safety guard in MessageHandlerInterceptorDefinition From 3485: move interceptor annotations to interception/annotation sub-packages, add package-info.java files, drop Message<?> generics, cleanup from review comments.
payloadType filtering is already achievable by declaring the payload as a method parameter, matching how @MessageHandlerInterceptor works.
Exception handlers are designed to deal with cross-cutting concerns and should therefore not rely on a specific payload. They can, however, take the entire message as a parameter, which also restricts them to handle only exceptions caused by that type of message.
smcvb
left a comment
smcvb
left a comment
There was a problem hiding this comment.
Bunch of naming concerns, wondering why we make new Message Handling Components for every registered exception handler (as this deviates from interceptor support, which it aligns with closely), but most importantly, I am wondering if we need a dedicated decorating MessageHandlingComponent for exception handling. Can't we have the declarative exception registration approach build on the declarative interception approach, as it is "just another interceptor" in the ned?> I'd wager that would limit custom code for the interceptors quite a bit.
| * @param exceptionHandler the exception handler to apply to all components | ||
| * @return this phase for further configuration | ||
| */ | ||
| default CompletePhase withExceptionHandler(EventHandlingExceptionHandler exceptionHandler) { |
There was a problem hiding this comment.
Can we only register one exception handler? That doesn't seem entirely correct to me. From an annotation perspective, users can also define several, dealing with different exceptions if needed. I believe the declarative approach currently misses that capability.
There was a problem hiding this comment.
You can register as many as you want. Just repeat the invocation
| * @param exceptionHandler the exception handler to apply to the query handling component | ||
| * @return this phase for further configuration | ||
| */ | ||
| QueryHandlerPhase withExceptionHandler(QueryHandlingExceptionHandler exceptionHandler); |
There was a problem hiding this comment.
The new method is a breaking change right now for implementers of this interface. I think we should have a default here.
There was a problem hiding this comment.
Aren't we the only implementers of this interface? Or put differently: do we ever expect anyone to implement this interface, and wouldn't a breaking API change then be better than a silent failure when a default implementation is unexpectedly invoked?
There was a problem hiding this comment.
I, indeed, don't expect users of Axon Framework to implement this. We just may see implementations in different repositories maintained by Axoniq, though.
We've already recently seen this between the Framework and Platform team. I similarly thought "what harm does it do to change some of these methods between 5.0 and 5.1." Well, I can tell you, the Platform team was harmed by that quite a bit.
If you feel strongly we need to make an exception here, I'll let it go. I mean, I get your POV. But I re-reverted my stance to "let's just not do this" after letting it slide between 5.0 and 5.1.
…ndling The declarative withExceptionHandler API on CommandHandlingModule, QueryHandlingModule, and EventHandlingComponentsConfigurer now delegates to the existing interceptor mechanism rather than introducing separate decorator component types. The method is defined as a default method on each interface, wrapping the exception handler as a MessageHandlerInterceptor via intercepted(). Parameters changed to ComponentBuilder<*ExceptionHandler> to align with the existing intercepted() API style. As a result, the ErrorHandling*HandlingComponent classes and the now-unused Delegating*HandlingComponent base classes are removed. The ordering semantics change accordingly: later-registered exception handlers are innermost (closest to the handler) and see exceptions first, consistent with how interceptor chains work. Also fixes the migration guide to note that the @ExceptionHandler payloadType attribute was removed in Axon Framework 5.
Fixes Vale Google.EmDash build error.
smcvb
changed the title
smcvb
left a comment
smcvb
left a comment
There was a problem hiding this comment.
Not approving nor requesting changes as I am slightly unsure whether my assumption is right that we cannot receive generic Message exception handlers in the declarative approach.
| * @return this phase for further configuration | ||
| */ | ||
| default CommandHandlerPhase withExceptionHandler( | ||
| ComponentBuilder<CommandHandlingExceptionHandler> handlerBuilder) { |
There was a problem hiding this comment.
Would this also allow us to register a generic exception handler that can deal with any message? I believe the CommandHandlingExceptionHandler now blocks us from doing this, as it expects a CommandMessage exactly. This is where the declarative approach now deviates from the annotated approach, wherein we can provide users the means to provide a generic exception handler.
Or, if I am wrong here, feel free to disregard my comment.
| * @param handlerBuilder builder for the exception handler to apply to all components | ||
| * @return this phase for further configuration | ||
| */ | ||
| default CompletePhase withExceptionHandler(ComponentBuilder<EventHandlingExceptionHandler> handlerBuilder) { |
There was a problem hiding this comment.
Similar "can we receive generic exception handlers?"-concern as placed on the command handling module.
| * @param handlerBuilder builder for the exception handler to apply to the query handling component | ||
| * @return this phase for further configuration | ||
| */ | ||
| default QueryHandlerPhase withExceptionHandler(ComponentBuilder<QueryHandlingExceptionHandler> handlerBuilder) { |
There was a problem hiding this comment.
Similar "can we receive generic exception handlers?"-concern as placed on the command handling module.
Relax the parameter on CommandHandlerPhase, CompletePhase, and QueryHandlerPhase withExceptionHandler methods to accept any MessageHandlingExceptionHandler typed at a supertype of the phase's message type. A single generic handler (e.g. a logging exception handler) can now be registered against any of the three phases without retyping per message kind. Existing typed-subinterface registrations continue to compile unchanged.
|
smcvb
left a comment
smcvb
left a comment
There was a problem hiding this comment.
My concerns have been addressed, hence I'm approving this pull request.
2 participants
Summary
CommandHandlingExceptionHandlerandQueryHandlingExceptionHandlerinterfaces for intercepting exceptions thrown by command/query handlersErrorHandlingCommandHandlingComponentandErrorHandlingQueryHandlingComponentdecorator implementations that wrap a delegate component and route failures through the registered exception handlersDelegatingCommandHandlingComponentandDelegatingQueryHandlingComponentbase classes for clean delegation patternswithExceptionHandler(...)onCommandHandlerPhaseandQueryHandlerPhaseconfiguration interfaces; multiple handlers chain in registration order (first registered sees the exception first)Notes
This PR depends on
feature/3485-annotated-handler-interceptorsand must be merged after it. It completes exception handler support across all three message types (event handling was added in the parent commit on this branch).