Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/envd/internal/services/process/handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,14 +493,14 @@ func (p *Handler) ResizeTty(size *pty.Winsize) error {

func (p *Handler) WriteStdin(data []byte) error {
if p.tty != nil {
return errors.New("tty assigned to process — input should be written to the pty, not the stdin")
return ErrStdinOnPty
}

p.stdinMu.Lock()
defer p.stdinMu.Unlock()

if p.stdin == nil {
return errors.New("stdin not enabled or closed")
return ErrStdinUnavailable
}

_, err := p.stdin.Write(data)
Expand All @@ -515,7 +515,7 @@ func (p *Handler) WriteStdin(data []byte) error {
// Only works for non-PTY processes.
func (p *Handler) CloseStdin() error {
if p.tty != nil {
return errors.New("cannot close stdin for PTY process — send Ctrl+D (0x04) instead")
return ErrCloseStdinOnPty
}

p.stdinMu.Lock()
Expand All @@ -535,7 +535,7 @@ func (p *Handler) CloseStdin() error {

func (p *Handler) WriteTty(data []byte) error {
if p.tty == nil {
return errors.New("tty not assigned to process — input should be written to the stdin, not the tty")
return ErrTtyUnavailable
}

_, err := p.tty.Write(data)
Expand Down
66 changes: 66 additions & 0 deletions packages/envd/internal/services/process/handler/input_error.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package handler

import (
"errors"
"io"
"io/fs"
"os"
"syscall"

"connectrpc.com/connect"
)

// Sentinel errors returned by the stdin/pty input methods for expected process
// lifecycle states — as opposed to genuine I/O failures. They let the service
// layer map an input failure to a precise Connect code instead of collapsing
// everything to CodeInternal (see issue #3622).
var (
// ErrStdinUnavailable means the process cannot accept stdin right now: it was
// started with stdin disabled, or stdin has already been closed. This is an
// expected precondition failure, not an envd fault.
ErrStdinUnavailable = errors.New("stdin not enabled or closed")

// ErrStdinOnPty means stdin was written to a PTY-backed process; input must
// go to the pty instead. An expected precondition failure.
ErrStdinOnPty = errors.New("tty assigned to process — input should be written to the pty, not the stdin")

// ErrTtyUnavailable means a pty write targeted a process that has no tty.
ErrTtyUnavailable = errors.New("tty not assigned to process — input should be written to the stdin, not the tty")

// ErrCloseStdinOnPty means CloseStdin was called on a PTY-backed process.
ErrCloseStdinOnPty = errors.New("cannot close stdin for PTY process — send Ctrl+D (0x04) instead")
)

// InputErrorCode maps a WriteStdin/WriteTty/CloseStdin failure to the Connect
// code the client should observe.
//
// - Expected process-lifecycle states (stdin disabled/closed, wrong pipe for
// the process type, or the child's read end already gone after exit) are
// CodeFailedPrecondition: the request was well-formed but the process is not
// in a state to accept it.
// - Everything else is a real underlying I/O failure and stays CodeInternal.
//
// CodeInternal is thus reserved for genuine invariant failures, so a client can
// distinguish a normal process-state transition from an unexpected envd fault.
// The process's EndEvent remains the authoritative exit signal; an input error
// alone must not be treated as the terminal process result.
func InputErrorCode(err error) connect.Code {
switch {
// Explicit precondition sentinels from the input methods.
case errors.Is(err, ErrStdinUnavailable),
errors.Is(err, ErrStdinOnPty),
errors.Is(err, ErrTtyUnavailable),
errors.Is(err, ErrCloseStdinOnPty):
return connect.CodeFailedPrecondition
// The process exited and its stdin/pty read end is gone: the write end sees
// EPIPE, or Go's file wrapper reports it was already closed (os.ErrClosed /
// io.ErrClosedPipe). All are expected once the process is no longer running.
case errors.Is(err, syscall.EPIPE),
errors.Is(err, os.ErrClosed),
errors.Is(err, io.ErrClosedPipe),
errors.Is(err, fs.ErrClosed):
return connect.CodeFailedPrecondition
default:
return connect.CodeInternal
}
}
156 changes: 156 additions & 0 deletions packages/envd/internal/services/process/handler/input_error_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
package handler

import (
"errors"
"fmt"
"io"
"io/fs"
"os"
"os/exec"
"syscall"
"testing"
"time"

"connectrpc.com/connect"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestInputErrorCode(t *testing.T) {
t.Parallel()

for _, tc := range []struct {
err error
name string
want connect.Code
}{
{
name: "stdin disabled or closed is a precondition failure",
err: ErrStdinUnavailable,
want: connect.CodeFailedPrecondition,
},
{
name: "stdin on a pty process is a precondition failure",
err: ErrStdinOnPty,
want: connect.CodeFailedPrecondition,
},
{
name: "pty write with no tty is a precondition failure",
err: ErrTtyUnavailable,
want: connect.CodeFailedPrecondition,
},
{
name: "close stdin on a pty process is a precondition failure",
err: ErrCloseStdinOnPty,
want: connect.CodeFailedPrecondition,
},
{
name: "wrapped precondition sentinel is still a precondition failure",
err: fmt.Errorf("error writing to stdin of process '%d': %w", 42, ErrStdinUnavailable),
want: connect.CodeFailedPrecondition,
},
{
name: "EPIPE after exit is a precondition failure",
err: fmt.Errorf("error writing to stdin of process '%d': %w", 42, syscall.EPIPE),
want: connect.CodeFailedPrecondition,
},
{
name: "file already closed after exit is a precondition failure",
err: fmt.Errorf("error writing to stdin of process '%d': %w", 42, os.ErrClosed),
want: connect.CodeFailedPrecondition,
},
{
name: "closed pipe after exit is a precondition failure",
err: fmt.Errorf("wrapped: %w", io.ErrClosedPipe),
want: connect.CodeFailedPrecondition,
},
{
name: "fs closed after exit is a precondition failure",
err: &fs.PathError{Op: "write", Path: "|1", Err: fs.ErrClosed},
want: connect.CodeFailedPrecondition,
},
{
name: "unexpected I/O failure stays internal",
err: fmt.Errorf("error writing to stdin of process '%d': %w", 42, syscall.EIO),
want: connect.CodeInternal,
},
{
name: "opaque error stays internal",
err: errors.New("something went wrong"),
want: connect.CodeInternal,
},
} {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()

got := InputErrorCode(tc.err)
assert.Equalf(t, tc.want, got, "got %s, want %s", got, tc.want)
})
}
}

// Asserts on the code carried by *connect.Error — what the client observes on the wire.
func TestInputErrorClientObservedCode(t *testing.T) {
t.Parallel()

precond := fmt.Errorf("error writing to stdin: %w", ErrStdinUnavailable)
assert.Equal(t, connect.CodeFailedPrecondition, connect.CodeOf(connect.NewError(InputErrorCode(precond), precond)))

internal := fmt.Errorf("error writing to stdin: %w", syscall.EIO)
assert.Equal(t, connect.CodeInternal, connect.CodeOf(connect.NewError(InputErrorCode(internal), internal)))
}

// WriteStdin returns the typed sentinels for the two synchronous precondition
// states so the service layer can map them without string matching.
func TestWriteStdinPreconditionSentinels(t *testing.T) {
t.Parallel()

t.Run("stdin disabled", func(t *testing.T) {
t.Parallel()
h := &Handler{} // stdin == nil, no tty
err := h.WriteStdin([]byte("x"))
require.Error(t, err)
assert.ErrorIs(t, err, ErrStdinUnavailable)
assert.Equal(t, connect.CodeFailedPrecondition, InputErrorCode(err))
})

t.Run("pty process rejects stdin", func(t *testing.T) {
t.Parallel()
// A non-nil tty routes stdin writes to ErrStdinOnPty before any pipe use.
h := &Handler{tty: os.NewFile(0, "fake-tty")}
err := h.WriteStdin([]byte("x"))
require.Error(t, err)
assert.ErrorIs(t, err, ErrStdinOnPty)
assert.Equal(t, connect.CodeFailedPrecondition, InputErrorCode(err))
})
}

// Reproduces issue #3622: a short-lived non-PTY process whose stdin read end is
// gone after exit. WriteStdin must surface an expected-lifecycle error that
// maps to CodeFailedPrecondition, not CodeInternal.
func TestWriteStdinAfterExitIsPrecondition(t *testing.T) {
t.Parallel()

cmd := exec.Command("/bin/sh", "-c", "exit 0")
stdin, err := cmd.StdinPipe()
require.NoError(t, err)
require.NoError(t, cmd.Start())

h := &Handler{stdin: stdin}

_ = cmd.Wait()
time.Sleep(50 * time.Millisecond)

var writeErr error
for i := 0; i < 100; i++ {
if writeErr = h.WriteStdin([]byte("hello\n")); writeErr != nil {
break
}
time.Sleep(5 * time.Millisecond)
}
require.Error(t, writeErr, "expected WriteStdin to fail after process exit")

code := InputErrorCode(writeErr)
assert.Equalf(t, connect.CodeFailedPrecondition, code,
"post-exit stdin write should be CodeFailedPrecondition, got %s (raw: %v)", code, writeErr)
}
10 changes: 5 additions & 5 deletions packages/envd/internal/services/process/input.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ func handleInput(process *handler.Handler, in *rpc.ProcessInput) error {
case *rpc.ProcessInput_Pty:
err := process.WriteTty(in.GetPty())
if err != nil {
return connect.NewError(connect.CodeInternal, fmt.Errorf("error writing to tty: %w", err))
return connect.NewError(handler.InputErrorCode(err), fmt.Errorf("error writing to tty: %w", err))
}

case *rpc.ProcessInput_Stdin:
err := process.WriteStdin(in.GetStdin())
if err != nil {
return connect.NewError(connect.CodeInternal, fmt.Errorf("error writing to stdin: %w", err))
return connect.NewError(handler.InputErrorCode(err), fmt.Errorf("error writing to stdin: %w", err))
}

default:
Expand Down Expand Up @@ -87,13 +87,13 @@ func (s *Service) CloseStdin(
_ context.Context,
req *connect.Request[rpc.CloseStdinRequest],
) (*connect.Response[rpc.CloseStdinResponse], error) {
handler, err := s.getProcess(req.Msg.GetProcess())
proc, err := s.getProcess(req.Msg.GetProcess())
if err != nil {
return nil, err
}

if err := handler.CloseStdin(); err != nil {
return nil, connect.NewError(connect.CodeUnknown, fmt.Errorf("error closing stdin: %w", err))
if err := proc.CloseStdin(); err != nil {
return nil, connect.NewError(handler.InputErrorCode(err), fmt.Errorf("error closing stdin: %w", err))
}

return connect.NewResponse(&rpc.CloseStdinResponse{}), nil
Expand Down