cubeapi: map missing sandbox template to bad request on create

[YangYuS8]

Summary

• return a proper not-found error code from CubeMaster when sandbox creation references a missing template
• remove the previous CubeAPI-side string matching approach
• map the missing-template create-sandbox error according to the E2B API behavior for POST /sandboxes

Problem

When creating a sandbox with a non-existent template, CubeMaster previously returned a params error and CubeAPI ended up returning an incorrect HTTP response.

Fix

• classify the missing-template case in CubeMaster using the proper error code
• keep other template resolution failures as params errors
• update CubeAPI create_sandbox to rely on CubeMaster error codes instead of string matching
• map the POST /sandboxes missing-template case according to the E2B spec for this endpoint
  Closes [Bug Report] CubeAPI returns HTTP 500 instead of 400/404 when creating sandbox with non-existent template #10

Validation

• cd CubeAPI && cargo +1.88.0 test
• cd CubeAPI && cargo +1.88.0 check
• cd CubeMaster && go test ./pkg/templatecenter -run TestGetTemplateRequestAssignsRuntimeRequestID

Notes

• This PR intentionally keeps the fix scoped to sandbox creation.
• Existing unrelated test failures in CubeMaster/pkg/service/httpservice/cube were not expanded beyond the minimum necessary scope.

[YangYuS8]

The failed GitHub Action appears to come from the repository’s automated Claude review workflow permission check for external contributors. The log shows my actor permission is read, and the action exits with Actor does not have write permissions to the repository. The code change itself was validated locally with cargo +1.88.0 test and cargo +1.88.0 check.

[fslongjin]

This is poorly written. The corresponding error codes should be returned in Cubemaster, rather than using string matching to determine them.

[fslongjin]

Perhaps the return value should adhere to the conventions specified in https://github.com/e2b-dev/E2B/blob/main/spec/openapi.yml#L1949 ?

[YangYuS8]

Addressed the requested changes in the latest update:

• moved the missing-template error classification into CubeMaster so CubeAPI no longer relies on string matching
• kept CubeAPI mapped by CubeMaster error codes
• aligned POST /sandboxes to return 400 Bad Request for the missing-template case based on the E2B spec linked in review
• updated the PR title/body to match the current implementation

Validation run locally:

• cd CubeAPI && cargo +1.88.0 test
• cd CubeAPI && cargo +1.88.0 check

Please take another look when convenient. Thanks!

[chenhengqi]

• Please organize you commits logically, one commit per component.
• Please follow our AI-Generated Code Policy in contributing guide.

[YangYuS8]

I reorganized the PR into two logical commits, one for CubeMaster and one for CubeAPI. I reviewed the AI-assisted changes myself and added my own Signed-off-by trailers according to the project’s AI-Generated Code Policy and DCO requirements.

[chenhengqi]

Why do we need this change?

[YangYuS8]

This import is needed because sandbox_preview_test.go already uses CubeLog.RequestTrace{} in existing tests. Without it, the cube package tests fail to compile before reaching the new sandbox creation tests. I kept the change minimal to only add the missing import.

[chenhengqi]

Please group variables:

var (
...
)

[YangYuS8]

Updated. I grouped the test hook variables into a single var (...) block.

[YangYuS8]

Hi @chenhengqi, gentle ping on this PR.

I’ve addressed the latest review comments and kept the PR organized as two signed-off commits, one for CubeMaster and one for CubeAPI. Please let me know if there is anything else I should adjust. Thanks!

[chenhengqi]

/cubebot review this pr

[github-actions]

Claude finished @chenhengqi's task in 2m 38s —— View job

---

PR Review

Reviewed the two commits on fix-issue-10-missing-template-404. The change is well-scoped and correctly propagates a missing-template error from CubeMaster through to CubeAPI. Below are my findings.

---

CubeMaster (sandbox_create.go)

Correct error classification. The errors.Is(err, templatecenter.ErrTemplateNotFound) check at line 55 works correctly because dealCubeboxCreateReqWithTemplate wraps the error with %w at cubeboxutil.go:371, preserving the error chain for errors.Is to traverse through GetTemplateRequest → GetDefinition → ErrTemplateNotFound.

Test hook pattern is consistent. The function variables createSandboxDealCubeboxCreateReqWithTemplateFn and createSandboxRunFn follow the same testability pattern already used in sandbox_preview.go (lines 20-21). Grouped into a single var block as requested in prior review.

Minor observation — EnsureTemplateLocalityReady errors. When dealCubeboxCreateReqWithTemplateCenter fails at cubeboxutil.go:374 (EnsureTemplateLocalityReady), it returns a wrapped error like "template X is not ready on any healthy node: ...". This is not ErrTemplateNotFound, so it will correctly fall through to ErrorCode_MasterParamsError. This is the right behavior — template locality failure is a different class of error.

---

CubeAPI (sandboxes.rs)

Double is_not_found mapping is correct but redundant in one path. Lines 341-357 check is_not_found() in two places:

1. state.cubemaster.create_sandbox(&req).await.map_err(...) — the HTTP call to CubeMaster itself. This catches the case where CubeMaster returns a 130404 API error.
2. resp.ret.into_result().map_err(...) — the response body ret_code. This catches the same 130404 inside the response JSON.

Both paths map to AppError::BadRequest. This is correct: CubeMaster returns the error code in the response body (via rsp.Ret.RetCode = int(retCode)), and the CubeAPI create_sandbox client will parse it through RetCode::into_result(). The first .map_err on the HTTP call handles any transport-level or non-standard errors. The second handles the standard API error path. Both mapping to BadRequest is the right call per the E2B spec for POST /sandboxes.

E2B spec alignment. Per the E2B OpenAPI spec, POST /sandboxes returns 400 for invalid template IDs. Mapping to AppError::BadRequest (→ HTTP 400) is correct. Using AppError::NotFound (→ HTTP 404) would be wrong for this endpoint since the resource being created doesn't exist yet — it's a bad request parameter.

---

Tests (sandbox_create_test.go)

Good test coverage. Two test cases cover the key branches:

• TestCreateSandboxMapsMissingTemplateToNotFound — verifies ErrTemplateNotFound → ErrorCode_NotFound
• TestCreateSandboxKeepsOtherTemplateErrorsAsParamsError — verifies generic errors → ErrorCode_MasterParamsError

Test style is consistent with sandbox_preview_test.go. The save/restore pattern via t.Cleanup is correct.

One suggestion — consider a table-driven test. The two tests share identical setup boilerplate (save/restore fns, construct request, call createSandbox, type-assert response). A table-driven test would reduce duplication:

func TestCreateSandboxTemplateErrorMapping(t *testing.T) {
    tests := []struct {
        name        string
        templateErr error
        wantCode    errorcode.ErrorCode
    }{
        {"missing template maps to not found", templatecenter.ErrTemplateNotFound, errorcode.ErrorCode_NotFound},
        {"other errors map to params error", assert.AnError, errorcode.ErrorCode_MasterParamsError},
    }
    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            // ... shared setup ...
        })
    }
}

This is a style preference, not a blocker.

---

sandbox_preview_test.go import addition

The CubeLog import addition (line 19) is needed because the file already references CubeLog.RequestTrace{} in existing tests. This is a pre-existing missing import that the PR fixes as a side effect of adding the new test file to the same package. Minimal and correct.

---

Summary

The PR is well-structured with two logical commits (CubeMaster then CubeAPI). The error propagation chain is correct:

GetDefinition → ErrTemplateNotFound → (wrapped with %w) → dealCubeboxCreateReqWithTemplateCenter → errors.Is check → ErrorCode_NotFound (130404) → CubeAPI is_not_found() → AppError::BadRequest (400)

No blocking issues found. The one suggestion about table-driven tests is optional. LGTM.