diff --git a/internal/app/shell/run.go b/internal/app/shell/run.go index 0727809..2aed4f3 100644 --- a/internal/app/shell/run.go +++ b/internal/app/shell/run.go @@ -38,6 +38,29 @@ func wrapCommand(cmd string, os remote.OS) string { return fmt.Sprintf("echo __DWSH_BEGIN__; ( %s ); echo __DWSH_RC_$?_END__\r", cmd) } +// truncationHintLength is the point past which a remote shell could have +// truncated the command line. The lowest limit measured is a shell reading in +// canonical mode, cut by the tty at 4094 characters; below that no remote is +// known to truncate, so the hint could only mislead. +const truncationHintLength = 4000 + +// timeoutErr explains a timeout, adding what a truncated command line looks +// like when the line was long enough for that to be possible — past its limit a +// remote shell cuts the line silently and the exit-code marker goes with it, so +// the wait can never end. +// +// It stays a possibility, never a diagnosis: a command that is simply still +// running looks exactly the same from here. Seeing the BEGIN marker only rules +// out the case where nothing ran at all, not the slow one. +func timeoutErr(out []byte, lineLen int) error { + if lineLen > truncationHintLength && reBegin.Match(out) { + return fmt.Errorf("timed out waiting for command to finish; the command line was %d characters, "+ + "and past a limit of its own the remote shell truncates the line silently, losing the exit-code "+ + "marker — if that is what happened here, upload it with `dwshell put` and run it by path", lineLen) + } + return fmt.Errorf("timed out waiting for command to finish") +} + // Run executes a single command non-interactively and returns its output and // exit code. It opens a fresh shell, sends the wrapped command, and reads until // the RC sentinel (or ctx/timeout fires). @@ -54,7 +77,8 @@ func Run(ctx context.Context, sess *session.Session, os remote.OS, command strin return nil, err } - if err := sh.Input(wrapCommand(command, os)); err != nil { + wrapped := wrapCommand(command, os) + if err := sh.Input(wrapped); err != nil { return nil, err } @@ -74,7 +98,8 @@ func Run(ctx context.Context, sess *session.Session, os remote.OS, command strin case <-ctx.Done(): return nil, ctx.Err() case <-timeoutCh: - return nil, fmt.Errorf("timed out waiting for command to finish") + // The line as typed, minus the Enter that submits it. + return nil, timeoutErr(buf.Bytes(), len(wrapped)-1) case chunk, ok := <-sh.Output(): if !ok { if sh.Err() != nil { diff --git a/internal/app/shell/timeout_test.go b/internal/app/shell/timeout_test.go new file mode 100644 index 0000000..32da72a --- /dev/null +++ b/internal/app/shell/timeout_test.go @@ -0,0 +1,36 @@ +package shell + +import ( + "strings" + "testing" +) + +// A short command line cannot have been truncated by any remote shell, so the +// timeout must not speculate about it. +func TestTimeoutErrStaysPlainForAShortLine(t *testing.T) { + err := timeoutErr([]byte("__DWSH_BEGIN__\r\nsome output\r\n"), 120) + if got := err.Error(); got != "timed out waiting for command to finish" { + t.Fatalf("expected the plain timeout message, got %q", got) + } +} + +// Long enough to have been cut, and something did run: offer truncation as a +// possibility, with the way around it. +func TestTimeoutErrHintsTruncationForALongLine(t *testing.T) { + err := timeoutErr([]byte("__DWSH_BEGIN__\r\nsome output\r\n"), 9000) + got := err.Error() + for _, want := range []string{"timed out", "9000", "truncate", "dwshell put"} { + if !strings.Contains(got, want) { + t.Errorf("message %q does not mention %q", got, want) + } + } +} + +// Nothing ran at all — the marker never came back — so truncation is not the +// story and the hint would mislead. +func TestTimeoutErrStaysPlainWhenNothingRan(t *testing.T) { + err := timeoutErr([]byte("no marker here"), 9000) + if got := err.Error(); got != "timed out waiting for command to finish" { + t.Fatalf("expected the plain timeout message, got %q", got) + } +}