Skip to content

Commit fddc707

Browse files
authored
Fix hanging on exec for missing command (#33)
1 parent c9c5580 commit fddc707

3 files changed

Lines changed: 55 additions & 9 deletions

File tree

cmd/api/api/exec.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,11 @@ func (s *ApiService) ExecHandler(w http.ResponseWriter, r *http.Request) {
138138
"duration_ms", duration.Milliseconds(),
139139
)
140140
// Send error message over WebSocket before closing
141-
ws.WriteMessage(websocket.TextMessage, []byte(fmt.Sprintf("Error: %v", err)))
141+
// Use BinaryMessage so the CLI writes it to stdout (it ignores TextMessage for output)
142+
// Use \r\n so it displays properly when client terminal is in raw mode
143+
ws.WriteMessage(websocket.BinaryMessage, []byte(fmt.Sprintf("Error: %v\r\n", err)))
144+
// Send exit code 127 (command not found - standard Unix convention)
145+
ws.WriteMessage(websocket.TextMessage, []byte(`{"exitCode":127}`))
142146
return
143147
}
144148

@@ -199,4 +203,3 @@ func (w *wsReadWriter) Write(p []byte) (n int, err error) {
199203
}
200204
return len(p), nil
201205
}
202-

cmd/exec/main.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -143,22 +143,22 @@ func main() {
143143

144144
// Use custom WebSocket dialer
145145
dialer := &websocket.Dialer{}
146-
146+
147147
// Set up headers for WebSocket connection (body was already sent)
148148
headers := http.Header{}
149149
headers.Set("Authorization", fmt.Sprintf("Bearer %s", jwtToken))
150150

151151
// Make HTTP POST with body
152152
client := &http.Client{}
153-
153+
154154
// Actually, we need a custom approach. Let me use a modified request
155155
// that sends body AND upgrades to WebSocket.
156156
// For simplicity, let's POST the JSON as the Sec-WebSocket-Protocol header value (hacky but works)
157157
// OR we can encode params in URL query string
158-
158+
159159
// Actually, the simplest approach: POST the body first, get a session ID, then connect WebSocket
160160
// But that requires server changes.
161-
161+
162162
// Let's use the approach where we send JSON as first WebSocket message after connect
163163
ws, resp, err := dialer.Dial(wsURL.String(), headers)
164164
if err != nil {
@@ -240,7 +240,10 @@ func runInteractive(ws *websocket.Conn) (int, error) {
240240
for {
241241
msgType, message, err := ws.ReadMessage()
242242
if err != nil {
243-
if !websocket.IsUnexpectedCloseError(err, websocket.CloseNormalClosure, websocket.CloseGoingAway) {
243+
if websocket.IsUnexpectedCloseError(err, websocket.CloseNormalClosure, websocket.CloseGoingAway) {
244+
// Unexpected close - report as error to avoid hanging
245+
errCh <- fmt.Errorf("connection closed unexpectedly: %w", err)
246+
} else {
244247
// Normal close, default exit code 0
245248
exitCodeCh <- 0
246249
}
@@ -310,8 +313,8 @@ func runNonInteractive(ws *websocket.Conn) (int, error) {
310313
msgType, message, err := ws.ReadMessage()
311314
if err != nil {
312315
// Connection closed is normal - default exit code 0
313-
if websocket.IsCloseError(err, websocket.CloseNormalClosure, websocket.CloseGoingAway, websocket.CloseAbnormalClosure) ||
314-
err == io.EOF {
316+
if websocket.IsCloseError(err, websocket.CloseNormalClosure, websocket.CloseGoingAway, websocket.CloseAbnormalClosure) ||
317+
err == io.EOF {
315318
exitCodeCh <- 0
316319
return
317320
}

lib/instances/exec_test.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"testing"
1010
"time"
1111

12+
"github.com/onkernel/hypeman/lib/exec"
1213
"github.com/onkernel/hypeman/lib/images"
1314
"github.com/onkernel/hypeman/lib/paths"
1415
"github.com/onkernel/hypeman/lib/system"
@@ -214,4 +215,43 @@ func TestExecConcurrent(t *testing.T) {
214215
"streams appear serialized - took %v, expected < %v", streamElapsed, maxExpected)
215216

216217
t.Logf("Long-running streams completed in %v (concurrent OK)", streamElapsed)
218+
219+
// Phase 3: Test command not found returns quickly (no hang)
220+
// Regression test for a hang that occurred when the command wasn't found.
221+
t.Log("Phase 3: Testing exec with non-existent command...")
222+
223+
// Test without TTY
224+
start := time.Now()
225+
var stdout, stderr strings.Builder
226+
_, err = exec.ExecIntoInstance(ctx, inst.VsockSocket, exec.ExecOptions{
227+
Command: []string{"nonexistent_command_asdfasdf"},
228+
Stdout: &stdout,
229+
Stderr: &stderr,
230+
TTY: false,
231+
})
232+
elapsed := time.Since(start)
233+
t.Logf("Exec (no TTY) completed in %v (error: %v)", elapsed, err)
234+
235+
require.Error(t, err, "exec should fail for non-existent command")
236+
require.Contains(t, err.Error(), "executable file not found", "error should mention command not found")
237+
require.Less(t, elapsed, 5*time.Second, "exec should not hang, took %v", elapsed)
238+
239+
// Test with TTY
240+
start = time.Now()
241+
stdout.Reset()
242+
stderr.Reset()
243+
_, err = exec.ExecIntoInstance(ctx, inst.VsockSocket, exec.ExecOptions{
244+
Command: []string{"nonexistent_command_xyz123"},
245+
Stdout: &stdout,
246+
Stderr: &stderr,
247+
TTY: true,
248+
})
249+
elapsed = time.Since(start)
250+
t.Logf("Exec (with TTY) completed in %v (error: %v)", elapsed, err)
251+
252+
require.Error(t, err, "exec with TTY should fail for non-existent command")
253+
require.Contains(t, err.Error(), "executable file not found", "error should mention command not found")
254+
require.Less(t, elapsed, 5*time.Second, "exec with TTY should not hang, took %v", elapsed)
255+
256+
t.Log("Command not found tests passed - exec does not hang")
217257
}

0 commit comments

Comments
 (0)