Skip to content

test(release): verify user-facing error contracts#690

Open
G4614 wants to merge 1 commit into
boxlite-ai:mainfrom
G4614:fix/rest-classify-exec-spawn-error
Open

test(release): verify user-facing error contracts#690
G4614 wants to merge 1 commit into
boxlite-ai:mainfrom
G4614:fix/rest-classify-exec-spawn-error

Conversation

@G4614

@G4614 G4614 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Add a release gate for actionable create and exec errors across the API, CLI, Python, Node, Go, and C SDKs.

Test plan:

  • python3 -m py_compile scripts/test/e2e/run_error_contracts.py
  • git diff --check
  • Debug Linux dev: make test:e2e:error-contracts (18 passed, 3 expected xfailed, 0 skipped)

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR improves error handling in BoxliteExec by implementing intelligent error classification. Previously, all execution-start failures returned HTTP 500. Now, typed SDK errors are mapped to specific status codes (400, 404, 409, 501, 429), with pattern-matching fallback for known error messages.

Changes

Error Classification Enhancement

Layer / File(s) Summary
Error classification implementation with SDK support
apps/runner/pkg/api/controllers/boxlite_exec.go
Imports strings and sdkboxlite to support typed error inspection. The classifyExecError function maps sdkboxlite.Error codes to appropriate HTTP status codes and adds pattern-based fallback for specific message formats before defaulting to 500. The BoxliteExec handler now calls classifyExecError(err) for status selection instead of always returning 500.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A rabbit hops through error states so clear,
Mapping codes to statuses, fixing fears,
No more all-500s for every mistake,
Just the right response for each path we take! 🎯

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description is missing the required template sections and does not clearly separate Summary, Changes, and How to verify. Rewrite the PR description to use the repository template with Summary, Changes, and How to verify sections, and add Risks/rollout only if needed.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is related to the user-facing error-contract work, though it emphasizes testing rather than the exec error-classification change.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@G4614
G4614 marked this pull request as ready for review June 9, 2026 12:05
@G4614
G4614 requested a review from a team as a code owner June 9, 2026 12:05
@G4614
G4614 enabled auto-merge June 9, 2026 12:05

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/runner/pkg/api/controllers/boxlite_exec.go`:
- Around line 238-255: The classifyExecError logic in boxlite_exec.go currently
maps several sdkboxlite error codes to HTTP statuses but misses
sdkboxlite.ErrSessionReaped; update the error switch inside classifyExecError
(the errors.As block that inspects *sdkboxlite.Error) to add a case for
sdkboxlite.ErrSessionReaped and return http.StatusGone (410); ensure you
reference the same bxErr variable and switch structure so all callers of
classifyExecError (Start, Signal, Kill, GetExecution, Resize) will correctly
return 410 instead of 500 for reaped sessions.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 8ac75ddb-c1ee-4eab-8322-9a8f62340012

📥 Commits

Reviewing files that changed from the base of the PR and between e4bbd0e and e659bbf.

📒 Files selected for processing (1)
  • apps/runner/pkg/api/controllers/boxlite_exec.go

Comment on lines +238 to +255
// Inspect typed boxlite SDK errors that don't have sentinel variants
// in the runner SDK wrapper. `ErrInvalidArgument` is the C-FFI shape
// for spawn-time validation failures the caller can fix.
var bxErr *sdkboxlite.Error
if errors.As(err, &bxErr) {
switch bxErr.Code {
case sdkboxlite.ErrInvalidArgument, sdkboxlite.ErrInvalidState:
return http.StatusBadRequest
case sdkboxlite.ErrNotFound:
return http.StatusNotFound
case sdkboxlite.ErrAlreadyExists:
return http.StatusConflict
case sdkboxlite.ErrUnsupported, sdkboxlite.ErrUnsupportedEngine:
return http.StatusNotImplemented
case sdkboxlite.ErrResourceExhausted:
return http.StatusTooManyRequests
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add mapping for ErrSessionReaped → HTTP 410 Gone.

The SDK defines ErrSessionReaped (code 21) with a documented server-side HTTP 410 status (see sdks/go/errors.go:39-42). Since classifyExecError is called by multiple endpoints beyond just Start() (Signal, Kill, GetExecution, Resize), it should comprehensively handle all SDK error codes with documented HTTP statuses to prevent returning 500 for this known client-facing scenario.

🔧 Proposed fix to add ErrSessionReaped mapping
 	var bxErr *sdkboxlite.Error
 	if errors.As(err, &bxErr) {
 		switch bxErr.Code {
 		case sdkboxlite.ErrInvalidArgument, sdkboxlite.ErrInvalidState:
 			return http.StatusBadRequest
 		case sdkboxlite.ErrNotFound:
 			return http.StatusNotFound
 		case sdkboxlite.ErrAlreadyExists:
 			return http.StatusConflict
 		case sdkboxlite.ErrUnsupported, sdkboxlite.ErrUnsupportedEngine:
 			return http.StatusNotImplemented
 		case sdkboxlite.ErrResourceExhausted:
 			return http.StatusTooManyRequests
+		case sdkboxlite.ErrSessionReaped:
+			return http.StatusGone
 		}
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Inspect typed boxlite SDK errors that don't have sentinel variants
// in the runner SDK wrapper. `ErrInvalidArgument` is the C-FFI shape
// for spawn-time validation failures the caller can fix.
var bxErr *sdkboxlite.Error
if errors.As(err, &bxErr) {
switch bxErr.Code {
case sdkboxlite.ErrInvalidArgument, sdkboxlite.ErrInvalidState:
return http.StatusBadRequest
case sdkboxlite.ErrNotFound:
return http.StatusNotFound
case sdkboxlite.ErrAlreadyExists:
return http.StatusConflict
case sdkboxlite.ErrUnsupported, sdkboxlite.ErrUnsupportedEngine:
return http.StatusNotImplemented
case sdkboxlite.ErrResourceExhausted:
return http.StatusTooManyRequests
}
}
// Inspect typed boxlite SDK errors that don't have sentinel variants
// in the runner SDK wrapper. `ErrInvalidArgument` is the C-FFI shape
// for spawn-time validation failures the caller can fix.
var bxErr *sdkboxlite.Error
if errors.As(err, &bxErr) {
switch bxErr.Code {
case sdkboxlite.ErrInvalidArgument, sdkboxlite.ErrInvalidState:
return http.StatusBadRequest
case sdkboxlite.ErrNotFound:
return http.StatusNotFound
case sdkboxlite.ErrAlreadyExists:
return http.StatusConflict
case sdkboxlite.ErrUnsupported, sdkboxlite.ErrUnsupportedEngine:
return http.StatusNotImplemented
case sdkboxlite.ErrResourceExhausted:
return http.StatusTooManyRequests
case sdkboxlite.ErrSessionReaped:
return http.StatusGone
}
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/runner/pkg/api/controllers/boxlite_exec.go` around lines 238 - 255, The
classifyExecError logic in boxlite_exec.go currently maps several sdkboxlite
error codes to HTTP statuses but misses sdkboxlite.ErrSessionReaped; update the
error switch inside classifyExecError (the errors.As block that inspects
*sdkboxlite.Error) to add a case for sdkboxlite.ErrSessionReaped and return
http.StatusGone (410); ensure you reference the same bxErr variable and switch
structure so all callers of classifyExecError (Start, Signal, Kill,
GetExecution, Resize) will correctly return 410 instead of 500 for reaped
sessions.

@G4614
G4614 force-pushed the fix/rest-classify-exec-spawn-error branch from e659bbf to e071a53 Compare July 23, 2026 12:12
@boxlite-agent

boxlite-agent Bot commented Jul 23, 2026

Copy link
Copy Markdown

📦 BoxLite review — couldn't complete

claude exited 1

stdout:
{"is_error":true,"duration_api_ms":0,"num_turns":1,"stop_reason":"stop_sequence","session_id":"79beab9e-c712-4185-97bb-d6a53ff3ecf1","total_cost_usd":0,"usage":{"input_tokens":0,"cache_creation_input_tokens":0,"cache_read_input_tokens":0,"output_tokens":0,"server_tool_use":{"web_search_requests":0,"web_fetch_requests":0},"service_tier":"standard","cache_creation":{"ephemeral_1h_input_tokens":0,"ephemeral_5m_input_tokens":0},"inference_geo":"","iterations":[],"speed":"standard"},"modelUsage":{},"permission_denials":[],"terminal_reason":"api_error","fast_mode_state":"off","subtype":"success","api_error_status":403,"result":"Your organization has disabled Claude subscription access for Claude Code · Use an Anthropic API key instead, or ask your admin to enable access","type":"result","duration_ms":329,"uuid":"c1fdfb1f-fec0-47d5-b4ed-46c1588e10b6"}

stderr:
<empty>

powered by BoxLite

@G4614
G4614 force-pushed the fix/rest-classify-exec-spawn-error branch from e071a53 to b880a76 Compare July 23, 2026 12:13
@G4614 G4614 changed the title fix(rest): classify exec spawn failures as 4xx, not 5xx test(release): verify user-facing error contracts Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant