Skip to content

Commit e5f19db

Browse files
7cea4701430283a4267f819a278be05d_pingqaCopilot
authored andcommitted
review: switch cleanup to defer
Per @SamMorrowDrums review — replace the manual cleanup() calls before each error return with a single defer right after cmd.Start(). Same behaviour, less code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c05e1bb commit e5f19db

1 file changed

Lines changed: 5 additions & 13 deletions

File tree

cmd/mcpcurl/main.go

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -408,11 +408,13 @@ func executeServerCommand(cmdStr, jsonRequest string) (string, error) {
408408
return "", fmt.Errorf("failed to start command: %w", err)
409409
}
410410

411-
// Ensure the child process is cleaned up on any error after Start()
412-
cleanup := func() {
411+
// Ensure the child process is cleaned up on every return path.
412+
// stdin must be closed before Wait so the server sees EOF and exits;
413+
// its non-zero exit status on EOF is expected, so we ignore the error.
414+
defer func() {
413415
_ = stdin.Close()
414416
_ = cmd.Wait()
415-
}
417+
}()
416418

417419
// Use a scanner with a large buffer for reading JSON-RPC responses
418420
scanner := bufio.NewScanner(stdoutPipe)
@@ -421,43 +423,33 @@ func executeServerCommand(cmdStr, jsonRequest string) (string, error) {
421423
// Step 1: Send MCP initialize request
422424
initReq, err := buildInitializeRequest()
423425
if err != nil {
424-
cleanup()
425426
return "", fmt.Errorf("failed to build initialize request: %w", err)
426427
}
427428
if _, err := io.WriteString(stdin, initReq+"\n"); err != nil {
428-
cleanup()
429429
return "", fmt.Errorf("failed to write initialize request: %w", err)
430430
}
431431

432432
// Step 2: Read initialize response (skip any server notifications)
433433
if _, err := readJSONRPCResponse(scanner); err != nil {
434-
cleanup()
435434
return "", fmt.Errorf("failed to read initialize response: %w, stderr: %s", err, stderr.String())
436435
}
437436

438437
// Step 3: Send initialized notification
439438
if _, err := io.WriteString(stdin, buildInitializedNotification()+"\n"); err != nil {
440-
cleanup()
441439
return "", fmt.Errorf("failed to write initialized notification: %w", err)
442440
}
443441

444442
// Step 4: Send the actual request
445443
if _, err := io.WriteString(stdin, jsonRequest+"\n"); err != nil {
446-
cleanup()
447444
return "", fmt.Errorf("failed to write request: %w", err)
448445
}
449446

450447
// Step 5: Read the actual response (skip any server notifications)
451448
response, err := readJSONRPCResponse(scanner)
452449
if err != nil {
453-
cleanup()
454450
return "", fmt.Errorf("failed to read response: %w, stderr: %s", err, stderr.String())
455451
}
456452

457-
// Close stdin and wait for process to exit. The server will see EOF and
458-
// exit with a non-zero status, which is expected — we already have the response.
459-
cleanup()
460-
461453
return response, nil
462454
}
463455

0 commit comments

Comments
 (0)