Tell a refused inline apart from an absent owner - #791
Merged
Conversation
TrySendAsync collapsed an error reply and a refused connection into the same false, and AddInlineAsync read that as "nobody owns the queue" and launched a viewer. But an owner that answered is still there and still holds the port, so the launched viewer cannot bind it - and AddInlineAsync returned Queued anyway, as soon as stdin was written. So a snapshot the owner declined - an older owner that does not understand the payload, or a handler that threw - was reported to the caller as queued while being held by nothing at all. Nothing was staged either, because staging is what NoViewerFound is for. The snapshot simply vanished, and the test that produced it went on reporting a failure with nowhere to review it. SendAsync returns three outcomes instead of two: NoOwner, Accepted, Refused. TrySendAsync stays as it was, as a wrapper, so PendingFiles and the rest are untouched. AddInlineAsync launches a viewer only on NoOwner, and reports NoViewerFound on a refusal - which is accurate from the caller's side, since in both cases the snapshot is pending nowhere and staging is the right answer. The rest of that item - RunInline ignoring response.Ok, and persisting through InlineStaging on a forward failure - is in DiffEngineViewer and not touched here.
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.
TrySendAsync collapsed an error reply and a refused connection into the same
false, and AddInlineAsync read that as "nobody owns the queue" and launched a
viewer. But an owner that answered is still there and still holds the port, so
the launched viewer cannot bind it - and AddInlineAsync returned Queued anyway,
as soon as stdin was written.
So a snapshot the owner declined - an older owner that does not understand the
payload, or a handler that threw - was reported to the caller as queued while
being held by nothing at all. Nothing was staged either, because staging is what
NoViewerFound is for. The snapshot simply vanished, and the test that produced
it went on reporting a failure with nowhere to review it.
SendAsync returns three outcomes instead of two: NoOwner, Accepted, Refused.
TrySendAsync stays as it was, as a wrapper, so PendingFiles and the rest are
untouched. AddInlineAsync launches a viewer only on NoOwner, and reports
NoViewerFound on a refusal - which is accurate from the caller's side, since in
both cases the snapshot is pending nowhere and staging is the right answer.
The rest of that item - RunInline ignoring response.Ok, and persisting through
InlineStaging on a forward failure - is in DiffEngineViewer and not touched
here.