Skip to content

Commit b07b0d7

Browse files
LeadGoEngineerPaperclip-Paperclip
andcommitted
fix(compat): return error on timeout instead of nil
exec.CommandContext kills the process when the deadline expires, and that kill surfaces as an *exec.ExitError on the signal path. The deadline check was after the ExitError check, so timeouts silently returned (res, nil) with a -1 exit code — masking hangs as clean non-zero exits. The package doc already promised timeouts return errors; this just lines the code up with the contract. Surfaced by the new TestRun_TimeoutReturnsError in compat_test.go. Co-Authored-By: Paperclip <[email protected]>
1 parent 66df147 commit b07b0d7

1 file changed

Lines changed: 9 additions & 3 deletions

File tree

compat/compat.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,14 +114,20 @@ func (r Runner) Run(ctx context.Context, args ...string) (Result, error) {
114114
res.ExitCode = 0
115115
return res, nil
116116
}
117+
// Timeout check first. exec.CommandContext kills the process when
118+
// the deadline expires, which surfaces as an *exec.ExitError on the
119+
// signal path. The package contract promises a non-nil error on
120+
// timeout, so we must detect that case before falling through to
121+
// the ExitError handler — otherwise a hung CLI looks like a clean
122+
// non-zero exit to the caller.
123+
if errors.Is(runCtx.Err(), context.DeadlineExceeded) {
124+
return res, fmt.Errorf("compat: %s timed out after %s", r.Binary, timeout)
125+
}
117126
var exitErr *exec.ExitError
118127
if errors.As(err, &exitErr) {
119128
res.ExitCode = exitErr.ExitCode()
120129
return res, nil
121130
}
122-
if errors.Is(runCtx.Err(), context.DeadlineExceeded) {
123-
return res, fmt.Errorf("compat: %s timed out after %s", r.Binary, timeout)
124-
}
125131
return res, fmt.Errorf("compat: failed to run %s: %w", r.Binary, err)
126132
}
127133

0 commit comments

Comments
 (0)