Repository navigation
Name the explicit DCB tag registration when inference cannot work (supersedes #5663) - #5676
Merged
Merged
Conversation
…work Supersedes #5663, whose API half landed independently in #5674. What is left is the part #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. #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 #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. #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 #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
Member
Author
This was referenced Oct 16, 2026
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.
Supersedes #5663 (@skoging), rebuilt on current master to eliminate its merge conflict, with that review's follow-ups fixed. Closes the remaining half of #5649.
Why a new PR
#5663 and #5674 independently implemented the same API. #5674 merged first (
8689bc8), so master already hasRegisterTagType<TTag, TInner>(unwrapper, tableSuffix)onEventGraphandIEventStoreOptions, the[RequiresUnreferencedCode]/[RequiresDynamicCode]annotations on the inferring overloads, the JasperFx 2.90.0 bump, and adcb.mdsection. #5663 then conflicted in four files, and reconciling it in place would have meant choosing between two signatures of one method.This branch keeps master's signature —
string? tableSuffix = null, which matchesTagTypeRegistration.FromUnwrapper's own default and the suffix-lessRegisterTagType<TTag>()— and carries over only what #5674 did not: the diagnostics, the validation, and the native coverage. It touches noDirectory.Packages.props, so there is nothing to conflict with.What it adds
The remedy, at runtime
RegisterTagType<TTag>()reflects over the tag's members withValueTypeInfoand closesTagTypeRegistration<TTag, TInner>withMakeGenericType. #5674's annotations warn at build time, 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 #5649 was reported.EventGraph.CreateTagTypenow turns the failure into anInvalidOperationExceptionnaming the line to write, with the original asInnerException:The shape follows #5600 and #5635:
FullNameInCode()in the prose,NameInCode()in the line,canEmitas a parameter so a JIT test reaches the native branch.The two exception families are gated differently, and that is the first review follow-up.
NotSupportedException/MissingMethodExceptionmean the generic could not be closed, which only happens with no JIT, so they are reinterpreted only whencanEmitis false.InvalidValueTypeExceptionis 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. It is reinterpreted either way, with a message covering both readings, because the explicit overload is the answer to both. #5663 gated all three on!canEmit, which left the one case that is actually about trimming with no remedy.Auto-discovery's silent AOT failure
The second follow-up, and the sharper of the two. #5657 logs a warning when discovery from a projection identity fails, but screens
InvalidValueTypeExceptioninto 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 away. So the one scenario #5649 reported was the one scenario that stayed quiet, while theNotSupportedExceptionpath got a warning.IsToleratedDiscoveryFailure(e, canEmit)now tolerates it only while dynamic code is available, and the warning names the registration line including the projection'sForAggregate<T>(). A trimmed non-AOT image is still silent, deliberately: nothing at runtime distinguishes it from an identity that was never a value type, and guessing the other way would warn on every projection whose identity is an ordinary custom type. The comment says so rather than leaving it to be rediscovered.Validation the overload did not have
unwrapperis refused.RegisterTagTypeoverload.mt_event_tag_is a real table name, soRegisterTagType<T>("")was accepted and put every tag type registered that way into one shared table.nullis the way to ask for the default;""never meant anything a caller wanted. Nothing in the suite relied on it.The third review note, deliberately not fixed
TagTypeRegistrationLineusesNameInCode(), so a tag type nested inside another type prints a line that needs its outer scope added. That is true of the #5600 and #5635 remedies too, and fixing only this one would make the three inconsistent. It belongs in a cosmetic pass over all three, not here. A remark on the method records the choice.Testing
Marten.AotRuntimeSmoke, published and run as a native osx-arm64 binary (.NET 10): 48/48. Six of those are new: tagging, aTagWithrule,Guid- andstring-backed tag queries,EventsExistAsync,FetchForWritingByTagsinto a[BoundaryAggregate]with an append,DcbConcurrencyException, and the inferring overload's message. The native run prints(inferred registration refused with the remedy, as natively)— the remedy is verified by running the binary, not by publishing it. Master has no native DCB coverage at all; GH-5673 TombstoneStream (hard stream delete) on JasperFx 2.90.0 #5674 touched this project by three lines of comment.explicit_tag_type_registration: 25 tests, all green. Covers both overloads registering the same shape,nulltaking the same default, first-registration-wins both ways, the conflict refusal, both validation refusals on both overloads,TagTablesandHStoreend-to-end on explicit registrations, the remedy text for a readable/unreadable/string-backed/renamed-member tag, and the discovery-tolerance predicate on both sides ofcanEmit.EventSourcingTestson net10.0: 2534 passed, 27 skipped, 0 failed.Marten.AotSmoke: builds clean, no IL warnings from the consumer's ownProgram.cs— the overload is analyzer-clean for consumers. It registers a tag through the new overload and deliberately never calls an inferring one.markdownlintandcspellclean on both changed docs.One thing worth knowing about the smoke app
dotnet runofMarten.AotRuntimeSmokefails one pre-existing check,an unregistered enum scalar projection names RegisterScalarType. It is not a regression — I reproduced it on clean master (7b4b409) with none of these changes, and it passes in the native run above.PublishAotturnsRuntimeFeature.IsDynamicCodeSupportedoff underdotnet runas well, so a JIT process takes the AOT refusal branch while still answering the query. That makesdotnet runof this project a misleading gate; only the published binary counts. The new DCB check is written to accept either outcome for the same reason.Credit
The API, the test suite, the AOT smoke checks and the docs here are @skoging's work from #5663, carried over and adapted; they also did the JasperFx half (JasperFx/jasperfx#1012) and the original native verification. Closing #5663 in favour of this.
🤖 Generated with Claude Code
https://claude.ai/code/session_01481eiEZg1Bv6pu4DrgPRg3