Surface Errors from UnitOfWork actions instead of hanging silently#4697
Merged
Conversation
fd7d05f to
320e380
Compare
…ging silently UnitOfWork#safe only caught Exception, so any Error (e.g. NoClassDefFoundError) thrown synchronously by an invocation action escaped uncaught. The dependent `result` future was then never completed, and because executeWithResult joined the commit and result futures with thenCombine - which only fires once *both* complete - the Unit of Work hung forever instead of failing. In practice this turns a hard, fail-fast classpath/dependency problem into a silent startup deadlock with no log, no stack trace and no exception: the only observable symptom is that a downstream lifecycle bean (e.g. the web server) never starts. Changes: - safe() now catches Throwable and logs Errors before completing the future exceptionally, so these severe-but-easily-missed problems are visible and propagate to the caller instead of being swallowed. - executeWithResult() uses thenCompose rather than thenCombine. thenCombine only fires once *both* the commit and result futures complete, so a failed commit would hang forever whenever `result` never settles - which happens whenever the INVOCATION phase does not run to completion (an action throwing an Error, or the commit failing in an earlier phase such as begin-transaction). thenCompose propagates a failed commit immediately; on a successful commit the invocation phase has already run, so `result` is complete and its value (or failure) is returned, leaving the happy path byte-for-byte unchanged.
320e380 to
72cdb67
Compare
Adds two regression tests for executeWithResult: - errorsThrownInInvocationAreReturnedInFutureInsteadOfHangingTheUnitOfWork: an Error (e.g. NoClassDefFoundError) thrown by the invocation must fail the returned future instead of leaving it pending forever. - executeWithResultFailsFastWhenCommitFailsBeforeInvocationRuns: when the commit fails before the INVOCATION phase runs, the result future never settles, so the returned future must fail fast (thenCompose) rather than block (thenCombine). Both fail against the pre-fix code (the future never completes) and pass with it.
smcvb
requested changes
Jul 1, 2026
smcvb
left a comment
Contributor
There was a problem hiding this comment.
Good catch @MateuszNaKodach! But I got some nitty comments. Given the gravity of the Unit of Work, I'll request changes, even though they're small.
Errors from UnitOfWork actions instead of hanging silently
Apply reviewer suggestions from PR #4697: - log escaped Throwables with an `instanceof Error` check, aligning the condition with the javadoc and log message that speak only of Error - name the discarded `thenCompose` result `ignored` for clarity - reword test comments to describe the observable contract rather than internal mechanics (thenCombine/#safe/'result' future) - assert on the failed future with AssertJ `hasRootCauseInstanceOf`, dropping the hand-rolled unwrap helper
smcvb
approved these changes
Jul 1, 2026
smcvb
left a comment
Contributor
There was a problem hiding this comment.
My concerns have been addressed, hence I'm approving this pull request.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
UnitOfWorksilently swallowsErrors thrown by an invocation action and, as a result, hangs forever instead of failing. A single missing class on the classpath turned into a startup deadlock with no log line, no stack trace, and no exception anywhere — one of the hardest possible things to debug.This PR makes the Unit of Work surface such failures (log + fail-fast) instead of hanging.
Why this matters
I personally lost several hours on this. Not because the problem was hard — once the underlying
Erroris in the logs, the cause is obvious and the fix takes a minute — but because nothing was logged at all. The app just hung with "connection refused," and I had to instrument the framework itself to find out anErrorhad been thrown and discarded.That is exactly the kind of black hole we should protect our customers from. A missing/incompatible class on the classpath is one of the most common real-world failure modes, and it is precisely the situation where a clear log line saves hours. Right now the framework hides it. With this change the same situation produces an immediate, logged failure with the real stack trace — minutes, not hours.
How it showed up (a real, very hard-to-debug case)
My app simply would not become reachable on startup.
curlto its HTTP port returnedConnection refused.The single most telling detail: the last application log line was the event processor announcing it was starting — and then nothing:
That was it. No next processor, no
Tomcat started, noStarted …Application, no stack trace, no error — the log just stopped mid-startup. Concretely, there was:APPLICATION FAILED TO START,Tomcat initialized with port 8080… but neverTomcat startedand neverStarted …Application,That stack tells you a lifecycle bean's start future never completes — but gives zero indication of why. The web server is bound in a later
SmartLifecyclephase than the event processors, so aPooledStreamingEventProcessorwhose start future never completes blocks the whole context from finishing startup, and the port is never bound. The only symptom the developer sees is "connection refused."It took instrumenting the framework (logging each Unit-of-Work phase transition and the state of the invocation future) to discover that an
Errorhad been thrown and lost. Once surfaced, the cause was obvious in seconds. That gap between "impossible to find" and "obvious once logged" is exactly what this PR closes.My setup
0f4742d(a DDD/event-sourcing sample on Axon 5)5.2.0-SNAPSHOT, Axon Server2026.0.0(DCB), JPATokenStore, Spring Boot3.5.xaxon-conversionuses Jackson 3 (tools.jackson:jackson-databind:3.2.0), while the Jackson 2.x line (jackson-databind:2.21.2) transitively pinnedjackson-annotations:2.21.com.fasterxml.jackson.annotation.JsonApplyView, which only exists injackson-annotations:2.22. So at runtime, the first token-store serialization threwNoClassDefFoundError: com/fasterxml/jackson/annotation/JsonApplyView.The dependency mismatch was my problem to fix (bump
jackson-annotationsto 2.22). But the framework should have failed loudly on it rather than hanging. Again — this could be any missing/incompatible class from any dependency; the framework behavior is what this PR addresses.Root cause
The startup path is:
Inside
UnitOfWork:A
NoClassDefFoundErroris anError, so it is not caught here. It escapessafe(...)before.whenComplete(alsoComplete(result))is ever attached, so theresultfuture is never completed (neither normally nor exceptionally).Then:
thenCombineonly fires when bothexecute()andresultcomplete.execute()(the commit) does complete — exceptionally — butresultnever does, so the combined future hangs forever. The processor's start future never settles, and the application deadlocks during startup.So two things conspired: (1) the
Errorwas swallowed/lost, and (2) the future combinator waited on a future that could never complete.The fix
1. Don't swallow
Errors; log them.safe()now catchesThrowable, completes the future exceptionally (so it propagates to the caller), and logsErrors loudly — because they indicate severe, easily-missed problems. Ordinary businessExceptions are intentionally not logged here (hot path; the caller already observes them via the failed future).2. Fail fast on commit failure.
executeWithResultusesthenComposeinstead ofthenCombine, so a failed commit propagates immediately rather than blocking on aresultfuture that may never settle. On a successful commit the invocation phase has already run, soresultis complete and its value/failure is returned as before.Why
thenComposeis the right combinator here (not just defense-in-depth)thenCombineis meant for two independent futures running in parallel: it completes once both finish. Butexecute()andresultare not independent —resultis a byproduct ofexecute().execute()(=commit()) drives the phases, andresultis completed mid-way, during the INVOCATION phase, viasafe(action).whenComplete(alsoComplete(result)). There is a single pipeline (the commit); nothing runs concurrently. SothenCombinehere just means "wait for the later of the two (normallyexecute()) and yieldresult" — which is precisely whatthenCompose(ignored -> result)expresses, but faithfully.thenComposepreserves both original guarantees:execute()completes), andresult, settled by then).…and it is strictly more correct, because
resultis only ever completed inside the INVOCATION phase. Soresultnever settles whenever that phase doesn't run to completion — andthenCombinethen hangs forever. There are two such cases:Error, soresultis orphaned. (Also addressed by fix Create 5 minute introduction to CQRS / Axon #1.)PRE_INVOCATION/ begin-transaction throws). The INVOCATION action never runs,resultis never completed, andthenCombinewould deadlock again even though the error is now caught.thenComposeshort-circuits onexecute()'s failure in both cases, so the Unit of Work fails fast regardless of where in the commit the failure occurs. The happy path is byte-for-byte equivalent.Before / after
resultnever completesERROR; future completes exceptionallyexecuteWithResultfutureAPPLICATION FAILED TO STARTwith the real stack traceNotes for reviewers
Throwablemeans severeErrors (e.g.OutOfMemoryError) also become failed futures rather than propagating on the executing thread. That is the intended behavior for this boundary: the action's outcome must be observable via the returned future, and anErrorlost on an arbitrary worker/callback thread is strictly worse than one surfaced to the caller.thenCombine→thenComposechange is behavioral only when the commit fails (it no longer blocks on aresultthat may never settle — including when the commit fails before the INVOCATION phase runs); the happy path is byte-for-byte unchanged. See "WhythenComposeis the right combinator here" above.NoClassDefFoundErrorstartup failure instead of a hang. Happy-path startup is unaffected.