fix(tsp): say why an inbound frame was not claimed as a reply - #136
Merged
Conversation
The claim predicate added with the TSP inbound work returned `false` for every failure mode without recording any of them. On a shared socket that is right in principle — a frame from another peer is not this request's problem — but it made the one case that *is* a problem indistinguishable from silence: when the VTA's reply is declined, the request waits out its full timeout and reports `timed out awaiting reply frame`, with no record anywhere that a frame arrived at all, let alone why it was rejected. That is a regression in diagnosability against the code it replaced, which threw a typed error naming the sender mismatch or the parse failure. The predicate is the only thing in the path that knows; if it does not report, by construction nothing downstream can. It now records the reason for the most recent decline — unpack failure, sender mismatch, unparseable payload, or a `threadId` that does not thread to this request (naming both values, plus the reply's own type and id) — and attaches it to the error the caller sees when the wait ends. The frame is still declined in exactly the same cases; only the silence is removed. Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
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
1 participant
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.
Silence was the bug I could actually prove
The claim predicate from #134 returned
falsefor every failure mode without recording any of them. On a shared socket that is right in principle — a frame from another peer is not this request's problem — but it made the one case that is a problem indistinguishable from silence.When the VTA's own reply gets declined, the request waits out its full timeout and reports
timed out awaiting reply frame, with no record that a frame arrived at all, let alone why it was rejected. Four completely different faults — unpack failure, unexpected sender, unparseable payload, athreadIdthat doesn't thread — all present identically, and each needs a different fix.That is a regression in diagnosability against the code this replaced, which threw a typed error naming the sender mismatch or the parse failure. The predicate is the only thing in the path that knows; if it doesn't report, by construction nothing downstream can.
What changed
It records the reason for the most recent decline and attaches it to the error the caller sees when the wait ends:
The
threadIdcase names both values plus the reply's own type and id, because a mismatch is only actionable if you can see what it was compared against.The frame is still declined in exactly the same cases. Only the silence is removed — the predicate logic is unchanged, byte for byte.
Field note
This was written to diagnose a live failure: vault entries would not load over TSP after a fresh enrolment, with nothing in the logs but a 30s timeout. The wallet worked with
preferTspoff, which narrowed it to the TSP path but no further.On the build with these diagnostics, TSP works — so the predicate is not inherently broken, and the earlier failure looks transient. The leading explanation is the pre-0.7.0 backlog of never-acked TSP frames draining on first connect after the upgrade; see the follow-up note in the linked discussion about serial inbound processing starving an in-flight reply. That hypothesis is unproven precisely because there was no decline line to read at the time — which is the argument for this change.
463 core tests pass; build clean.