Skip to content

Commit 9506419

Browse files
kdy1kdy1
authored
feat(derun): unify user-facing error message contracts (#295)
## Summary - unify derun user-facing error messages to stable formats (`invalid arguments`, `failed to`, `parse <field>`) - preserve compatibility-critical tokens for automation/MCP consumers (`session not found`, `parse <field>`, `session_id is required`, `cursor is required`) - improve CLI stderr, MCP JSON-RPC error messages, and transport failure messages that flow into session `final.error` - document the error-message contract updates in derun docs ## Testing - go test ./cmds/derun/...
1 parent 918d56d commit 9506419

18 files changed

Lines changed: 184 additions & 77 deletions

‎cmds/derun/internal/cli/errors.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package cli
2+
3+
import "fmt"
4+
5+
func formatUsageError(reason, hint string) string {
6+
if hint == "" {
7+
return fmt.Sprintf("invalid arguments: %s", reason)
8+
}
9+
return fmt.Sprintf("invalid arguments: %s; hint: %s", reason, hint)
10+
}
11+
12+
func formatRuntimeError(action string, err error) string {
13+
if err == nil {
14+
return fmt.Sprintf("failed to %s", action)
15+
}
16+
return fmt.Sprintf("failed to %s: %v", action, err)
17+
}

‎cmds/derun/internal/cli/mcp.go‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,23 +26,26 @@ func ExecuteMCP(args []string) int {
2626
return 2
2727
}
2828
if len(fs.Args()) != 0 {
29-
fmt.Fprintln(os.Stderr, "mcp command does not accept positional arguments")
29+
fmt.Fprintln(
30+
os.Stderr,
31+
formatUsageError("mcp command does not accept positional arguments", "use `derun mcp` without extra arguments"),
32+
)
3033
return 2
3134
}
3235

3336
stateRoot, err := resolveStateRootForMCP()
3437
if err != nil {
35-
fmt.Fprintf(os.Stderr, "resolve state root: %v\n", err)
38+
fmt.Fprintln(os.Stderr, formatRuntimeError("resolve state root", err))
3639
return 1
3740
}
3841
store, err := state.New(stateRoot)
3942
if err != nil {
40-
fmt.Fprintf(os.Stderr, "init state store: %v\n", err)
43+
fmt.Fprintln(os.Stderr, formatRuntimeError("initialize state store", err))
4144
return 1
4245
}
4346
logger, err := logging.New(stateRoot)
4447
if err != nil {
45-
fmt.Fprintf(os.Stderr, "init logger: %v\n", err)
48+
fmt.Fprintln(os.Stderr, formatRuntimeError("initialize logger", err))
4649
return 1
4750
}
4851
defer logger.Close()
@@ -51,7 +54,7 @@ func ExecuteMCP(args []string) int {
5154

5255
server := mcp.NewServer(store, logger, 10*time.Minute, defaultRetention)
5356
if err := server.Serve(context.Background(), os.Stdin, os.Stdout); err != nil {
54-
fmt.Fprintf(os.Stderr, "run mcp server: %v\n", err)
57+
fmt.Fprintln(os.Stderr, formatRuntimeError("run mcp server", err))
5558
return 1
5659
}
5760
return 0

‎cmds/derun/internal/cli/root.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ func Execute(args []string) int {
2828
case contracts.DerunCommandMCP:
2929
return ExecuteMCP(args[1:])
3030
default:
31-
fmt.Fprintf(os.Stderr, "unknown command: %s\n", args[0])
31+
fmt.Fprintln(
32+
os.Stderr,
33+
formatUsageError(fmt.Sprintf("unknown command %q", args[0]), "run `derun help` to see available commands"),
34+
)
3235
printUsage()
3336
return 2
3437
}
@@ -40,7 +43,10 @@ func executeHelp(args []string) int {
4043
return 0
4144
}
4245
if len(args) > 1 {
43-
fmt.Fprintln(os.Stderr, "help command accepts at most one topic")
46+
fmt.Fprintln(
47+
os.Stderr,
48+
formatUsageError("help command accepts at most one topic", "use `derun help` or `derun help <run|mcp>`"),
49+
)
4450
return 2
4551
}
4652

@@ -52,7 +58,10 @@ func executeHelp(args []string) int {
5258
printMCPUsage()
5359
return 0
5460
default:
55-
fmt.Fprintf(os.Stderr, "unknown help topic: %s\n", args[0])
61+
fmt.Fprintln(
62+
os.Stderr,
63+
formatUsageError(fmt.Sprintf("unknown help topic %q", args[0]), "use `derun help` to list supported topics"),
64+
)
5665
printUsage()
5766
return 2
5867
}

‎cmds/derun/internal/cli/root_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func TestExecuteHelpCommandRejectsUnknownTopic(t *testing.T) {
9292
if exitCode != 2 {
9393
t.Fatalf("unexpected exit code: got=%d want=2", exitCode)
9494
}
95-
if !strings.Contains(stderrOutput, "unknown help topic: unknown-topic") {
95+
if !strings.Contains(stderrOutput, `invalid arguments: unknown help topic "unknown-topic"`) {
9696
t.Fatalf("expected unknown topic error: %q", stderrOutput)
9797
}
9898
}

‎cmds/derun/internal/cli/run.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,10 @@ func selectTransportMode(ttyAttached bool, goos string) contracts.DerunTransport
8383

8484
func validateRetentionDuration(retentionDuration time.Duration) error {
8585
if retentionDuration <= 0 {
86-
return errors.New("retention must be positive")
86+
return errors.New(formatUsageError("retention must be positive", "use values like 1s, 5m, or 24h"))
8787
}
8888
if retentionDuration%time.Second != 0 {
89-
return errors.New("retention must be a whole number of seconds (for example: 1s, 30s, 5m)")
89+
return errors.New(formatUsageError("retention must use whole-second precision", "use values like 1s, 30s, or 5m"))
9090
}
9191
return nil
9292
}

‎cmds/derun/internal/cli/run_flow.go‎

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,20 @@ func parseRunRequest(args []string) (runRequest, int) {
6666
return runRequest{}, 2
6767
}
6868
if !hasSeparator {
69-
fmt.Fprintln(os.Stderr, "run command requires '--' separator before target command")
69+
fmt.Fprintln(
70+
os.Stderr,
71+
formatUsageError(
72+
"run command requires '--' separator before target command",
73+
"use `derun run [flags] -- <command> [args...]`",
74+
),
75+
)
7076
return runRequest{}, 2
7177
}
7278
if len(commandArgs) == 0 {
73-
fmt.Fprintln(os.Stderr, "run command requires target command")
79+
fmt.Fprintln(
80+
os.Stderr,
81+
formatUsageError("run command requires target command", "provide a command after `--`"),
82+
)
7483
return runRequest{}, 2
7584
}
7685
if err := validateRetentionDuration(request.retentionDuration); err != nil {
@@ -85,18 +94,18 @@ func parseRunRequest(args []string) (runRequest, int) {
8594
func initRunRuntime(request runRequest) (*runRuntime, int) {
8695
stateRoot, err := resolveStateRootForRun()
8796
if err != nil {
88-
fmt.Fprintf(os.Stderr, "resolve state root: %v\n", err)
97+
fmt.Fprintln(os.Stderr, formatRuntimeError("resolve state root", err))
8998
return nil, 1
9099
}
91100

92101
store, err := state.New(stateRoot)
93102
if err != nil {
94-
fmt.Fprintf(os.Stderr, "init state store: %v\n", err)
103+
fmt.Fprintln(os.Stderr, formatRuntimeError("initialize state store", err))
95104
return nil, 1
96105
}
97106
logger, err := logging.New(stateRoot)
98107
if err != nil {
99-
fmt.Fprintf(os.Stderr, "init logger: %v\n", err)
108+
fmt.Fprintln(os.Stderr, formatRuntimeError("initialize logger", err))
100109
return nil, 1
101110
}
102111

@@ -121,7 +130,7 @@ func prepareSession(runtimeState *runRuntime, request runRequest) (*preparedRunS
121130
if sessionID == "" {
122131
sessionID, err = generateUniqueSessionID(runtimeState.store, runtimeState.logger)
123132
if err != nil {
124-
fmt.Fprintf(os.Stderr, "generate session id: %v\n", err)
133+
fmt.Fprintln(os.Stderr, formatRuntimeError("generate session id", err))
125134
return nil, 1
126135
}
127136
} else {
@@ -132,30 +141,42 @@ func prepareSession(runtimeState *runRuntime, request runRequest) (*preparedRunS
132141
"session_id": sessionID,
133142
"reason": string(sessionIDRejectionReasonInvalid),
134143
})
135-
fmt.Fprintf(os.Stderr, "invalid session id: %s\n", sessionID)
144+
fmt.Fprintln(
145+
os.Stderr,
146+
formatUsageError(
147+
fmt.Sprintf("invalid session id %q", sessionID),
148+
"use a unique path-safe id (for example: 01J0S444444444444444444444)",
149+
),
150+
)
136151
return nil, 2
137152
}
138-
fmt.Fprintf(os.Stderr, "check session metadata: %v\n", err)
153+
fmt.Fprintln(os.Stderr, formatRuntimeError("check session metadata", err))
139154
return nil, 1
140155
}
141156
if hasMetadata {
142157
runtimeState.logger.Event("session_id_rejected", map[string]any{
143158
"session_id": sessionID,
144159
"reason": string(sessionIDRejectionReasonMetadataExists),
145160
})
146-
fmt.Fprintf(os.Stderr, "session id already exists: %s\n", sessionID)
161+
fmt.Fprintln(
162+
os.Stderr,
163+
formatUsageError(
164+
fmt.Sprintf("session id already exists %q", sessionID),
165+
"omit `--session-id` or choose a different id",
166+
),
167+
)
147168
return nil, 2
148169
}
149170
}
150171

151172
if err := runtimeState.store.EnsureSessionDir(sessionID); err != nil {
152-
fmt.Fprintf(os.Stderr, "prepare session directory: %v\n", err)
173+
fmt.Fprintln(os.Stderr, formatRuntimeError("prepare session directory", err))
153174
return nil, 1
154175
}
155176

156177
workingDir, err := os.Getwd()
157178
if err != nil {
158-
fmt.Fprintf(os.Stderr, "resolve working directory: %v\n", err)
179+
fmt.Fprintln(os.Stderr, formatRuntimeError("resolve working directory", err))
159180
return nil, 1
160181
}
161182

@@ -174,7 +195,7 @@ func prepareSession(runtimeState *runRuntime, request runRequest) (*preparedRunS
174195
PID: 0,
175196
}
176197
if err := runtimeState.store.WriteMeta(meta); err != nil {
177-
fmt.Fprintf(os.Stderr, "write metadata: %v\n", err)
198+
fmt.Fprintln(os.Stderr, formatRuntimeError("write session metadata", err))
178199
return nil, 1
179200
}
180201

@@ -202,7 +223,7 @@ func executeTransport(runtimeState *runRuntime, preparedSession *preparedRunSess
202223
}
203224
preparedSession.meta.PID = pid
204225
if err := runtimeState.store.WriteMeta(preparedSession.meta); err != nil {
205-
return fmt.Errorf("write meta file: %w", err)
226+
return fmt.Errorf("failed to write session metadata file: %w", err)
206227
}
207228
return nil
208229
}
@@ -237,7 +258,7 @@ func executeTransport(runtimeState *runRuntime, preparedSession *preparedRunSess
237258
preparedSession.transportMode = contracts.DerunTransportModePipe
238259
preparedSession.meta.TransportMode = preparedSession.transportMode
239260
if err := runtimeState.store.WriteMeta(preparedSession.meta); err != nil {
240-
fmt.Fprintf(os.Stderr, "write fallback metadata: %v\n", err)
261+
fmt.Fprintln(os.Stderr, formatRuntimeError("write fallback metadata", err))
241262
return runExecutionResult{}, 1
242263
}
243264
result, runErr = runPipe()
@@ -281,7 +302,7 @@ func persistFinalState(runtimeState *runRuntime, preparedSession *preparedRunSes
281302
}
282303

283304
if err := runtimeState.store.WriteFinal(final); err != nil {
284-
fmt.Fprintf(os.Stderr, "write final metadata: %v\n", err)
305+
fmt.Fprintln(os.Stderr, formatRuntimeError("write final metadata", err))
285306
return 1
286307
}
287308

@@ -290,7 +311,7 @@ func persistFinalState(runtimeState *runRuntime, preparedSession *preparedRunSes
290311

291312
func resolveRunExitCode(execution runExecutionResult) int {
292313
if execution.runErr != nil {
293-
fmt.Fprintf(os.Stderr, "run command: %v\n", execution.runErr)
314+
fmt.Fprintln(os.Stderr, formatRuntimeError("execute command", execution.runErr))
294315
return 1
295316
}
296317
if execution.runResult.SignalNum > 0 {

‎cmds/derun/internal/mcp/errors.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
package mcp
2+
3+
import "fmt"
4+
5+
func formatUsageError(reason, hint string) string {
6+
if hint == "" {
7+
return fmt.Sprintf("invalid arguments: %s", reason)
8+
}
9+
return fmt.Sprintf("invalid arguments: %s; hint: %s", reason, hint)
10+
}
11+
12+
func requiredFieldError(field, expected string) error {
13+
if expected == "" {
14+
return fmt.Errorf("%s is required", field)
15+
}
16+
return fmt.Errorf("%s is required; expected %s", field, expected)
17+
}
18+
19+
func wrapRuntimeError(action string, err error) error {
20+
if err == nil {
21+
return fmt.Errorf("failed to %s", action)
22+
}
23+
return fmt.Errorf("failed to %s: %w", action, err)
24+
}

‎cmds/derun/internal/mcp/output_helpers.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ type outputPayloadOptions struct {
1919
func parseRequiredSessionID(args map[string]any) (string, error) {
2020
sessionID, ok := args["session_id"].(string)
2121
if !ok || sessionID == "" {
22-
return "", fmt.Errorf("session_id is required")
22+
return "", requiredFieldError("session_id", "a non-empty string")
2323
}
2424
return sessionID, nil
2525
}
@@ -28,15 +28,15 @@ func parseCursor(args map[string]any, required bool) (uint64, error) {
2828
rawCursor, exists := args["cursor"]
2929
if !exists {
3030
if required {
31-
return 0, fmt.Errorf("cursor is required")
31+
return 0, requiredFieldError("cursor", "a non-empty decimal string")
3232
}
3333
return 0, nil
3434
}
3535

3636
cursorString, ok := rawCursor.(string)
3737
if !ok || cursorString == "" {
3838
if required {
39-
return 0, fmt.Errorf("cursor is required")
39+
return 0, requiredFieldError("cursor", "a non-empty decimal string")
4040
}
4141
return 0, nil
4242
}

0 commit comments

Comments
 (0)