Repository navigation
Construct the boundary aggregate's projection directly, and say when a tag type is lost (#5649) - #5657
Merged
Conversation
…a tag type is lost (#5649) Two hazards in how tag types reach the store, both invisible until Native AOT. EventGraph.Build<TDoc>() closed SingleStreamProjection<,> through CloseAndBuildAs for a [BoundaryAggregate] even though both type arguments are known where it stands -- TDoc from the caller, and string unconditionally per #324. ILC has no such instantiation, so every DCB boundary fetch threw MissingMethodException. Written out, the instantiation is rooted at compile time. autoDiscoverTagTypesFromProjections swallowed every exception through one bare catch. An identity that simply is not a strong-typed identifier should be passed over without a word, but the AOT failure of MakeGenericMethod was being passed over just as quietly, leaving a store running with the tag type absent and nothing anywhere saying so -- the queries that need it then fail far from the cause. It is still not rethrown: this runs inside the store's Initialize, and a throw would stop an application starting over a feature it may not use, which is the trade the CreateTagTypeMethod lookup above it already makes. Verified by running the native binary, not by a clean publish. The smoke harness gains a boundary-aggregate check, which fails on the unfixed code with the exception the issue reports: FAIL boundary aggregate aggregator source System.MissingMethodException: No parameterless constructor defined for type 'Marten.Events.Aggregation.SingleStreamProjection`2[Toegangsgrens,System.String]'. at JasperFx.Core.Reflection.TypeExtensions.CloseAndBuildAs[T](Type, Type[]) and passes with it, 24 of 24. Does not close #5649. RegisterTagType<TTag> still closes its own open generic, and that needs a reflection-free construction path in JasperFx.Events plus a new Marten overload -- discussed on the issue. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3
erdtsieck
pushed a commit
to erdtsieck/marten
that referenced
this pull request
Oct 19, 2026
… cannot work Supersedes JasperFx#5663, whose API half landed independently in JasperFx#5674. What is left is the part JasperFx#5674 did not carry: the diagnostics that point at the overload, the argument validation, and the native coverage. RegisterTagType<TTag>() reflects over the tag's members with ValueTypeInfo and closes TagTypeRegistration<TTag, TInner> with MakeGenericType. JasperFx#5674 annotated both inferring overloads RequiresUnreferencedCode/RequiresDynamicCode, but an analyzer warning only reaches an application that is itself AOT-analyzed -- a store configured from inside a library gets nothing, which is how JasperFx#5649 was reported in the first place. EventGraph.CreateTagType now turns the failure into an InvalidOperationException naming the one line to write, keeping the original as its inner exception. The two halves are gated differently on purpose. NotSupportedException and MissingMethodException mean the generic could not be closed, which only happens with no JIT, so they are reinterpreted only when dynamic code is unavailable. InvalidValueTypeException is ambiguous -- the tag may genuinely not be a single-value wrapper, or a trimmed image may have lost the members that say so, and no runtime flag separates those because a trimmed non-AOT image still has dynamic code -- so it is reinterpreted either way, with a message covering both readings. The explicit overload is the answer to both. Auto-discovery from projection identities had the inverse of that problem. JasperFx#5657 logs a warning for a discovery failure, but screens InvalidValueTypeException into a silent catch as "this identity was never a value type" -- which is exactly how a trimmed native image reports a perfectly good tag whose members were trimmed. The one scenario JasperFx#5649 reported was the one scenario that stayed quiet. IsToleratedDiscoveryFailure now tolerates it only while dynamic code is available, and the warning names the registration line including the projection's ForAggregate<T>(). A trimmed non-AOT image is still silent, because guessing the other way would warn on every projection whose identity is an ordinary custom type. Argument validation the overload did not have: a null unwrapper is refused, and re-registering a tag type under a DIFFERENT inner type now throws rather than silently keeping the first registration and leaving this call's unwrapper unused. BEHAVIOUR CHANGE: an empty or whitespace tag table suffix is refused on every RegisterTagType overload. mt_event_tag_ is a real table name, so "" was accepted and put every tag type registered that way into one shared table. Pass null for the default. Marten.AotRuntimeSmoke gains six DCB checks -- tagging, a TagWith rule, Guid- and string-backed tag queries, EventsExistAsync, FetchForWritingByTags into a [BoundaryAggregate] with an append, DcbConcurrencyException, and the inferring overload's message. Verified by RUNNING the native osx-arm64 binary: 48/48, and the remedy message does fire natively. Marten.AotSmoke registers a tag through the overload to hold the consumer-side analyzer axis clean. Co-authored-by: Tore Hammervoll <hammervoll.tore@gmail.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3
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.
Part of #5649 — deliberately not
Fixes, see below.Two hazards in how DCB tag types reach the store, both invisible until Native AOT. Neither needs an API decision, so they're split out from the
RegisterTagTypequestion still under discussion on the issue.Build<TDoc>()closed a generic it already knewEventGraph.Build<TDoc>()went throughCloseAndBuildAsfor a[BoundaryAggregate]even though both type arguments are known where it stands —TDocfrom the caller, andstringunconditionally per #324. ILC has no such instantiation, so every DCB boundary fetch threwMissingMethodException. Writing it out roots the instantiation at compile time.It carries a CS8714, the same one
ProjectionOptions.cs:374already takes for the identical construction:Build<TDoc>()is public and unconstrained, so the compiler can't see thatTDocsatisfiesnotnull. Constraining a public signature to silence it would be source-breaking for no runtime gain —CloseAndBuildAsnever checked it either.Auto-discovery dropped tag types in silence
autoDiscoverTagTypesFromProjectionsswallowed every exception through one barecatch. An identity that simply isn't a strong-typed identifier should be passed over without a word — but the AOT failure ofMakeGenericMethodwas being passed over just as quietly, leaving a store running with the tag type absent and nothing anywhere saying so. The queries that need it then fail far from the cause. @skoging called this one from reading the code; it's real.It is still not rethrown. This runs inside the store's
Initialize, and a throw would stop an application from starting over a feature it may not use — the same trade theCreateTagTypeMethodlookup immediately above it already makes, and the lesson from #5619. It logs a warning naming the identity type, the projection, and the remedy.Verified by running the native binary
Not by a clean publish, which proves nothing here.
Marten.AotRuntimeSmokegains a boundary-aggregate check. On the unfixed code, published natively for osx-arm64 and run, it reproduces the issue's reported exception verbatim:With the fix:
OK boundary aggregate aggregator source, 24 of 24, exit 0.The check calls
Build<TDoc>()directly rather than going throughAggregateByTagsAsync, because reaching the boundary fetch needs a registered tag type andRegisterTagType<TTag>closes its own open generic — the separate hole this PR doesn't close. This isolates the one site the fix moves, and it's the site the reported exception came from.Testing
Marten.AotRuntimeSmoke, native osx-arm64: 24/24, red-checked as above.*Dcb*): 196 passed, 2 skipped, 0 failed.EventSourcingTests: 2474 passed, 27 skipped, 0 failed.Two xUnit tests are added. Be aware of what they are and aren't: the
Build<TDoc>()one is a characterization test — it pins the constructed type so the refactor can't change behaviour, and it passes on master too, because the AOT failure can't be reproduced under the JIT. The native check above is the real red/green. The second test pins the regression the logging change risks, which is warning ordinary applications on every start about identities there was never anything to discover from.Why not
Fixes #5649RegisterTagType<TTag>still closesTagTypeRegistration<,>withMakeGenericType, andCreate<TTag>additionally reflects over the tag type throughValueTypeInfo.ForType. That needs a reflection-free construction path in JasperFx.Events plus a new Marten overload — discussed in this comment, including why a source generator is the right fourth step rather than the first.🤖 Generated with Claude Code
https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3