Fail fast when a model processor prerequisite is missing - #4284
Open
vogella wants to merge 2 commits into
Open
Conversation
Contributor
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
BindingToModelProcessor logged that CommandManager or ContextManager was null and then passed the nulls to the BindingManager constructor, which rejects them. The resulting NPE was swallowed by ModelAssembler and only surfaced much later as an InjectionException on BindingService, hiding the real cause. It now throws with a message naming the processor that should have provided the missing value, and reads the two @reference fields that establish the ordering, so they can no longer be dropped as unused. ModelAssembler logs a failing processor as an error and names it, instead of a warning without the class.
vogella
force-pushed
the
binding-processor-fail-fast
branch
from
August 26, 2026 22:18
07554a2 to
4f7e866
Compare
Contributor
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.
BindingToModelProcessorlogged thatCommandManagerorContextManagerwas null and then handed those nulls to theBindingManagerconstructor, which rejects them.ModelAssemblerswallowed the resulting NPE, soBindingManagernever reached the application context and the failure only surfaced much later as anInjectionExceptiononBindingServiceduring workbench startup, several frames away from the real cause and naming none of the three processors involved.The processor now fails immediately with a message naming the processor that should have supplied the missing value, and
ModelAssemblerreports a failing processor as an error including its class name rather than a warning that says only "Could not run processor". The two@Referencefields that order this processor after its two siblings are now read inprocess(), so an unused-field cleanup can no longer drop them and silently break the ordering that has carried this since the DS migration in #2402.This came out of a real investigation: removing those two annotations is invisible to javac, to the Java builder, to CI and to every headless test, yet it leaves a dead workbench with nothing in the log connecting the symptom to the cause. Failing fast turns that into a single readable log entry.