Repository navigation
fix(exec): do not replay a remote command that may have started - #535
Open
pcbeingused333 wants to merge 3 commits into
Open
pcbeingused333 wants to merge 3 commits into
pcbeingused333 wants to merge 3 commits into
Conversation
brev exec treated every SSH error as a lost connection and, when the instance was still RUNNING, ran the same command again. A non-zero remote exit and a session that dropped mid-command both took that path, so a long job or an @script could start a second time. Retry only when ssh reports that the session never started. Any other failure is returned without a second run.
Matching bare errno strings anywhere in the ssh -E log treated a session that had already started as a failed connect. "Read from remote host: Connection timed out" and an earlier "Connection refused" followed by "Broken pipe" both ran the remote command again. Retry only when the last non-empty diagnostic line starts with an OpenSSH pre-session prefix.
sshpkt_fatal appends "Connection closed by <ip|UNKNOWN> port <n>" after kex_exchange_identification logs the banner close. The last line alone is also a dropped session, so replay only when the previous non-empty line is that kex prefix.
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.
Fixes #457
brev exectreated every error from the first SSH attempt as a lost connection. When the instance was still RUNNING, it refreshed the SSH config and ran the same remote command again. A command that exited non-zero and a session that dropped after the command had started both took that path. The second run is a new process, so a long job or an@scriptstarts over while the first one may still be writing the same output.The first attempt is the execution. It is retried only when ssh's own log shows the session never started. That log is taken with
ssh -E, so it is not mixed with the remote command's stderr, and it is still printed after ssh returns. A last non-empty line that starts withssh: connect to host,ssh: Could not resolve hostname, orkex_exchange_identification:still waits for SSH and runs the command once. That covers a translated strerror under the connect prefix, and a multi-address failure whose last line is still a connect error. A banner failure is the same case written as two lines:kex_exchange_identification:and thenConnection closed by <ip|UNKNOWN> port <n>orConnection reset by <ip|UNKNOWN> port <n>from sshpkt_fatal. That second line is retried only when the previous non-empty line is the kex line.Connection closed by 10.0.0.8 port 22by itself is a drop after the session exists and is not retried. Any other status is returned without a second run. Exit codes other than 255 are the remote command's own status. Exit 255 is not retried when the last line is a drop after the session exists, includingRead from remote host …: Connection timed outandBroken pipe, even if an earlier line saysConnection refused. An empty diagnostic is not retried either.Test
go test -count=1 -timeout 180s -run 'TestExecDoesNotReplay|TestExecStillRecovers|TestSSHFailureAllowsReplay|TestRunSSHWithTimeout' ./pkg/cmd/execWithout the replay check, a remote exit 1 and an exit 255 with
Broken pipeboth entered SSH recovery and printedConnection failed. With the check, those cases return the original error and invoke the command once. The same holds forRead from remote host brev: Connection timed outand for a log whose first line isssh: connect to host …: Connection refusedand whose last line isBroken pipe. A refused connection whose last line is stillssh: connect to hostenters recovery, including when every address failed before a session. The classifier accepts a non-English strerror when it sits under that prefix. A banner close whose lines arekex_exchange_identification: Connection closed by remote hostandConnection closed by UNKNOWN port 65535enters recovery, as does the matching reset pair. A loneConnection closed by 10.0.0.8 port 22does not, including when aBroken pipeline sits between it and an earlier kex line.This change was prepared with AI assistance.