fix(transit/message): yield legit JSON null payload in ReceiveAllAsync (fixes #220)#263
Merged
Merged
Conversation
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.
Problem (fixes #220)
MessageTransit<TSend, TReceive>.ReceiveAllAsyncoverloadednullas a sentinel for two distinct conditions:ReceiveAsyncreturnsdefaultafter the length-prefix read returns 0 bytesnullpayload — 4-byte length prefix +6E 75 6C 6C; deserializes tonullfor nullableTReceiveThe receive loop checked
if (_receiveEof || message is null) yield break;. The value-type half of this bug was previously fixed (#177) by adding the explicit_receiveEofflag, but the secondarymessage is nullclause was kept and still terminates the stream on the first legitimate JSONnullmessage. Every subsequent peer message is silently lost — no exception, no event, no log.Example (pre-fix): peer sends
"hello",null,"world". Consumer'sawait foreachoverReceiveAllAsync()yields only"hello"and exits."world"is permanently dropped.Fix
Drop the secondary
message is nullterminator._receiveEofis the sole signal — and it is reliably set insideReceiveAsyncwhenever the length-prefix or payload read returns 0 bytes. The yield ismessage!because a legit JSONnullpayload is a valid yielded value for nullableTReceive.No public-API change.
Regression Test
Added in
tests/NetConduit.Transit.Message.UnitTests/MessageTransitTests.csunder#region ReceiveAllAsync:MessageTransit_ReceiveAllAsync_YieldsLegitimateJsonNullPayload_AndContinues—MessageTransit<string?, string?>sends"hello",null,"world"; asserts the receiver yields all three in order before EOF.Pre-fix verification (stashed the prod change, ran the new test):
Post-fix: passes.
Existing tests
MessageTransit_ReceiveAllAsync_StopsOnChannelCloseand..._WhenReceiveTypeIsValueType(covering EOF for reference and value-typeTReceiverespectively, #177) continue to pass —_receiveEofcorrectly drives termination for both cases.Verification
dotnet build -c Debug— 0 warnings, 0 errors across net8.0 / net9.0 / net10.0dotnet test -c Debug --no-build— 635 passed / 1 known-flake (PublicApiTests.OnChannelOpened_DoesNotFire_BeforeRemoteAccept, passes in isolation) / 1 pre-existing skip (MemoryLeak_SubMuxChaos_MemoryStaysBounded)