fix: KEEP-1191 exit non-zero when a write fails before the first poll - #105
Merged
Conversation
A write response can already carry a terminal status. When that status was "failed", the immediate branch printed the result table and returned nil, so `kh ex transfer --wait` and `kh ex cc --wait` exited zero on a failed write. The identical failure discovered one poll later exited non-zero, because pollExecStatus classified it separately and did so correctly. Two paths decided what a terminal status meant and disagreed. Both now call terminalExecError, so the decision cannot drift again. The write response carries no error detail, so that path reports the execution id; the status path keeps surfacing the server message.
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.
A write response can already carry a terminal status. When that status was
failed, the immediate branch incmd/execute/transfer.goandcmd/execute/contract_call.goprinted the result table and returned nil, sokh ex transfer --waitandkh ex cc --waitexited zero on a write that failed. The identical failure discovered one poll later exited non-zero, becausepollExecStatusclassified it separately and did so correctly.A script gating on
kh ex transfer --wait && ./next-step.shtherefore proceeded on a failed transfer, but only when the failure was fast enough to be in the write response.Root cause is duplication: two places decided what a terminal status meant, and they disagreed. Both now call
terminalExecError, so the decision cannot drift again. The write response carries no error field, so that path reports the execution id; the status path keeps surfacing the server's message.Both new tests were mutation-checked - reverting the guard makes each fail with
expected an error ..., got nil, which is the reported symptom.go build,go vetandgo test -race ./...all pass. Formatting note:cmd/execute/{transfer,contract_call,status}_test.goandstatus.goare unformatted onmain(anIOStreams:alignment); I left that alone rather than mixing an unrelated reformat into this change.Merge order matters. Open contributor PRs #95 and #99 both rework this same terminal-status handling in
transfer.go, and they already conflict with each other. Landing this first means each rebases once onto the shared helper rather than reconciling three versions.Tracked as KEEP-1191. Found while finishing #95 and #99; pre-existing on
main, not introduced by either.