From 378b62338926677616f4575e9fd2184f847c1455 Mon Sep 17 00:00:00 2001 From: Michael Stolarz Date: Mon, 20 Jul 2026 15:18:19 -0700 Subject: [PATCH 1/3] chore: restore toolkit lint baseline --- cli/apply.go | 14 ++++++++------ cli/core/sandbox_templates.go | 10 +++------- cli/deploy_integration_test.go | 12 +++++++----- cli/drive.go | 6 +++--- cli/push.go | 2 +- cli/run.go | 5 +++-- 6 files changed, 25 insertions(+), 24 deletions(-) diff --git a/cli/apply.go b/cli/apply.go index 3b66db7f..0d788740 100644 --- a/cli/apply.go +++ b/cli/apply.go @@ -773,15 +773,16 @@ func PutFn(resource *core.Resource, resourceName string, name string, resourceOb result.CallbackSecret = extractCallbackSecret(opResult.Response) } - if resourceName == "Preview" { + switch resourceName { + case "Preview": printPreviewURL(opResult.Response, resourceName, name, "configured") - } else if resourceName == "PreviewToken" { + case "PreviewToken": if tokenURL := buildPreviewTokenURL(opResult.Response, parentName, metadata); tokenURL != "" { core.Print(fmt.Sprintf("Resource %s:%s configured url=%s\n", resourceName, name, tokenURL)) } else { core.Print(fmt.Sprintf("Resource %s:%s configured\n", resourceName, name)) } - } else { + default: core.Print(fmt.Sprintf("Resource %s:%s configured\n", resourceName, name)) } @@ -817,15 +818,16 @@ func PostFn(resource *core.Resource, resourceName string, name string, resourceO result.CallbackSecret = extractCallbackSecret(opResult.Response) } - if resourceName == "Preview" { + switch resourceName { + case "Preview": printPreviewURL(opResult.Response, resourceName, name, "created") - } else if resourceName == "PreviewToken" { + case "PreviewToken": if tokenURL := buildPreviewTokenURL(opResult.Response, parentName, metadata); tokenURL != "" { core.Print(fmt.Sprintf("Resource %s:%s created url=%s\n", resourceName, name, tokenURL)) } else { core.Print(fmt.Sprintf("Resource %s:%s created\n", resourceName, name)) } - } else { + default: core.Print(fmt.Sprintf("Resource %s:%s created\n", resourceName, name)) } diff --git a/cli/core/sandbox_templates.go b/cli/core/sandbox_templates.go index 4ee20a27..979ba965 100644 --- a/cli/core/sandbox_templates.go +++ b/cli/core/sandbox_templates.go @@ -193,7 +193,6 @@ func PromptSandboxTemplateOptions(directory string, templates Templates) Templat func sandboxTemplatesForDisplay(templates Templates) Templates { knownTemplates := Templates{} - remainingTemplates := Templates{} for _, name := range []string{sandboxScratchTemplate, sandboxClaudeCodeTemplate, sandboxCodexTemplate} { for _, t := range templates { @@ -208,10 +207,7 @@ func sandboxTemplatesForDisplay(templates Templates) Templates { return knownTemplates } - for _, t := range templates { - remainingTemplates = append(remainingTemplates, t) - } - return remainingTemplates + return append(Templates{}, templates...) } func sandboxTemplateLabel(t Template) string { @@ -325,7 +321,7 @@ func removeSandboxFile(dir, name string) error { if err != nil { return fmt.Errorf("failed to open sandbox directory: %w", err) } - defer root.Close() + defer func() { _ = root.Close() }() return root.Remove(name) } @@ -338,7 +334,7 @@ func writeSandboxFile(dir, name string, data []byte, perm os.FileMode) error { if err != nil { return fmt.Errorf("failed to open sandbox directory: %w", err) } - defer root.Close() + defer func() { _ = root.Close() }() info, err := root.Lstat(name) if err == nil { diff --git a/cli/deploy_integration_test.go b/cli/deploy_integration_test.go index c77982d1..9529755b 100644 --- a/cli/deploy_integration_test.go +++ b/cli/deploy_integration_test.go @@ -988,7 +988,7 @@ func TestZipContainsDockerConfig(t *testing.T) { // Read the zip and verify .docker/config.json is present reader, err := zip.OpenReader(d.archive.Name()) require.NoError(t, err) - defer reader.Close() + defer func() { require.NoError(t, reader.Close()) }() var found bool for _, f := range reader.File { @@ -997,8 +997,9 @@ func TestZipContainsDockerConfig(t *testing.T) { rc, err := f.Open() require.NoError(t, err) data, err := io.ReadAll(rc) - rc.Close() + closeErr := rc.Close() require.NoError(t, err) + require.NoError(t, closeErr) var config core.DockerConfig require.NoError(t, json.Unmarshal(data, &config)) @@ -1039,7 +1040,7 @@ func TestZipExcludesRawDockerDir(t *testing.T) { reader, err := zip.OpenReader(d.archive.Name()) require.NoError(t, err) - defer reader.Close() + defer func() { require.NoError(t, reader.Close()) }() dockerConfigCount := 0 for _, f := range reader.File { @@ -1048,8 +1049,9 @@ func TestZipExcludesRawDockerDir(t *testing.T) { rc, err := f.Open() require.NoError(t, err) data, err := io.ReadAll(rc) - rc.Close() + closeErr := rc.Close() require.NoError(t, err) + require.NoError(t, closeErr) // Should contain the injected config, not the on-disk one var config core.DockerConfig @@ -1081,7 +1083,7 @@ func TestZipNoDockerConfigWhenNil(t *testing.T) { reader, err := zip.OpenReader(d.archive.Name()) require.NoError(t, err) - defer reader.Close() + defer func() { require.NoError(t, reader.Close()) }() for _, f := range reader.File { assert.NotEqual(t, ".docker/config.json", f.Name, "should not contain .docker/config.json when dockerConfigJSON is nil") diff --git a/cli/drive.go b/cli/drive.go index 73d61a6b..1e5cf700 100644 --- a/cli/drive.go +++ b/cli/drive.go @@ -175,7 +175,7 @@ the blfs filesystem. It can be used as a recovery tool when mounts are lost.`, core.PrintError("Drive mount", err) core.ExitWithError(err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() respBody, err := io.ReadAll(resp.Body) if err != nil { @@ -243,7 +243,7 @@ func DriveUnmountCmd() *cobra.Command { core.PrintError("Drive unmount", err) core.ExitWithError(err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() respBody, err := io.ReadAll(resp.Body) if err != nil { @@ -301,7 +301,7 @@ func DriveMountsCmd() *cobra.Command { core.PrintError("Drive mounts", err) core.ExitWithError(err) } - defer resp.Body.Close() + defer func() { _ = resp.Body.Close() }() respBody, err := io.ReadAll(resp.Body) if err != nil { diff --git a/cli/push.go b/cli/push.go index de04c0b3..7f8e3801 100644 --- a/cli/push.go +++ b/cli/push.go @@ -751,7 +751,7 @@ func renderCodeBlock(code string) string { var b strings.Builder b.WriteString(border.Sprint(" ┌─────────────────────────────────────────────────────────") + "\n") for _, line := range strings.Split(code, "\n") { - b.WriteString(fmt.Sprintf(" %s %s\n", border.Sprint("│"), codeColor.Sprint(line))) + fmt.Fprintf(&b, " %s %s\n", border.Sprint("│"), codeColor.Sprint(line)) } b.WriteString(border.Sprint(" └─────────────────────────────────────────────────────────")) return b.String() diff --git a/cli/run.go b/cli/run.go index 115192d4..7e654c65 100644 --- a/cli/run.go +++ b/cli/run.go @@ -335,11 +335,12 @@ This is useful for testing specific endpoints or non-standard API calls.`, fmt.Println() } // For JSON output, wrap the accumulated text - if outputFormat == "json" { + switch outputFormat { + case "json": result := map[string]string{"output": accumulated.String()} jsonData, _ := json.MarshalIndent(result, "", " ") fmt.Println(string(jsonData)) - } else if outputFormat == "yaml" { + case "yaml": result := map[string]string{"output": accumulated.String()} yamlData, _ := yaml.Marshal(result) fmt.Print(string(yamlData)) From 54d92fc1883c18426ce2a6e8834f3f8011868811 Mon Sep 17 00:00:00 2001 From: Michael Stolarz Date: Tue, 21 Jul 2026 10:25:04 -0700 Subject: [PATCH 2/3] fix(cli): classify and sanitize Sentry failures (ENG-4048) --- cli/apply.go | 47 +++- cli/auth/client_credentials.go | 2 +- cli/auth/device.go | 20 +- cli/auth/utils.go | 16 +- cli/auth/utils_integration_test.go | 11 + cli/cli_test.go | 30 +++ cli/connect.go | 25 +- cli/core/auth_source.go | 4 +- cli/core/buildenv.go | 15 +- cli/core/completion.go | 8 +- cli/core/create.go | 52 ++-- cli/core/create_test.go | 14 + cli/core/dockerconfig.go | 30 ++- cli/core/errors.go | 2 +- cli/core/root.go | 5 +- cli/core/sandbox_templates.go | 5 +- cli/core/sentry.go | 395 ++++++++++++++++++++++++++--- cli/core/sentry_test.go | 298 +++++++++++++++++++--- cli/core/templates.go | 22 +- cli/core/utils.go | 67 +++-- cli/delete.go | 30 ++- cli/deploy.go | 65 ++++- cli/drive.go | 51 ++-- cli/get.go | 26 +- cli/images.go | 66 +++-- cli/login.go | 5 +- cli/logs.go | 26 +- cli/monitor/logs.go | 5 +- cli/push.go | 39 ++- cli/push_test.go | 10 + cli/run.go | 38 ++- cli/serve.go | 5 +- cli/server/commands.go | 16 +- cli/server/commands_go.go | 5 +- cli/server/commands_python.go | 5 +- cli/server/commands_ts.go | 19 +- cli/server/serve_package.go | 2 +- cli/token.go | 10 +- 38 files changed, 1242 insertions(+), 249 deletions(-) diff --git a/cli/apply.go b/cli/apply.go index 0d788740..d6b261c8 100644 --- a/cli/apply.go +++ b/cli/apply.go @@ -27,6 +27,7 @@ type ResourceOperationResult struct { ErrorMsg string CallbackSecret string MetadataURL string + cause error } type ApplyResult struct { @@ -35,6 +36,20 @@ type ApplyResult struct { Result ResourceOperationResult } +func summarizeApplyFailures(results []ApplyResult) (hasFailures, allFailuresExpected bool) { + allFailuresExpected = true + for _, result := range results { + if result.Result.Status != "failed" { + continue + } + hasFailures = true + if !core.IsExpectedCLIError(result.Result.cause) { + allFailuresExpected = false + } + } + return hasFailures, allFailuresExpected +} + // ApplyOption defines a function type for apply options type ApplyOption func(*applyOptions) @@ -155,14 +170,9 @@ via -e flag for .env files or -s flag for command-line secrets.`, core.ExitWithError(err) } - // Check if any resources failed - hasFailures := false - for _, result := range applyResults { - if result.Result.Status == "failed" { - hasFailures = true - break - } - } + // Check if any resources failed without allowing one unexpected + // failure to be hidden by expected failures in the same manifest. + hasFailures, allFailuresExpected := summarizeApplyFailures(applyResults) outputFmt := core.GetOutputFormat() if outputFmt == "json" || outputFmt == "yaml" { @@ -170,7 +180,11 @@ via -e flag for .env files or -s flag for command-line secrets.`, } if hasFailures { - core.ExitWithError(fmt.Errorf("one or more resources failed to apply")) + err := fmt.Errorf("one or more resources failed to apply") + if allFailuresExpected { + err = core.MarkExpectedError(err, core.CLIErrorOperational) + } + core.ExitWithError(err) } }, } @@ -212,6 +226,10 @@ func ApplyResources(results []core.Result) ([]ApplyResult, error) { Result: ResourceOperationResult{ Status: "failed", ErrorMsg: fmt.Sprintf("metadata.%s is required", resource.ParentField), + cause: core.MarkExpectedError( + fmt.Errorf("metadata.%s is required", resource.ParentField), + core.CLIErrorValidation, + ), }, }) continue @@ -697,12 +715,14 @@ func PostThenPutFn(resource *core.Resource, resourceName string, name string, re return &ResourceOperationResult{ Status: "failed", ErrorMsg: errorMsg, + cause: err, } } if opResult == nil { return &ResourceOperationResult{ Status: "failed", ErrorMsg: "operation returned no result", + cause: fmt.Errorf("operation returned no result"), } } @@ -756,10 +776,15 @@ func PutFn(resource *core.Resource, resourceName string, name string, resourceOb return &ResourceOperationResult{ Status: "failed", ErrorMsg: errorMsg, + cause: err, } } if opResult == nil { - return nil + return &ResourceOperationResult{ + Status: "failed", + ErrorMsg: "operation returned no result", + cause: fmt.Errorf("operation returned no result"), + } } result := ResourceOperationResult{ @@ -798,12 +823,14 @@ func PostFn(resource *core.Resource, resourceName string, name string, resourceO return &ResourceOperationResult{ Status: "failed", ErrorMsg: errorMsg, + cause: err, } } if opResult == nil { return &ResourceOperationResult{ Status: "failed", ErrorMsg: "operation returned no result", + cause: fmt.Errorf("operation returned no result"), } } diff --git a/cli/auth/client_credentials.go b/cli/auth/client_credentials.go index 4a4374e8..ef77ce32 100644 --- a/cli/auth/client_credentials.go +++ b/cli/auth/client_credentials.go @@ -15,7 +15,7 @@ func LoginClientCredentials(workspace string, clientCredentials string) { err := validateWorkspace(workspace, creds) if err != nil { - err = fmt.Errorf("failed to access workspace '%s': %s", workspace, err) + err = fmt.Errorf("failed to access workspace '%s': %w", workspace, err) core.PrintError("Login", err) core.ExitWithError(err) } diff --git a/cli/auth/device.go b/cli/auth/device.go index 85ec8a7b..6ff67881 100644 --- a/cli/auth/device.go +++ b/cli/auth/device.go @@ -133,7 +133,10 @@ func deviceModeLoginFinalize(deviceCode string, workspace string, retries int) { deviceModeLoginFinalize(deviceCode, workspace, retries-1) return } else { - err := fmt.Errorf("login timed out waiting for confirmation") + err := core.MarkExpectedError( + fmt.Errorf("login timed out waiting for confirmation"), + core.CLIErrorOperational, + ) core.PrintError("Login", err) core.ExitWithError(err) } @@ -149,7 +152,10 @@ func deviceModeLoginFinalize(deviceCode string, workspace string, retries int) { deviceModeLoginFinalize(deviceCode, workspace, retries-1) return } else { - err := fmt.Errorf("login timed out waiting for confirmation") + err := core.MarkExpectedError( + fmt.Errorf("login timed out waiting for confirmation"), + core.CLIErrorOperational, + ) core.PrintError("Login", err) core.ExitWithError(err) } @@ -164,7 +170,10 @@ func deviceModeLoginFinalize(deviceCode string, workspace string, retries int) { if res.StatusCode != http.StatusOK { // This is a real error, not just pending - err := fmt.Errorf("authentication failed with status %d: %s", res.StatusCode, string(body)) + err := core.MarkExpectedError( + fmt.Errorf("authentication failed with status %d: %s", res.StatusCode, string(body)), + core.CLIErrorAuthentication, + ) core.PrintError("Login", err) core.ExitWithError(err) } @@ -186,7 +195,10 @@ func deviceModeLoginFinalize(deviceCode string, workspace string, retries int) { return } if len(workspaces) == 0 { - err := fmt.Errorf("no workspaces are available for your account.\nVisit %s to create one", blaxel.GetAppURL()) + err := core.MarkExpectedError( + fmt.Errorf("no workspaces are available for your account.\nVisit %s to create one", blaxel.GetAppURL()), + core.CLIErrorOperational, + ) core.PrintError("Login", err) core.ExitWithError(err) return diff --git a/cli/auth/utils.go b/cli/auth/utils.go index b9e7e190..15e8362c 100644 --- a/cli/auth/utils.go +++ b/cli/auth/utils.go @@ -8,6 +8,17 @@ import ( "github.com/blaxel-ai/sdk-go/option" ) +type workspaceValidationError struct { + workspace string + cause error +} + +func (e *workspaceValidationError) Error() string { + return fmt.Sprintf("permission denied for workspace %q", e.workspace) +} + +func (e *workspaceValidationError) Unwrap() error { return e.cause } + // WorkspaceClient interface for workspace lookups (allows mocking) type WorkspaceClient interface { Get(ctx context.Context, workspaceName string, opts ...option.RequestOption) (*blaxel.Workspace, error) @@ -72,8 +83,9 @@ func validateWorkspaceWithFactory(workspace string, credentials blaxel.Credentia // before the workspace is persisted as the current context. if workspace != "" { if _, err := client.Get(context.Background(), workspace); err != nil { - // Use one message for every explicit workspace validation failure. - return fmt.Errorf("permission denied for workspace %q", workspace) + // Keep the stable, non-sensitive user message while preserving the + // concrete cause for typed telemetry classification. + return &workspaceValidationError{workspace: workspace, cause: err} } return nil } diff --git a/cli/auth/utils_integration_test.go b/cli/auth/utils_integration_test.go index 595c7570..9eeacbe4 100644 --- a/cli/auth/utils_integration_test.go +++ b/cli/auth/utils_integration_test.go @@ -7,6 +7,7 @@ import ( blaxel "github.com/blaxel-ai/sdk-go" "github.com/blaxel-ai/sdk-go/option" + "github.com/blaxel-ai/toolkit/cli/core" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -134,6 +135,16 @@ func TestValidateWorkspaceError(t *testing.T) { require.Error(t, err) assert.Equal(t, "permission denied for workspace \"test-workspace\"", err.Error()) assert.NotContains(t, err.Error(), "API error") + assert.False(t, core.IsExpectedCLIError(err), "an untyped client failure must remain reportable") +} + +func TestValidateWorkspaceTypedAuthenticationErrorIsExpected(t *testing.T) { + factory := mockClientFactory(nil, &blaxel.Error{StatusCode: 403}) + + err := validateWorkspaceWithFactory("test-workspace", blaxel.Credentials{APIKey: "key"}, factory) + require.Error(t, err) + assert.Equal(t, "permission denied for workspace \"test-workspace\"", err.Error()) + assert.True(t, core.IsExpectedCLIError(err)) } // TestValidateWorkspaceMissingWorkspace tests explicit workspace validation failure wording. diff --git a/cli/cli_test.go b/cli/cli_test.go index 552394f8..6f395e1c 100644 --- a/cli/cli_test.go +++ b/cli/cli_test.go @@ -3,6 +3,7 @@ package cli import ( "context" "encoding/json" + "errors" "os" "path/filepath" "reflect" @@ -13,6 +14,35 @@ import ( "github.com/stretchr/testify/require" ) +func TestSummarizeApplyFailuresDoesNotHideUnexpectedFailure(t *testing.T) { + expected := core.MarkExpectedError(errors.New("invalid manifest"), core.CLIErrorValidation) + unexpected := errors.New("internal apply invariant failed") + + tests := []struct { + name string + results []ApplyResult + hasFailures bool + allFailuresExpected bool + }{ + {name: "success", results: []ApplyResult{{Result: ResourceOperationResult{Status: "created"}}}, allFailuresExpected: true}, + {name: "expected only", results: []ApplyResult{{Result: ResourceOperationResult{Status: "failed", cause: expected}}}, hasFailures: true, allFailuresExpected: true}, + {name: "unexpected only", results: []ApplyResult{{Result: ResourceOperationResult{Status: "failed", cause: unexpected}}}, hasFailures: true, allFailuresExpected: false}, + {name: "mixed", results: []ApplyResult{ + {Result: ResourceOperationResult{Status: "failed", cause: expected}}, + {Result: ResourceOperationResult{Status: "failed", cause: unexpected}}, + }, hasFailures: true, allFailuresExpected: false}, + {name: "missing cause", results: []ApplyResult{{Result: ResourceOperationResult{Status: "failed"}}}, hasFailures: true, allFailuresExpected: false}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + hasFailures, allFailuresExpected := summarizeApplyFailures(test.results) + assert.Equal(t, test.hasFailures, hasFailures) + assert.Equal(t, test.allFailuresExpected, allFailuresExpected) + }) + } +} + func TestGetCmd(t *testing.T) { cmd := GetCmd() diff --git a/cli/connect.go b/cli/connect.go index 38d68124..1f598a6c 100644 --- a/cli/connect.go +++ b/cli/connect.go @@ -58,7 +58,10 @@ Examples: // Check if stdin is a terminal if !term.IsTerminal(int(os.Stdin.Fd())) { - err := fmt.Errorf("this command requires an interactive terminal") + err := core.MarkExpectedError( + fmt.Errorf("this command requires an interactive terminal"), + core.CLIErrorUsage, + ) core.PrintError("Connect", err) core.ExitWithError(err) } @@ -67,7 +70,10 @@ Examples: currentContext, _ := blaxel.CurrentContext() workspace := currentContext.Workspace if workspace == "" { - err := fmt.Errorf("no workspace found in current context. Please run 'bl login' first") + err := core.MarkExpectedError( + fmt.Errorf("no workspace found in current context. Please run 'bl login' first"), + core.CLIErrorAuthentication, + ) core.PrintError("Connect", err) core.ExitWithError(err) } @@ -75,7 +81,10 @@ Examples: // Load credentials credentials, _ := blaxel.LoadCredentials(workspace) if !credentials.IsValid() { - err := fmt.Errorf("no valid credentials found. Please run 'bl login' first") + err := core.MarkExpectedError( + fmt.Errorf("no valid credentials found. Please run 'bl login' first"), + core.CLIErrorAuthentication, + ) core.PrintError("Connect", err) core.ExitWithError(err) } @@ -86,7 +95,10 @@ Examples: token = credentials.APIKey } if token == "" { - err := fmt.Errorf("no access token or Blaxel API key found. Please run 'bl login' first") + err := core.MarkExpectedError( + fmt.Errorf("no access token or Blaxel API key found. Please run 'bl login' first"), + core.CLIErrorAuthentication, + ) core.PrintError("Connect", err) core.ExitWithError(err) } @@ -97,7 +109,10 @@ Examples: if err != nil { var apiErr *blaxel.Error if isBlaxelError(err, &apiErr) && apiErr.StatusCode == 404 { - err := fmt.Errorf("sandbox '%s' not found", sandboxName) + err := core.MarkExpectedError( + fmt.Errorf("sandbox '%s' not found", sandboxName), + core.CLIErrorNotFound, + ) core.PrintError("Connect", err) // List available sandboxes diff --git a/cli/core/auth_source.go b/cli/core/auth_source.go index 7278302f..8615f10c 100644 --- a/cli/core/auth_source.go +++ b/cli/core/auth_source.go @@ -1,6 +1,7 @@ package core import ( + "errors" "fmt" "os" "strings" @@ -73,7 +74,8 @@ func IsAuthError(err error) bool { return false } // Try the SDK concrete type first. - if e, ok := err.(*blaxel.Error); ok { + var e *blaxel.Error + if errors.As(err, &e) { return e.StatusCode == 401 || e.StatusCode == 403 } msg := strings.ToLower(err.Error()) diff --git a/cli/core/buildenv.go b/cli/core/buildenv.go index 343eefd2..8bb75830 100644 --- a/cli/core/buildenv.go +++ b/cli/core/buildenv.go @@ -35,7 +35,10 @@ func ReadBuildEnv(projectDir string, customPath string) (map[string]string, erro return nil, nil } if os.IsNotExist(err) && required { - return nil, fmt.Errorf(".env.build file not found: %s", filePath) + return nil, MarkExpectedError( + fmt.Errorf(".env.build file not found: %s", filePath), + CLIErrorNotFound, + ) } return nil, fmt.Errorf("failed to read .env.build file: %w", err) } @@ -59,14 +62,20 @@ func parseBuildEnv(content string) (map[string]string, error) { eqIdx := strings.Index(line, "=") if eqIdx < 0 { - return nil, fmt.Errorf(".env.build line %d: invalid format (expected KEY=VALUE): %s", lineNum, line) + return nil, MarkExpectedError( + fmt.Errorf(".env.build line %d: invalid format (expected KEY=VALUE): %s", lineNum, line), + CLIErrorValidation, + ) } key := strings.TrimSpace(line[:eqIdx]) value := strings.TrimSpace(line[eqIdx+1:]) if key == "" { - return nil, fmt.Errorf(".env.build line %d: empty key", lineNum) + return nil, MarkExpectedError( + fmt.Errorf(".env.build line %d: empty key", lineNum), + CLIErrorValidation, + ) } args[key] = value diff --git a/cli/core/completion.go b/cli/core/completion.go index 9fa68b47..7982cd60 100644 --- a/cli/core/completion.go +++ b/cli/core/completion.go @@ -11,7 +11,8 @@ import ( // bashCompletionShim provides a fallback implementation of _get_comp_words_by_ref // for systems (like macOS with bash 3.2) that don't have bash-completion installed. // Without this, cobra's generated bash completion fails with: -// _get_comp_words_by_ref: command not found +// +// _get_comp_words_by_ref: command not found const bashCompletionShim = `# Shim: provide _get_comp_words_by_ref if bash-completion is not installed. # This allows completions to work on macOS default bash (3.2) without # requiring 'brew install bash-completion'. @@ -98,7 +99,10 @@ PowerShell: case "powershell": return cmd.Root().GenPowerShellCompletionWithDesc(os.Stdout) default: - return fmt.Errorf("unsupported shell: %s", args[0]) + return MarkExpectedError( + fmt.Errorf("unsupported shell: %s", args[0]), + CLIErrorValidation, + ) } }, } diff --git a/cli/core/create.go b/cli/core/create.go index 0ff1fb85..bbf7fc94 100644 --- a/cli/core/create.go +++ b/cli/core/create.go @@ -126,12 +126,11 @@ func runCreateFlowWithDeps( dirArg = generateRandomDirectoryName(cfg.TemplateType) } - // If directory arg provided, ensure it doesn't already exist + // If directory arg provided, ensure it doesn't already exist. if dirArg != "" { - if _, err := os.Stat(dirArg); !os.IsNotExist(err) { - createErr := fmt.Errorf("directory '%s' already exists", dirArg) - PrintError(cfg.ErrorPrefix, createErr) - return createErr + if err := ensureCreateDirectoryAvailable(dirArg); err != nil { + PrintError(cfg.ErrorPrefix, err) + return err } } @@ -150,14 +149,16 @@ func runCreateFlowWithDeps( if selectedDir == "" { selectedDir = templateNameFlag } - if _, err := os.Stat(selectedDir); !os.IsNotExist(err) { - createErr := fmt.Errorf("directory '%s' already exists", selectedDir) - PrintError(cfg.ErrorPrefix, createErr) - return createErr + if err := ensureCreateDirectoryAvailable(selectedDir); err != nil { + PrintError(cfg.ErrorPrefix, err) + return err } opts = CreateDefaultTemplateOptions(selectedDir, templateNameFlag, templates) if opts.TemplateName == "" { - createErr := fmt.Errorf("template '%s' not found", templateNameFlag) + createErr := MarkExpectedError( + fmt.Errorf("template '%s' not found", templateNameFlag), + CLIErrorNotFound, + ) PrintError(cfg.ErrorPrefix, createErr) printAvailableTemplates(templates, cfg.TemplateType) return createErr @@ -165,7 +166,10 @@ func runCreateFlowWithDeps( case cfg.NoTTY && cfg.TemplateType == "mcp": // Special-case retained behavior: for MCP with --yes but no template we require directory and pick default if dirArg == "" { - createErr := fmt.Errorf("directory name is required") + createErr := MarkExpectedError( + fmt.Errorf("directory name is required"), + CLIErrorUsage, + ) PrintError(cfg.ErrorPrefix, createErr) return createErr } @@ -175,14 +179,16 @@ func runCreateFlowWithDeps( opts = promptFunc(dirArg, templates) // Safety checks if opts.Directory == "" { - createErr := fmt.Errorf("directory name is required") + createErr := MarkExpectedError( + fmt.Errorf("directory name is required"), + CLIErrorUsage, + ) PrintError(cfg.ErrorPrefix, createErr) return createErr } - if _, err := os.Stat(opts.Directory); !os.IsNotExist(err) { - createErr := fmt.Errorf("directory '%s' already exists", opts.Directory) - PrintError(cfg.ErrorPrefix, createErr) - return createErr + if err := ensureCreateDirectoryAvailable(opts.Directory); err != nil { + PrintError(cfg.ErrorPrefix, err) + return err } } @@ -233,6 +239,20 @@ func runCreateFlowWithDeps( return nil } +func ensureCreateDirectoryAvailable(directory string) error { + _, err := os.Stat(directory) + if err == nil { + return MarkExpectedError( + fmt.Errorf("directory '%s' already exists", directory), + CLIErrorConflict, + ) + } + if os.IsNotExist(err) { + return nil + } + return fmt.Errorf("failed to inspect directory %q: %w", directory, err) +} + func normalizeTemplateNameFlag(templateNameFlag string, templateType string) string { if templateType == "sandbox" { if templateName, ok := sandboxTemplateAlias(templateNameFlag); ok { diff --git a/cli/core/create_test.go b/cli/core/create_test.go index 1475d764..35f00734 100644 --- a/cli/core/create_test.go +++ b/cli/core/create_test.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" "regexp" + "strings" "testing" blaxel "github.com/blaxel-ai/sdk-go" @@ -167,6 +168,19 @@ func TestRunCreateFlowWithDepsReturnsErrorWhenDirectoryExists(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "already exists") + assert.True(t, IsExpectedCLIError(err)) +} + +func TestEnsureCreateDirectoryAvailableDoesNotMislabelInspectionFailureAsConflict(t *testing.T) { + tooLong := filepath.Join(t.TempDir(), strings.Repeat("x", 5000)) + + err := ensureCreateDirectoryAvailable(tooLong) + + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to inspect directory") + classification := classifyCLIError(err) + assert.True(t, classification.expected) + assert.Equal(t, CLIErrorOperational, classification.category) } func TestRunCreateFlowWithDepsReturnsErrorWhenTemplateNotFound(t *testing.T) { diff --git a/cli/core/dockerconfig.go b/cli/core/dockerconfig.go index cca019d9..4595fe2b 100644 --- a/cli/core/dockerconfig.go +++ b/cli/core/dockerconfig.go @@ -25,23 +25,38 @@ type DockerConfig struct { func ParseRegistryCred(cred string) (string, string, error) { registry, userPass, found := strings.Cut(cred, "=") if !found { - return "", "", fmt.Errorf("invalid registry credential format: expected registry=username:password") + return "", "", MarkExpectedError( + fmt.Errorf("invalid registry credential format: expected registry=username:password"), + CLIErrorValidation, + ) } if registry == "" { - return "", "", fmt.Errorf("invalid registry credential: registry cannot be empty") + return "", "", MarkExpectedError( + fmt.Errorf("invalid registry credential: registry cannot be empty"), + CLIErrorValidation, + ) } username, password, found := strings.Cut(userPass, ":") if !found { - return "", "", fmt.Errorf("invalid registry credential: expected username:password after '='") + return "", "", MarkExpectedError( + fmt.Errorf("invalid registry credential: expected username:password after '='"), + CLIErrorValidation, + ) } if username == "" { - return "", "", fmt.Errorf("invalid registry credential: username cannot be empty") + return "", "", MarkExpectedError( + fmt.Errorf("invalid registry credential: username cannot be empty"), + CLIErrorValidation, + ) } if password == "" { - return "", "", fmt.Errorf("invalid registry credential: password cannot be empty") + return "", "", MarkExpectedError( + fmt.Errorf("invalid registry credential: password cannot be empty"), + CLIErrorValidation, + ) } auth := base64.StdEncoding.EncodeToString([]byte(userPass)) @@ -57,7 +72,10 @@ func LoadDockerConfigFile(path string) (*DockerConfig, error) { var config DockerConfig if err := json.Unmarshal(data, &config); err != nil { - return nil, fmt.Errorf("failed to parse docker config file: %w", err) + return nil, MarkExpectedError( + fmt.Errorf("failed to parse docker config file: %w", err), + CLIErrorValidation, + ) } if config.Auths == nil { diff --git a/cli/core/errors.go b/cli/core/errors.go index c49b49b7..1884b47d 100644 --- a/cli/core/errors.go +++ b/cli/core/errors.go @@ -45,5 +45,5 @@ func ErrorHandler(request *http.Request, kind string, name string, body string) } err = fmt.Errorf("%s", errMsg) } - return err + return MarkExpectedHTTPError(err, errorModel.Code) } diff --git a/cli/core/root.go b/cli/core/root.go index 9c376ad8..0b123227 100644 --- a/cli/core/root.go +++ b/cli/core/root.go @@ -233,6 +233,9 @@ var rootCmd = &cobra.Command{ Use: "bl", Short: "Blaxel CLI - manage and deploy AI agents, sandboxes, and resources", PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + // Command paths contain only registered command names, never user args. + SetSentryTag("command.class", cmd.CommandPath()) + // Skip version warning for specific commands/conditions shouldSkipWarning := skipVersionWarning || cmd.Name() == "__complete" || @@ -424,7 +427,7 @@ func Execute(releaseVersion string, releaseCommit string, releaseDate string) er SetSentryTag("version", version) SetSentryTag("commit", commit) - SetSentryTag("workspace", workspace) + SetSentryTag("command.class", "bl command-resolution") return rootCmd.Execute() } diff --git a/cli/core/sandbox_templates.go b/cli/core/sandbox_templates.go index 979ba965..d1d595ed 100644 --- a/cli/core/sandbox_templates.go +++ b/cli/core/sandbox_templates.go @@ -356,7 +356,10 @@ func writeSandboxFile(dir, name string, data []byte, perm os.FileMode) error { func validateSandboxFileName(name string) error { if name == "" || name == "." || filepath.IsAbs(name) || name != filepath.Base(name) { - return fmt.Errorf("invalid sandbox file target %q", name) + return MarkExpectedError( + fmt.Errorf("invalid sandbox file target %q", name), + CLIErrorValidation, + ) } return nil } diff --git a/cli/core/sentry.go b/cli/core/sentry.go index d721dd5f..ba0f6a01 100644 --- a/cli/core/sentry.go +++ b/cli/core/sentry.go @@ -1,45 +1,337 @@ package core import ( + "context" + "errors" + "flag" + "fmt" + "io/fs" + "net" + "net/http" "os" + "os/exec" + "path" + "regexp" + "strings" "time" + blaxel "github.com/blaxel-ai/sdk-go" + "github.com/charmbracelet/huh" "github.com/getsentry/sentry-go" + "github.com/gorilla/websocket" + "github.com/spf13/pflag" ) -// SentryDSN is the default Sentry DSN for the CLI +// SentryDSN is the default Sentry DSN for the CLI. var SentryDSN = "" -// SentryConfig holds the configuration for Sentry initialization +// SentryConfig holds the configuration for Sentry initialization. type SentryConfig struct { DSN string Release string } -// InitSentry initializes the Sentry SDK with the given configuration -func InitSentry(cfg SentryConfig) error { - SentryDSN = cfg.DSN - if SentryDSN == "" { +// CLIErrorCategory is a stable, non-sensitive telemetry classification. +type CLIErrorCategory string + +const ( + CLIErrorUsage CLIErrorCategory = "usage" + CLIErrorValidation CLIErrorCategory = "validation" + CLIErrorAuthentication CLIErrorCategory = "authentication" + CLIErrorNotFound CLIErrorCategory = "not_found" + CLIErrorConflict CLIErrorCategory = "conflict" + CLIErrorOperational CLIErrorCategory = "operational" + CLIErrorInternal CLIErrorCategory = "internal" + CLIErrorPanic CLIErrorCategory = "panic" +) + +type classifiedCLIError struct { + category CLIErrorCategory + cause error +} + +func (e *classifiedCLIError) Error() string { return e.cause.Error() } +func (e *classifiedCLIError) Unwrap() error { return e.cause } + +func isExpectedCategory(category CLIErrorCategory) bool { + switch category { + case CLIErrorUsage, + CLIErrorValidation, + CLIErrorAuthentication, + CLIErrorNotFound, + CLIErrorConflict, + CLIErrorOperational: + return true + default: + return false + } +} + +// MarkExpectedError preserves the local error while explicitly excluding a +// handled usage, validation, authentication, or operational failure from +// error-level Sentry reporting. +func MarkExpectedError(err error, category CLIErrorCategory) error { + if err == nil { return nil } - environment := os.Getenv("BL_ENV") - if environment == "" { - environment = "prod" + if !isExpectedCategory(category) { + return err + } + return &classifiedCLIError{category: category, cause: err} +} + +// MarkExpectedHTTPError classifies a handled HTTP response without changing +// the error text shown to the user. +func MarkExpectedHTTPError(err error, statusCode int) error { + if err == nil || statusCode < http.StatusBadRequest { + return err + } + return MarkExpectedError(err, categoryForHTTPStatus(statusCode)) +} + +type sentryCLIError struct { + category CLIErrorCategory +} + +func (e *sentryCLIError) Error() string { + return fmt.Sprintf("unexpected CLI failure (%s)", e.category) +} + +type errorClassification struct { + category CLIErrorCategory + expected bool +} + +var ( + cobraArgumentError = regexp.MustCompile( + `^(?:accepts (?:at most )?\d+ arg\(s\)|accepts between \d+ and \d+ arg\(s\)|requires at least \d+ arg\(s\))(?:, .*)?$`, + ) + cobraRequiredFlagError = regexp.MustCompile(`^required flag\(s\) ".+" not set$`) + cobraUnknownCommand = regexp.MustCompile(`^unknown command ".+" for ".+"$`) +) + +func categoryForHTTPStatus(statusCode int) CLIErrorCategory { + switch statusCode { + case 400, 405, 406, 411, 413, 414, 415, 422: + return CLIErrorValidation + case 401, 403: + return CLIErrorAuthentication + case 404, 410: + return CLIErrorNotFound + case 409, 412: + return CLIErrorConflict + default: + return CLIErrorOperational + } +} + +func classifyCLIError(err error) errorClassification { + if err == nil { + return errorClassification{category: CLIErrorInternal, expected: true} + } + + var classified *classifiedCLIError + if errors.As(err, &classified) { + return errorClassification{category: classified.category, expected: true} + } + + var apiError *blaxel.Error + if errors.As(err, &apiError) && apiError.StatusCode >= 400 { + return errorClassification{ + category: categoryForHTTPStatus(apiError.StatusCode), + expected: true, + } + } + + var unknownFlag *pflag.NotExistError + var invalidFlag *pflag.InvalidValueError + var missingFlagValue *pflag.ValueRequiredError + var invalidFlagSyntax *pflag.InvalidSyntaxError + if errors.As(err, &unknownFlag) || + errors.As(err, &invalidFlag) || + errors.As(err, &missingFlagValue) || + errors.As(err, &invalidFlagSyntax) || + errors.Is(err, flag.ErrHelp) { + return errorClassification{category: CLIErrorUsage, expected: true} + } + + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return errorClassification{category: CLIErrorOperational, expected: true} + } + if errors.Is(err, huh.ErrUserAborted) { + return errorClassification{category: CLIErrorUsage, expected: true} + } + if errors.Is(err, websocket.ErrBadHandshake) { + return errorClassification{category: CLIErrorOperational, expected: true} + } + + if errors.Is(err, fs.ErrNotExist) { + return errorClassification{category: CLIErrorNotFound, expected: true} + } + if errors.Is(err, fs.ErrExist) { + return errorClassification{category: CLIErrorConflict, expected: true} + } + if errors.Is(err, fs.ErrPermission) { + return errorClassification{category: CLIErrorOperational, expected: true} + } + + var pathError *os.PathError + if errors.As(err, &pathError) { + category := CLIErrorOperational + if errors.Is(pathError, fs.ErrNotExist) { + category = CLIErrorNotFound + } + return errorClassification{category: category, expected: true} } - err := sentry.Init(sentry.ClientOptions{ - Dsn: SentryDSN, - Environment: environment, + var networkError net.Error + if errors.As(err, &networkError) { + return errorClassification{category: CLIErrorOperational, expected: true} + } + + var processExit *exec.ExitError + if errors.As(err, &processExit) { + return errorClassification{category: CLIErrorOperational, expected: true} + } + var processStart *exec.Error + if errors.As(err, &processStart) { + category := CLIErrorOperational + if errors.Is(processStart, fs.ErrNotExist) { + category = CLIErrorNotFound + } + return errorClassification{category: category, expected: true} + } + + message := strings.TrimSpace(err.Error()) + if cobraArgumentError.MatchString(message) || + cobraRequiredFlagError.MatchString(message) || + cobraUnknownCommand.MatchString(message) { + return errorClassification{category: CLIErrorUsage, expected: true} + } + + return errorClassification{category: CLIErrorInternal, expected: false} +} + +// IsExpectedCLIError reports whether an error has a typed or explicitly +// marked expected classification. It does not inspect broad message fragments. +func IsExpectedCLIError(err error) bool { + return err != nil && classifyCLIError(err).expected +} + +func sanitizeSentryEvent(event *sentry.Event, _ *sentry.EventHint) *sentry.Event { + if event == nil { + return nil + } + + // Host/user/request context is not needed to diagnose a CLI implementation + // defect and can contain machine or resource identifiers. + event.ServerName = "" + event.User = sentry.User{} + event.Request = nil + event.Breadcrumbs = nil + event.Contexts = nil + event.Modules = nil + event.Threads = nil + event.Message = "" + event.Transaction = "" + event.Logger = "" + event.Dist = "" + event.Fingerprint = nil + event.DebugMeta = nil + event.Attachments = nil + event.Type = "" + event.StartTime = time.Time{} + event.Spans = nil + event.TransactionInfo = nil + event.CheckIn = nil + event.MonitorConfig = nil + event.Logs = nil + event.Metrics = nil + event.Environment = normalizeEnvironment(event.Environment) + event.Release = sanitizeTagValue(event.Release) + event.Platform = "go" + event.Level = sentry.LevelError + + allowedTags := map[string]struct{}{ + "version": {}, + "commit": {}, + "command.class": {}, + "error.category": {}, + } + safeTags := make(map[string]string, len(event.Tags)) + for key, value := range event.Tags { + if _, ok := allowedTags[key]; ok { + safeTags[key] = sanitizeTagValue(value) + } + } + event.Tags = safeTags + + for exceptionIndex := range event.Exception { + exception := &event.Exception[exceptionIndex] + exception.Type = "CLIInternalError" + exception.Value = "Unexpected CLI failure" + exception.Module = "github.com/blaxel-ai/toolkit/cli/core" + exception.ThreadID = 0 + exception.Mechanism = nil + if exception.Stacktrace == nil { + continue + } + + safeFrames := make([]sentry.Frame, 0, len(exception.Stacktrace.Frames)) + for _, frame := range exception.Stacktrace.Frames { + if frame.Module != "github.com/blaxel-ai/toolkit" && + !strings.HasPrefix(frame.Module, "github.com/blaxel-ai/toolkit/") { + continue + } + safeFrames = append(safeFrames, sentry.Frame{ + Function: frame.Function, + Module: frame.Module, + Filename: path.Base(strings.ReplaceAll(frame.Filename, "\\", "/")), + Lineno: frame.Lineno, + Colno: frame.Colno, + InApp: true, + }) + } + exception.Stacktrace.Frames = safeFrames + exception.Stacktrace.FramesOmitted = nil + } + + return event +} + +func normalizeEnvironment(environment string) string { + if environment == "dev" { + return "dev" + } + return "prod" +} + +// InitSentry initializes the SDK with a minimal, allowlisted event surface. +func InitSentry(cfg SentryConfig) error { + if cfg.DSN == "" { + SentryDSN = "" + return nil + } + + if err := sentry.Init(sentry.ClientOptions{ + Dsn: cfg.DSN, + Environment: normalizeEnvironment(os.Getenv("BL_ENV")), Release: cfg.Release, AttachStacktrace: true, - }) - if err != nil { + BeforeSend: sanitizeSentryEvent, + SendDefaultPII: false, + MaxBreadcrumbs: -1, + MaxErrorDepth: 1, + }); err != nil { + SentryDSN = "" return err } + + SentryDSN = cfg.DSN return nil } -// FlushSentry flushes buffered events before the program exits +// FlushSentry flushes buffered events before the program exits. func FlushSentry(timeout time.Duration) { if SentryDSN == "" { return @@ -47,62 +339,83 @@ func FlushSentry(timeout time.Duration) { sentry.Flush(timeout) } -// CaptureException captures an error and sends it to Sentry +func captureUnexpectedError(err error) bool { + classification := classifyCLIError(err) + if err == nil || classification.expected || SentryDSN == "" { + return false + } + + sentry.WithScope(func(scope *sentry.Scope) { + scope.SetTag("error.category", string(classification.category)) + sentry.CaptureException(&sentryCLIError{category: classification.category}) + }) + return true +} + +// CaptureException reports only unexpected CLI defects and never forwards the +// original error string or wrapped error chain. func CaptureException(err error) { - if err == nil { - return + captureUnexpectedError(err) +} + +var safeTagValue = regexp.MustCompile(`^[A-Za-z0-9._+ -]+$`) + +func sanitizeTagValue(value string) string { + value = strings.TrimSpace(value) + if value == "" || len(value) > 80 || !safeTagValue.MatchString(value) { + return "unknown" } - sentry.CaptureException(err) + return value } -// SetSentryTag sets a tag on the current scope +// SetSentryTag accepts only build-controlled or command-taxonomy tags. func SetSentryTag(key, value string) { if SentryDSN == "" { return } - sentry.ConfigureScope(func(scope *sentry.Scope) { - scope.SetTag(key, value) - }) + + switch key { + case "version", "commit", "command.class": + sentry.ConfigureScope(func(scope *sentry.Scope) { + scope.SetTag(key, sanitizeTagValue(value)) + }) + } } -// RecoverWithSentry recovers from a panic and sends it to Sentry -// Usage: defer core.RecoverWithSentry() +// RecoverWithSentry reports a sanitized panic category, then preserves Go's +// original panic behavior and value for the local process. func RecoverWithSentry() { if SentryDSN == "" { return } - if r := recover(); r != nil { - sentry.CurrentHub().Recover(r) + if recovered := recover(); recovered != nil { + sentry.WithScope(func(scope *sentry.Scope) { + scope.SetTag("error.category", string(CLIErrorPanic)) + sentry.CaptureException(&sentryCLIError{category: CLIErrorPanic}) + }) sentry.Flush(2 * time.Second) - panic(r) // Re-panic after capturing + panic(recovered) } } -// ExitWithError captures the error to Sentry and exits with code 1. -// When the error looks like an auth failure it also prints a hint about -// the credential source (env var vs config file) so the user can spot -// stale or mismatched credentials immediately. +// ExitWithError preserves local hints and exit code while reporting only +// sanitized, unexpected implementation defects. func ExitWithError(err error) { if IsAuthError(err) { PrintAuthSourceHint() } - if err != nil && SentryDSN != "" { - sentry.CaptureException(err) + if captureUnexpectedError(err) { sentry.Flush(2 * time.Second) } os.Exit(1) } -// ExitWithMessage captures a message to Sentry and exits with code 1 -func ExitWithMessage(msg string) { - if msg != "" && SentryDSN != "" { - sentry.CaptureMessage(msg) - sentry.Flush(2 * time.Second) - } +// ExitWithMessage is a user-facing expected exit and is never error telemetry. +func ExitWithMessage(_ string) { os.Exit(1) } -// Exit captures to Sentry and exits with the given code +// Exit exits with the given code after flushing any previously queued event. func Exit(code int) { if code != 0 && SentryDSN != "" { sentry.Flush(2 * time.Second) diff --git a/cli/core/sentry_test.go b/cli/core/sentry_test.go index b4858c09..625f90fe 100644 --- a/cli/core/sentry_test.go +++ b/cli/core/sentry_test.go @@ -1,66 +1,306 @@ package core import ( + "context" + "encoding/json" "errors" + "fmt" + "io/fs" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" "testing" "time" + blaxel "github.com/blaxel-ai/sdk-go" + "github.com/charmbracelet/huh" + "github.com/getsentry/sentry-go" + "github.com/gorilla/websocket" + "github.com/spf13/cobra" + "github.com/spf13/pflag" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestSentryConfigStruct(t *testing.T) { - cfg := SentryConfig{ - DSN: "https://test@sentry.io/123", - Release: "v1.0.0", - } +const testSentryDSN = "https://public@example.com/123" + +func bindMockSentry(t *testing.T) *sentry.MockTransport { + t.Helper() + + transport := &sentry.MockTransport{} + client, err := sentry.NewClient(sentry.ClientOptions{ + Dsn: testSentryDSN, + Release: "0.1.106-test", + Environment: "prod", + AttachStacktrace: true, + BeforeSend: sanitizeSentryEvent, + Transport: transport, + }) + require.NoError(t, err) + + hub := sentry.CurrentHub() + originalDSN := SentryDSN + hub.PushScope() + hub.BindClient(client) + SentryDSN = testSentryDSN + t.Cleanup(func() { + hub.PopScope() + SentryDSN = originalDSN + }) + + return transport +} - assert.Equal(t, "https://test@sentry.io/123", cfg.DSN) +func unknownFlagError(t *testing.T) error { + t.Helper() + flags := pflag.NewFlagSet("test", pflag.ContinueOnError) + return flags.Parse([]string{"--definitely-unknown"}) +} + +func missingFlagValueError(t *testing.T) error { + t.Helper() + flags := pflag.NewFlagSet("test", pflag.ContinueOnError) + flags.String("name", "", "test flag") + return flags.Parse([]string{"--name"}) +} + +func TestSentryConfigStruct(t *testing.T) { + cfg := SentryConfig{DSN: testSentryDSN, Release: "v1.0.0"} + assert.Equal(t, testSentryDSN, cfg.DSN) assert.Equal(t, "v1.0.0", cfg.Release) } func TestInitSentryWithEmptyDSN(t *testing.T) { - cfg := SentryConfig{ - DSN: "", - Release: "v1.0.0", - } + err := InitSentry(SentryConfig{Release: "v1.0.0"}) + require.NoError(t, err) + assert.Empty(t, SentryDSN) +} - err := InitSentry(cfg) - assert.NoError(t, err) +func TestInitSentryDoesNotEnableCaptureWhenInitializationFails(t *testing.T) { + err := InitSentry(SentryConfig{DSN: "://invalid", Release: "v1.0.0"}) + require.Error(t, err) assert.Empty(t, SentryDSN) } -func TestFlushSentryWithEmptyDSN(t *testing.T) { - // Reset DSN - SentryDSN = "" +func TestNormalizeEnvironment(t *testing.T) { + assert.Equal(t, "dev", normalizeEnvironment("dev")) + assert.Equal(t, "prod", normalizeEnvironment("prod")) + assert.Equal(t, "prod", normalizeEnvironment("customer-workspace")) +} - // Should not panic - FlushSentry(time.Second) +func TestExpectedErrorClassification(t *testing.T) { + missingFile := &os.PathError{Op: "open", Path: "/private/customer/file", Err: os.ErrNotExist} + _, missingManifest := GetResults("apply", filepath.Join(t.TempDir(), "missing.yaml"), false) + require.Error(t, missingManifest) + apiNotFound := &blaxel.Error{StatusCode: 404} + positional := cobra.ExactArgs(1)(&cobra.Command{Use: "test"}, nil) + + tests := []struct { + name string + err error + category CLIErrorCategory + }{ + {name: "unknown flag", err: unknownFlagError(t), category: CLIErrorUsage}, + {name: "missing flag value", err: missingFlagValueError(t), category: CLIErrorUsage}, + {name: "missing argument", err: positional, category: CLIErrorUsage}, + {name: "missing file", err: missingFile, category: CLIErrorNotFound}, + {name: "wrapped missing manifest", err: missingManifest, category: CLIErrorNotFound}, + {name: "existing file", err: fmt.Errorf("create file: %w", fs.ErrExist), category: CLIErrorConflict}, + {name: "file permission", err: fmt.Errorf("open file: %w", fs.ErrPermission), category: CLIErrorOperational}, + {name: "process exit", err: &exec.ExitError{}, category: CLIErrorOperational}, + {name: "missing executable", err: &exec.Error{Name: "missing", Err: fs.ErrNotExist}, category: CLIErrorNotFound}, + {name: "API not found", err: fmt.Errorf("request failed: %w", apiNotFound), category: CLIErrorNotFound}, + {name: "authentication", err: fmt.Errorf("request failed: %w", &blaxel.Error{StatusCode: 401}), category: CLIErrorAuthentication}, + {name: "deadline", err: context.DeadlineExceeded, category: CLIErrorOperational}, + {name: "handled HTTP response", err: MarkExpectedHTTPError(errors.New("request failed (HTTP 429): private body"), 429), category: CLIErrorOperational}, + {name: "explicit validation", err: MarkExpectedError(errors.New("bad config"), CLIErrorValidation), category: CLIErrorValidation}, + {name: "cancelled login", err: huh.ErrUserAborted, category: CLIErrorUsage}, + {name: "missing credentials", err: MarkExpectedError(errors.New("no valid credentials found. Please run 'bl login' first"), CLIErrorAuthentication), category: CLIErrorAuthentication}, + {name: "denied device authorization", err: MarkExpectedError(errors.New("authentication failed with status 400: access_denied"), CLIErrorAuthentication), category: CLIErrorAuthentication}, + {name: "image build failure", err: MarkExpectedError(errors.New("image build failed"), CLIErrorOperational), category: CLIErrorOperational}, + {name: "deployment failure", err: MarkExpectedError(errors.New("deployment failed for /snapshot: apply returned no results"), CLIErrorOperational), category: CLIErrorOperational}, + {name: "non-interactive command", err: MarkExpectedError(errors.New("this command requires an interactive terminal"), CLIErrorUsage), category: CLIErrorUsage}, + {name: "websocket handshake", err: fmt.Errorf("failed to connect to terminal: %w", websocket.ErrBadHandshake), category: CLIErrorOperational}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + classification := classifyCLIError(test.err) + assert.True(t, classification.expected) + assert.Equal(t, test.category, classification.category) + }) + } +} + +func TestMarkExpectedErrorPreservesLocalMessageAndCause(t *testing.T) { + cause := errors.New("local detail remains visible") + marked := MarkExpectedError(cause, CLIErrorValidation) + require.Error(t, marked) + assert.Equal(t, cause.Error(), marked.Error()) + assert.ErrorIs(t, marked, cause) + assert.True(t, IsExpectedCLIError(marked)) + assert.False(t, IsExpectedCLIError(cause)) + assert.False(t, IsExpectedCLIError(nil)) + assert.Nil(t, MarkExpectedError(nil, CLIErrorValidation)) + assert.Same(t, cause, MarkExpectedError(cause, CLIErrorInternal)) + assert.Same(t, cause, MarkExpectedError(cause, CLIErrorPanic)) + assert.Same(t, cause, MarkExpectedHTTPError(cause, 200)) +} + +func TestExpectedErrorsCreateNoSentryEvents(t *testing.T) { + transport := bindMockSentry(t) + + expectedErrors := []error{ + unknownFlagError(t), + cobra.ExactArgs(1)(&cobra.Command{Use: "test"}, nil), + &os.PathError{Op: "open", Path: "/private/customer/file", Err: os.ErrNotExist}, + fmt.Errorf("API request: %w", &blaxel.Error{StatusCode: 404}), + MarkExpectedError(errors.New("permission denied: please login"), CLIErrorAuthentication), + MarkExpectedHTTPError(errors.New("raw response body"), 503), + huh.ErrUserAborted, + MarkExpectedError(errors.New("no valid credentials found. Please run 'bl login' first"), CLIErrorAuthentication), + MarkExpectedError(errors.New("image build failed"), CLIErrorOperational), + MarkExpectedError(errors.New("this command requires an interactive terminal"), CLIErrorUsage), + fmt.Errorf("failed to connect to terminal: %w", websocket.ErrBadHandshake), + } + + for _, err := range expectedErrors { + assert.False(t, captureUnexpectedError(err)) + } + assert.Empty(t, transport.Events()) +} + +func TestUnmarkedLookalikeErrorsRemainReportable(t *testing.T) { + transport := bindMockSentry(t) + + lookalikes := []error{ + errors.New("internal index not found"), + errors.New("internal operation timed out"), + errors.New("invalid internal state"), + errors.New("permission denied while loading internal state"), + errors.New("client not initialized"), + errors.New("user aborted"), + errors.New("websocket: bad handshake"), + } + + for _, err := range lookalikes { + assert.True(t, captureUnexpectedError(err)) + } + + events := transport.Events() + require.Len(t, events, len(lookalikes)) + for _, event := range events { + assert.Equal(t, "internal", event.Tags["error.category"]) + assert.Equal(t, "Unexpected CLI failure", event.Exception[0].Value) + } +} + +func TestUnexpectedErrorCreatesOneSanitizedEvent(t *testing.T) { + transport := bindMockSentry(t) + SetSentryTag("version", "0.1.106") + SetSentryTag("commit", "abc1234") + SetSentryTag("command.class", "bl deploy") + SetSentryTag("workspace", "private-workspace") + sentry.ConfigureScope(func(scope *sentry.Scope) { + scope.SetTag("commit", "/Users/customer/private-project") + scope.SetUser(sentry.User{ID: "private-user"}) + scope.SetContext("private-context", sentry.Context{"resource": "private-resource"}) + scope.SetFingerprint([]string{"private-fingerprint"}) + scope.SetRequest(httptest.NewRequest("POST", "https://example.com/private-resource", strings.NewReader("private-body"))) + scope.AddAttachment(&sentry.Attachment{Filename: "private.txt", Payload: []byte("private-attachment")}) + scope.AddBreadcrumb(&sentry.Breadcrumb{Message: "private-breadcrumb"}, 10) + }) + + err := errors.New("internal invariant failed for /Users/customer/private-project and token secret-123") + assert.True(t, captureUnexpectedError(err)) + + events := transport.Events() + require.Len(t, events, 1) + event := events[0] + require.Len(t, event.Exception, 1) + assert.Equal(t, "CLIInternalError", event.Exception[0].Type) + assert.Equal(t, "Unexpected CLI failure", event.Exception[0].Value) + assert.Equal(t, "internal", event.Tags["error.category"]) + assert.Equal(t, "bl deploy", event.Tags["command.class"]) + assert.Equal(t, "0.1.106", event.Tags["version"]) + assert.Equal(t, "unknown", event.Tags["commit"]) + assert.NotContains(t, event.Tags, "workspace") + assert.Empty(t, event.ServerName) + assert.Nil(t, event.Request) + assert.Empty(t, event.User) + assert.Nil(t, event.Attachments) + assert.Nil(t, event.Fingerprint) + assert.Nil(t, event.Contexts) + assert.Nil(t, event.DebugMeta) + + for _, frame := range event.Exception[0].Stacktrace.Frames { + assert.Empty(t, frame.AbsPath) + assert.False(t, strings.Contains(frame.Filename, "/")) + assert.True(t, strings.HasPrefix(frame.Module, "github.com/blaxel-ai/toolkit/")) + assert.Nil(t, frame.Vars) + assert.Nil(t, frame.PreContext) + assert.Empty(t, frame.ContextLine) + assert.Nil(t, frame.PostContext) + } + + serialized, marshalErr := json.Marshal(event) + require.NoError(t, marshalErr) + assert.NotContains(t, string(serialized), "private-project") + assert.NotContains(t, string(serialized), "secret-123") + assert.NotContains(t, string(serialized), "private-workspace") + assert.NotContains(t, string(serialized), "private-user") + assert.NotContains(t, string(serialized), "private-resource") + assert.NotContains(t, string(serialized), "private-fingerprint") + assert.NotContains(t, string(serialized), "private-body") + assert.NotContains(t, string(serialized), "private-attachment") + assert.NotContains(t, string(serialized), "private-breadcrumb") + assert.NotContains(t, string(serialized), "/Users/customer") } func TestCaptureExceptionWithNil(t *testing.T) { - // Should not panic with nil error + transport := bindMockSentry(t) CaptureException(nil) + assert.Empty(t, transport.Events()) } -func TestCaptureExceptionWithError(t *testing.T) { - // Reset DSN to ensure it doesn't actually send to Sentry +func TestFlushSentryWithEmptyDSN(t *testing.T) { SentryDSN = "" - - err := errors.New("test error") - // Should not panic - CaptureException(err) + FlushSentry(time.Second) } func TestSetSentryTagWithEmptyDSN(t *testing.T) { SentryDSN = "" - - // Should not panic - SetSentryTag("key", "value") + SetSentryTag("version", "v1") } func TestRecoverWithSentryEmptyDSN(t *testing.T) { SentryDSN = "" - - // Should not panic when DSN is empty RecoverWithSentry() } + +func TestRecoverWithSentrySanitizesEventAndPreservesPanic(t *testing.T) { + transport := bindMockSentry(t) + const panicValue = "panic with private resource secret-123" + + var recovered any + func() { + defer func() { recovered = recover() }() + func() { + defer RecoverWithSentry() + panic(panicValue) + }() + }() + + assert.Equal(t, panicValue, recovered) + events := transport.Events() + require.Len(t, events, 1) + require.Len(t, events[0].Exception, 1) + assert.Equal(t, "panic", events[0].Tags["error.category"]) + assert.Equal(t, "Unexpected CLI failure", events[0].Exception[0].Value) + serialized, err := json.Marshal(events[0]) + require.NoError(t, err) + assert.NotContains(t, string(serialized), "secret-123") + assert.NotContains(t, string(serialized), "private resource") +} diff --git a/cli/core/templates.go b/cli/core/templates.go index ae29c624..a056f2ea 100644 --- a/cli/core/templates.go +++ b/cli/core/templates.go @@ -2,6 +2,7 @@ package core import ( "context" + "errors" "fmt" "os" "os/exec" @@ -9,7 +10,6 @@ import ( "path/filepath" "regexp" "slices" - "strings" blaxel "github.com/blaxel-ai/sdk-go" "github.com/charmbracelet/huh/spinner" @@ -49,10 +49,12 @@ func RetrieveTemplates(templateType string) (Templates, error) { } resp, err := client.Templates.List(context.Background()) if err != nil { - // Check if it's an authentication error - errMsg := err.Error() - if strings.Contains(errMsg, "401") || strings.Contains(errMsg, "403") { - return nil, fmt.Errorf("authentication required: please log in to your workspace using 'bl login'.\nIf you don't have a workspace yet, visit https://app.blaxel.ai to create one") + var apiErr *blaxel.Error + if errors.As(err, &apiErr) && (apiErr.StatusCode == 401 || apiErr.StatusCode == 403) { + return nil, MarkExpectedError( + fmt.Errorf("authentication required: please log in to your workspace using 'bl login'.\nIf you don't have a workspace yet, visit https://app.blaxel.ai to create one"), + CLIErrorAuthentication, + ) } return nil, err } @@ -112,7 +114,10 @@ func RetrieveTemplatesWithSpinner(templateType string, noTTY bool, errorPrefix s } if len(templates) == 0 { - err := fmt.Errorf("no %s templates available. Please contact support", templateType) + err := MarkExpectedError( + fmt.Errorf("no %s templates available. Please contact support", templateType), + CLIErrorOperational, + ) PrintError(errorPrefix, err) return nil, err } @@ -255,7 +260,10 @@ func (t Templates) Find(name string) (Template, error) { return template, nil } } - return Template{}, fmt.Errorf("template not found") + return Template{}, MarkExpectedError( + fmt.Errorf("template not found"), + CLIErrorNotFound, + ) } func (t Template) Clone(opts TemplateOptions) error { diff --git a/cli/core/utils.go b/cli/core/utils.go index d6fd5ee5..de3000ee 100644 --- a/cli/core/utils.go +++ b/cli/core/utils.go @@ -94,7 +94,7 @@ func handleSecret(filePath string, content string) (string, error) { formTemplates.WithTheme(GetHuhTheme()) err := formTemplates.Run() if err != nil { - return content, fmt.Errorf("error handling secret: %v", err) + return content, fmt.Errorf("error handling secret: %w", err) } } for key, value := range values { @@ -112,7 +112,7 @@ func getResultsWrapper(action string, filePath string, recursive bool, n int) ([ } else { fileInfo, err := os.Stat(filePath) if err != nil { - return nil, fmt.Errorf("error getting file info: %v", err) + return nil, fmt.Errorf("error getting file info: %w", err) } // If the path is a directory, read all files in the directory if fileInfo.IsDir() { @@ -127,7 +127,7 @@ func getResultsWrapper(action string, filePath string, recursive bool, n int) ([ } file, err := os.Open(filePath) if err != nil { - return nil, fmt.Errorf("error opening file: %v", err) + return nil, fmt.Errorf("error opening file: %w", err) } defer func() { _ = file.Close() }() reader = file @@ -135,7 +135,7 @@ func getResultsWrapper(action string, filePath string, recursive bool, n int) ([ // Read the entire content as a string first content, err := io.ReadAll(reader) if err != nil { - return nil, fmt.Errorf("error reading content: %v", err) + return nil, fmt.Errorf("error reading content: %w", err) } contentStr := string(content) @@ -162,7 +162,7 @@ func getResultsWrapper(action string, filePath string, recursive bool, n int) ([ }) contentStr, err = handleSecret(filePath, contentStr) if err != nil { - return nil, fmt.Errorf("error handling secret: %v", err) + return nil, fmt.Errorf("error handling secret: %w", err) } } // Lire et parser les documents YAML @@ -174,7 +174,10 @@ func getResultsWrapper(action string, filePath string, recursive bool, n int) ([ break } if err != nil { - return nil, fmt.Errorf("error decoding YAML: %v", err) + return nil, MarkExpectedError( + fmt.Errorf("error decoding YAML: %w", err), + CLIErrorValidation, + ) } results = append(results, result) } @@ -185,7 +188,7 @@ func handleDirectory(action string, filePath string, recursive bool, n int) ([]R var results []Result files, err := os.ReadDir(filePath) if err != nil { - return nil, fmt.Errorf("error reading directory %s: %v", filePath, err) + return nil, fmt.Errorf("error reading directory %s: %w", filePath, err) } for _, file := range files { @@ -277,7 +280,10 @@ func FindGoEntryFile(directory string) (string, error) { } rel = filepath.ToSlash(rel) if !safeGoCmdEntrypointPattern.MatchString(rel) { - return "", fmt.Errorf("unsupported Go entrypoint path %q; automatic cmd/*/main.go detection only supports command directory names with letters, numbers, dots, underscores, and hyphens; configure [entrypoint] prod = \"go run ./cmd/\" in blaxel.toml", rel) + return "", MarkExpectedError( + fmt.Errorf("unsupported Go entrypoint path %q; automatic cmd/*/main.go detection only supports command directory names with letters, numbers, dots, underscores, and hyphens; configure [entrypoint] prod = \"go run ./cmd/\" in blaxel.toml", rel), + CLIErrorValidation, + ) } candidates = append(candidates, rel) } @@ -285,7 +291,10 @@ func FindGoEntryFile(directory string) (string, error) { return "", nil } if len(candidates) > 1 { - return "", fmt.Errorf("multiple Go entrypoints found under cmd/*/main.go (%s); configure [entrypoint] prod = \"go run ./cmd/\" in blaxel.toml", strings.Join(candidates, ", ")) + return "", MarkExpectedError( + fmt.Errorf("multiple Go entrypoints found under cmd/*/main.go (%s); configure [entrypoint] prod = \"go run ./cmd/\" in blaxel.toml", strings.Join(candidates, ", ")), + CLIErrorValidation, + ) } return candidates[0], nil } @@ -649,16 +658,25 @@ const MaxDurationSeconds = 365 * 24 * 60 * 60 // 31,536,000 seconds func ParseDurationToSeconds(duration string) (int, error) { duration = strings.TrimSpace(duration) if duration == "" { - return 0, fmt.Errorf("empty duration string") + return 0, MarkExpectedError( + fmt.Errorf("empty duration string"), + CLIErrorValidation, + ) } // Try parsing as plain integer first if seconds, err := strconv.Atoi(duration); err == nil { if seconds < 0 { - return 0, fmt.Errorf("negative duration not allowed: %d", seconds) + return 0, MarkExpectedError( + fmt.Errorf("negative duration not allowed: %d", seconds), + CLIErrorValidation, + ) } if seconds > MaxDurationSeconds { - return 0, fmt.Errorf("duration exceeds maximum allowed (%d seconds / ~1 year): %d", MaxDurationSeconds, seconds) + return 0, MarkExpectedError( + fmt.Errorf("duration exceeds maximum allowed (%d seconds / ~1 year): %d", MaxDurationSeconds, seconds), + CLIErrorValidation, + ) } return seconds, nil } @@ -667,16 +685,25 @@ func ParseDurationToSeconds(duration string) (int, error) { re := regexp.MustCompile(`^(\d+)([smhdw])$`) matches := re.FindStringSubmatch(strings.ToLower(duration)) if len(matches) != 3 { - return 0, fmt.Errorf("invalid duration format: %s (expected formats: 30s, 5m, 1h, 2d, 1w)", duration) + return 0, MarkExpectedError( + fmt.Errorf("invalid duration format: %s (expected formats: 30s, 5m, 1h, 2d, 1w)", duration), + CLIErrorValidation, + ) } value, err := strconv.Atoi(matches[1]) if err != nil { - return 0, fmt.Errorf("invalid numeric value in duration: %s", duration) + return 0, MarkExpectedError( + fmt.Errorf("invalid numeric value in duration: %s", duration), + CLIErrorValidation, + ) } if value < 0 { - return 0, fmt.Errorf("negative duration not allowed: %s", duration) + return 0, MarkExpectedError( + fmt.Errorf("negative duration not allowed: %s", duration), + CLIErrorValidation, + ) } // Define multipliers and max safe values for each unit to prevent overflow @@ -697,12 +724,18 @@ func ParseDurationToSeconds(duration string) (int, error) { unit := matches[2] config, ok := units[unit] if !ok { - return 0, fmt.Errorf("unknown duration unit: %s", unit) + return 0, MarkExpectedError( + fmt.Errorf("unknown duration unit: %s", unit), + CLIErrorValidation, + ) } // Check bounds before multiplication to prevent overflow if value > config.maxValue { - return 0, fmt.Errorf("duration exceeds maximum allowed (~1 year): %s (max: %d%s)", duration, config.maxValue, unit) + return 0, MarkExpectedError( + fmt.Errorf("duration exceeds maximum allowed (~1 year): %s (max: %d%s)", duration, config.maxValue, unit), + CLIErrorValidation, + ) } return value * config.multiplier, nil diff --git a/cli/delete.go b/cli/delete.go index 913131ab..77e3ae6a 100644 --- a/cli/delete.go +++ b/cli/delete.go @@ -99,6 +99,7 @@ separately if needed.`, // At this point, results contains all your YAML documents hasFailures := false + allFailuresExpected := true var deleted []deleteEntry var failed []deleteEntry for _, result := range results { @@ -107,6 +108,9 @@ separately if needed.`, name := result.Metadata.(map[string]interface{})["name"].(string) if err := DeleteFn(resource, name); err != nil { hasFailures = true + if !core.IsExpectedCLIError(err) { + allFailuresExpected = false + } failed = append(failed, deleteEntry{Kind: resource.Kind, Name: name}) } else { deleted = append(deleted, deleteEntry{Kind: resource.Kind, Name: name}) @@ -117,7 +121,11 @@ separately if needed.`, printDeleteStructuredOutput(deleted, failed) if hasFailures { - core.ExitWithError(fmt.Errorf("one or more deletions failed")) + err := fmt.Errorf("one or more deletions failed") + if allFailuresExpected { + err = core.MarkExpectedError(err, core.CLIErrorOperational) + } + core.ExitWithError(err) } }, } @@ -153,7 +161,10 @@ separately if needed.`, ValidArgsFunction: GetResourceValidArgsFunction(resourceKind), Run: func(cmd *cobra.Command, args []string) { if len(args) == 0 { - err := fmt.Errorf("no resource name provided") + err := core.MarkExpectedError( + fmt.Errorf("no resource name provided"), + core.CLIErrorValidation, + ) fmt.Println(err) core.ExitWithError(err) } @@ -166,11 +177,15 @@ separately if needed.`, } hasFailures := false + allFailuresExpected := true var deleted []deleteEntry var failed []deleteEntry for _, name := range args { if err := DeleteFn(resource, name); err != nil { hasFailures = true + if !core.IsExpectedCLIError(err) { + allFailuresExpected = false + } failed = append(failed, deleteEntry{Kind: resource.Kind, Name: name}) } else { deleted = append(deleted, deleteEntry{Kind: resource.Kind, Name: name}) @@ -178,7 +193,11 @@ separately if needed.`, } printDeleteStructuredOutput(deleted, failed) if hasFailures { - core.ExitWithError(fmt.Errorf("one or more deletions failed")) + err := fmt.Errorf("one or more deletions failed") + if allFailuresExpected { + err = core.MarkExpectedError(err, core.CLIErrorOperational) + } + core.ExitWithError(err) } }, } @@ -191,7 +210,10 @@ separately if needed.`, func DeleteFn(resource *core.Resource, name string) error { if resource.Delete == nil { hint := nestedResourceHint(resource, "delete") - err := fmt.Errorf("'bl delete %s' is not supported directly.%s", resource.Singular, hint) + err := core.MarkExpectedError( + fmt.Errorf("'bl delete %s' is not supported directly.%s", resource.Singular, hint), + core.CLIErrorValidation, + ) fmt.Fprintln(os.Stderr, err) return err } diff --git a/cli/deploy.go b/cli/deploy.go index f8ae89eb..73fdbfb7 100644 --- a/cli/deploy.go +++ b/cli/deploy.go @@ -203,7 +203,10 @@ all projects in a monorepo (looks for blaxel.toml in subdirectories).`, } if parsed <= 0 { core.PrintError("Deploy", fmt.Errorf("timeout must be a positive duration, got %q", timeoutStr)) - core.ExitWithError(fmt.Errorf("invalid timeout")) + core.ExitWithError(core.MarkExpectedError( + fmt.Errorf("invalid timeout"), + core.CLIErrorValidation, + )) } deployTimeout = parsed } @@ -805,14 +808,20 @@ func getResource(resourceType, name string) (map[string]interface{}, error) { case "volume-template", "volumetemplate", "vt": result, err = client.VolumeTemplates.Get(ctx, name) default: - return nil, fmt.Errorf("unknown resource type: %s", resourceType) + return nil, core.MarkExpectedError( + fmt.Errorf("unknown resource type: %s", resourceType), + core.CLIErrorValidation, + ) } if err != nil { // Check if it's a not found error var apiErr *blaxel.Error if isBlaxelErrorDeploy(err, &apiErr) && apiErr.StatusCode == 404 { - return nil, fmt.Errorf("%s %s not found. please deploy with a build first", resourceType, name) + return nil, core.MarkExpectedError( + fmt.Errorf("%s %s not found. please deploy with a build first", resourceType, name), + core.CLIErrorNotFound, + ) } return nil, err } @@ -1041,7 +1050,10 @@ func (d *Deployment) ApplyInteractive() error { // Check if any resources failed for _, r := range resources { if r.Status == deploy.StatusFailed { - return fmt.Errorf("deployment failed for %s/%s: %v", r.Kind, r.Name, r.Error) + if r.Error == nil { + return fmt.Errorf("deployment failed for %s/%s without error detail", r.Kind, r.Name) + } + return fmt.Errorf("deployment failed for %s/%s: %w", r.Kind, r.Name, r.Error) } } @@ -1358,7 +1370,10 @@ func (d *Deployment) deployResourceInteractive(resource *deploy.Resource, model if logWatcher != nil { logWatcher.Stop() } - model.UpdateResource(idx, deploy.StatusFailed, "Deployment timeout", fmt.Errorf("deployment timed out after %s", d.timeout)) + model.UpdateResource(idx, deploy.StatusFailed, "Deployment timeout", core.MarkExpectedError( + fmt.Errorf("deployment timed out after %s", d.timeout), + core.CLIErrorOperational, + )) return case <-staleFailedGracePeriod: // Grace period expired - if status is still FAILED, accept it as real @@ -1440,14 +1455,20 @@ func (d *Deployment) deployResourceInteractive(resource *deploy.Resource, model if logWatcher != nil { logWatcher.Stop() } - model.UpdateResource(idx, deploy.StatusFailed, "Deployment failed", fmt.Errorf("resource deployment failed")) + model.UpdateResource(idx, deploy.StatusFailed, "Deployment failed", core.MarkExpectedError( + fmt.Errorf("resource deployment failed"), + core.CLIErrorOperational, + )) model.AddBuildLog(idx, "Status changed to: FAILED - Deployment failed") return case "DEACTIVATED", "DEACTIVATING", "DELETING": if logWatcher != nil { logWatcher.Stop() } - model.UpdateResource(idx, deploy.StatusFailed, fmt.Sprintf("Unexpected status: %s", status), fmt.Errorf("resource is being deactivated or deleted")) + model.UpdateResource(idx, deploy.StatusFailed, fmt.Sprintf("Unexpected status: %s", status), core.MarkExpectedError( + fmt.Errorf("resource is being deactivated or deleted"), + core.CLIErrorOperational, + )) model.AddBuildLog(idx, fmt.Sprintf("Unexpected status: %s", status)) return default: @@ -1540,7 +1561,10 @@ func (d *Deployment) deployAdditionalResource(resource *deploy.Resource, model * if logWatcher != nil { logWatcher.Stop() } - model.UpdateResource(idx, deploy.StatusFailed, "Timeout", fmt.Errorf("deployment timed out after %s", additionalTimeout)) + model.UpdateResource(idx, deploy.StatusFailed, "Timeout", core.MarkExpectedError( + fmt.Errorf("deployment timed out after %s", additionalTimeout), + core.CLIErrorOperational, + )) ticker.Stop() return case <-ticker.C: @@ -1603,14 +1627,20 @@ func (d *Deployment) deployAdditionalResource(resource *deploy.Resource, model * if logWatcher != nil { logWatcher.Stop() } - model.UpdateResource(idx, deploy.StatusFailed, "Failed", fmt.Errorf("deployment failed")) + model.UpdateResource(idx, deploy.StatusFailed, "Failed", core.MarkExpectedError( + fmt.Errorf("deployment failed"), + core.CLIErrorOperational, + )) ticker.Stop() return case "DEACTIVATED", "DEACTIVATING", "DELETING": if logWatcher != nil { logWatcher.Stop() } - model.UpdateResource(idx, deploy.StatusFailed, fmt.Sprintf("Unexpected status: %s", status), fmt.Errorf("resource is being deactivated or deleted")) + model.UpdateResource(idx, deploy.StatusFailed, fmt.Sprintf("Unexpected status: %s", status), core.MarkExpectedError( + fmt.Errorf("resource is being deactivated or deleted"), + core.CLIErrorOperational, + )) ticker.Stop() return default: @@ -1739,7 +1769,10 @@ func (d *Deployment) renderDryRunStructuredOutput(outputFmt string, skipBuild bo case "yaml": return yaml.Marshal(result) default: - return nil, fmt.Errorf("unsupported dry-run output format %q", outputFmt) + return nil, core.MarkExpectedError( + fmt.Errorf("unsupported dry-run output format %q", outputFmt), + core.CLIErrorValidation, + ) } } @@ -2107,7 +2140,13 @@ func (d *Deployment) createArchive(_ string, writer archiveWriter) error { // Validate that the directory exists if _, err := os.Stat(archiveRoot); err != nil { - return fmt.Errorf("volume template directory does not exist: %s", volumeDir) + if os.IsNotExist(err) { + return core.MarkExpectedError( + fmt.Errorf("volume template directory does not exist: %s", volumeDir), + core.CLIErrorNotFound, + ) + } + return fmt.Errorf("failed to inspect volume template directory %q: %w", volumeDir, err) } } @@ -2464,7 +2503,7 @@ func deployPackage(dryRun bool, name string) bool { func getDeployCommands(dryRun bool, defaultName string) ([]server.PackageCommand, error) { pwd, err := os.Getwd() if err != nil { - return nil, fmt.Errorf("error getting current directory: %v", err) + return nil, fmt.Errorf("error getting current directory: %w", err) } command := server.PackageCommand{ Name: "root", diff --git a/cli/drive.go b/cli/drive.go index 1e5cf700..3d6c331c 100644 --- a/cli/drive.go +++ b/cli/drive.go @@ -8,7 +8,6 @@ import ( "io" "net/http" "net/url" - "os" "strings" blaxel "github.com/blaxel-ai/sdk-go" @@ -283,9 +282,9 @@ func DriveMountsCmd() *cobra.Command { var sandboxName string cmd := &cobra.Command{ - Use: "mounts", - Short: "List mounted drives in a sandbox", - Long: `List all currently mounted drives in a sandbox environment.`, + Use: "mounts", + Short: "List mounted drives in a sandbox", + Long: `List all currently mounted drives in a sandbox environment.`, Example: ` # List all mounted drives bl drive mounts --sandbox my-sandbox`, Run: func(cmd *cobra.Command, args []string) { @@ -401,8 +400,9 @@ func DriveListCmd() *cobra.Command { Run: func(cmd *cobra.Command, args []string) { r := driveResource() if r == nil { - core.PrintError("Drive", fmt.Errorf("drive resource not found")) - core.ExitWithError(fmt.Errorf("drive resource not found")) + err := fmt.Errorf("internal drive resource registry invariant failed") + core.PrintError("Drive", err) + core.ExitWithError(err) } ListFnPaginated(r, pageLimit, pageCursor, fetchAll) }, @@ -429,8 +429,9 @@ func DriveGetCmd() *cobra.Command { Run: func(cmd *cobra.Command, args []string) { r := driveResource() if r == nil { - core.PrintError("Drive", fmt.Errorf("drive resource not found")) - core.ExitWithError(fmt.Errorf("drive resource not found")) + err := fmt.Errorf("internal drive resource registry invariant failed") + core.PrintError("Drive", err) + core.ExitWithError(err) } GetFn(r, args[0]) }, @@ -502,8 +503,9 @@ func DriveDeleteCmd() *cobra.Command { Run: func(cmd *cobra.Command, args []string) { r := driveResource() if r == nil { - core.PrintError("Drive", fmt.Errorf("drive resource not found")) - core.ExitWithError(fmt.Errorf("drive resource not found")) + err := fmt.Errorf("internal drive resource registry invariant failed") + core.PrintError("Drive", err) + core.ExitWithError(err) } if err := DeleteFn(r, args[0]); err != nil { core.ExitWithError(err) @@ -517,14 +519,20 @@ func resolveSandbox(ctx context.Context, sandboxName string) (sandboxURL, token currentContext, _ := blaxel.CurrentContext() workspace := currentContext.Workspace if workspace == "" { - err := fmt.Errorf("no workspace found in current context. Please run 'bl login' first") + err := core.MarkExpectedError( + fmt.Errorf("no workspace found in current context. Please run 'bl login' first"), + core.CLIErrorAuthentication, + ) core.PrintError("Drive", err) core.ExitWithError(err) } credentials, _ := blaxel.LoadCredentials(workspace) if !credentials.IsValid() { - err := fmt.Errorf("no valid credentials found. Please run 'bl login' first") + err := core.MarkExpectedError( + fmt.Errorf("no valid credentials found. Please run 'bl login' first"), + core.CLIErrorAuthentication, + ) core.PrintError("Drive", err) core.ExitWithError(err) } @@ -534,7 +542,10 @@ func resolveSandbox(ctx context.Context, sandboxName string) (sandboxURL, token token = credentials.APIKey } if token == "" { - err := fmt.Errorf("no access token or Blaxel API key found. Please run 'bl login' first") + err := core.MarkExpectedError( + fmt.Errorf("no access token or Blaxel API key found. Please run 'bl login' first"), + core.CLIErrorAuthentication, + ) core.PrintError("Drive", err) core.ExitWithError(err) } @@ -544,7 +555,10 @@ func resolveSandbox(ctx context.Context, sandboxName string) (sandboxURL, token if err != nil { var apiErr *blaxel.Error if isBlaxelError(err, &apiErr) && apiErr.StatusCode == 404 { - err = fmt.Errorf("sandbox '%s' not found", sandboxName) + err = core.MarkExpectedError( + fmt.Errorf("sandbox '%s' not found", sandboxName), + core.CLIErrorNotFound, + ) core.PrintError("Drive", err) sandboxes, listErr := client.Sandboxes.List(ctx) @@ -599,11 +613,11 @@ func handleSandboxAPIError(body []byte, statusCode int, operation string) { if err := json.Unmarshal(body, &apiErr); err == nil && apiErr.Error != "" { err := fmt.Errorf("failed to %s (HTTP %d): %s", operation, statusCode, apiErr.Error) core.PrintError("Drive", err) - core.ExitWithError(err) + core.ExitWithError(core.MarkExpectedHTTPError(err, statusCode)) } err := fmt.Errorf("failed to %s (HTTP %d): %s", operation, statusCode, string(body)) core.PrintError("Drive", err) - core.ExitWithError(err) + core.ExitWithError(core.MarkExpectedHTTPError(err, statusCode)) } // outputDriveData marshals the given data to JSON or YAML format and prints it. @@ -618,8 +632,9 @@ func outputDriveData(data interface{}, format string) { output, err = yaml.Marshal(data) } if err != nil { - core.PrintError("Drive", fmt.Errorf("failed to marshal output: %w", err)) - os.Exit(1) + err = fmt.Errorf("failed to marshal output: %w", err) + core.PrintError("Drive", err) + core.ExitWithError(err) } fmt.Println(string(output)) } diff --git a/cli/get.go b/cli/get.go index 374e569c..6062578b 100644 --- a/cli/get.go +++ b/cli/get.go @@ -355,7 +355,10 @@ Output formats: client := core.GetClient() if client == nil { core.PrintError("Sandbox Hub", fmt.Errorf("client not initialized, please log in with 'bl login'")) - core.ExitWithError(fmt.Errorf("client not initialized")) + core.ExitWithError(core.MarkExpectedError( + fmt.Errorf("client not initialized"), + core.CLIErrorAuthentication, + )) } resp, err := client.Sandboxes.GetHub(context.Background()) if err != nil { @@ -463,7 +466,10 @@ Output formats: client := core.GetClient() if client == nil { core.PrintError("MCP Hub", fmt.Errorf("client not initialized, please log in with 'bl login'")) - core.ExitWithError(fmt.Errorf("client not initialized")) + core.ExitWithError(core.MarkExpectedError( + fmt.Errorf("client not initialized"), + core.CLIErrorAuthentication, + )) } var resp []mcpHubDefinition @@ -525,7 +531,10 @@ func GetFn(resource *core.Resource, name string) { if resource.Get == nil { hint := nestedResourceHint(resource, "get") - err := fmt.Errorf("%s'bl get %s ' is not supported directly.%s", formattedError, resource.Singular, hint) + err := core.MarkExpectedError( + fmt.Errorf("%s'bl get %s ' is not supported directly.%s", formattedError, resource.Singular, hint), + core.CLIErrorValidation, + ) core.PrintError("Get", err) core.ExitWithError(err) } @@ -701,7 +710,7 @@ func ListExecPaginated(resource *core.Resource, limit int, cursor string) ([]int formattedError := fmt.Sprintf("Resource %s error: ", resource.Kind) result, err := core.ListPaginated(resource, limit, cursor) if err != nil { - return nil, core.PaginationMeta{}, fmt.Errorf("%s%v", formattedError, err) + return nil, core.PaginationMeta{}, fmt.Errorf("%s%w", formattedError, err) } return result.Items, result.Meta, nil } @@ -713,7 +722,7 @@ func ListExec(resource *core.Resource) ([]interface{}, error) { if resource.Paginated && resource.APIPath != "" { items, _, err := ListExecPaginated(resource, core.DefaultPageLimit, "") if err != nil { - return nil, fmt.Errorf("%s%v", formattedError, err) + return nil, fmt.Errorf("%s%w", formattedError, err) } return items, nil } @@ -722,7 +731,10 @@ func ListExec(resource *core.Resource) ([]interface{}, error) { // support pagination (e.g. IntegrationConnection, VolumeTemplate). if resource.List == nil { hint := nestedResourceHint(resource, "get") - return nil, fmt.Errorf("%s'bl get %s' is not supported directly.%s", formattedError, resource.Plural, hint) + return nil, core.MarkExpectedError( + fmt.Errorf("%s'bl get %s' is not supported directly.%s", formattedError, resource.Plural, hint), + core.CLIErrorValidation, + ) } ctx := context.Background() @@ -744,7 +756,7 @@ func ListExec(resource *core.Resource) ([]interface{}, error) { } if err, ok := results[1].Interface().(error); ok && err != nil { - return nil, fmt.Errorf("%s%v", formattedError, err) + return nil, fmt.Errorf("%s%w", formattedError, err) } // The new SDK returns typed responses (e.g., *[]Agent), not *http.Response diff --git a/cli/images.go b/cli/images.go index 1f0c5d8f..2e5f6945 100644 --- a/cli/images.go +++ b/cli/images.go @@ -29,7 +29,10 @@ func parseImageRef(ref string) (resourceType, imageName, tag string, err error) // Split resourceType/imageName imageParts := strings.SplitN(imageRef, "/", 2) if len(imageParts) != 2 { - return "", "", "", fmt.Errorf("invalid image reference format. Expected 'resourceType/imageName' or 'resourceType/imageName:tag', got '%s'", ref) + return "", "", "", core.MarkExpectedError( + fmt.Errorf("invalid image reference format. Expected 'resourceType/imageName' or 'resourceType/imageName:tag', got '%s'", ref), + core.CLIErrorValidation, + ) } resourceType = imageParts[0] @@ -133,7 +136,7 @@ func ListAllImages() { imageList, err := client.Images.List(ctx) if err != nil { - err = fmt.Errorf("error listing images: %v", err) + err = fmt.Errorf("error listing images: %w", err) fmt.Println(err) core.ExitWithError(err) } @@ -148,7 +151,7 @@ func ListAllImages() { // Convert to JSON for manipulation jsonData, err := json.Marshal(imageList) if err != nil { - err = fmt.Errorf("error parsing images: %v", err) + err = fmt.Errorf("error parsing images: %w", err) fmt.Println(err) core.ExitWithError(err) } @@ -156,7 +159,7 @@ func ListAllImages() { // Parse the response var images []interface{} if err := json.Unmarshal(jsonData, &images); err != nil { - err = fmt.Errorf("error parsing response: %v", err) + err = fmt.Errorf("error parsing response: %w", err) fmt.Println(err) core.ExitWithError(err) } @@ -185,7 +188,7 @@ func getImageLatest(resourceType, imageName string) { imageResult, err := client.Images.Get(ctx, imageName, blaxel.ImageGetParams{ResourceType: resourceType}) if err != nil { - err = fmt.Errorf("error getting image %s/%s: %v", resourceType, imageName, err) + err = fmt.Errorf("error getting image %s/%s: %w", resourceType, imageName, err) fmt.Println(err) core.ExitWithError(err) } @@ -211,7 +214,7 @@ func getImage(resourceType, imageName, tag string) { imageResult, err := client.Images.Get(ctx, imageName, blaxel.ImageGetParams{ResourceType: resourceType}) if err != nil { - err = fmt.Errorf("error getting image %s/%s: %v", resourceType, imageName, err) + err = fmt.Errorf("error getting image %s/%s: %w", resourceType, imageName, err) fmt.Println(err) core.ExitWithError(err) } @@ -219,7 +222,7 @@ func getImage(resourceType, imageName, tag string) { // Convert to JSON for manipulation jsonData, err := json.Marshal(imageResult) if err != nil { - err = fmt.Errorf("error parsing image: %v", err) + err = fmt.Errorf("error parsing image: %w", err) fmt.Println(err) core.ExitWithError(err) } @@ -227,7 +230,7 @@ func getImage(resourceType, imageName, tag string) { // Parse the response var image map[string]interface{} if err := json.Unmarshal(jsonData, &image); err != nil { - err = fmt.Errorf("error parsing response: %v", err) + err = fmt.Errorf("error parsing response: %w", err) fmt.Println(err) core.ExitWithError(err) } @@ -247,7 +250,10 @@ func getImage(resourceType, imageName, tag string) { spec["tags"] = filteredTags if len(filteredTags) == 0 { - err := fmt.Errorf("tag '%s' not found for image %s/%s", tag, resourceType, imageName) + err := core.MarkExpectedError( + fmt.Errorf("tag '%s' not found for image %s/%s", tag, resourceType, imageName), + core.CLIErrorNotFound, + ) fmt.Println(err) core.ExitWithError(err) } @@ -434,28 +440,42 @@ WARNING: Deleting an image without specifying a tag will remove ALL tags.`, bl delete image agent/img1:v1 agent/img2:v2`, Run: func(cmd *cobra.Command, args []string) { if len(args) == 0 { - err := fmt.Errorf("no image reference provided\nUsage: bl delete image resourceType/imageName[:tag]") + err := core.MarkExpectedError( + fmt.Errorf("no image reference provided\nUsage: bl delete image resourceType/imageName[:tag]"), + core.CLIErrorUsage, + ) fmt.Println(err) core.ExitWithError(err) } hasFailures := false + allFailuresExpected := true for _, arg := range args { // Parse the image reference resourceType, imageName, tag, err := parseImageRef(arg) if err != nil { fmt.Printf("Error: %v\n", err) hasFailures = true + if !core.IsExpectedCLIError(err) { + allFailuresExpected = false + } continue } if err := deleteImage(resourceType, imageName, tag); err != nil { hasFailures = true + if !core.IsExpectedCLIError(err) { + allFailuresExpected = false + } } } if hasFailures { - core.ExitWithError(fmt.Errorf("one or more image deletions failed")) + err := fmt.Errorf("one or more image deletions failed") + if allFailuresExpected { + err = core.MarkExpectedError(err, core.CLIErrorOperational) + } + core.ExitWithError(err) } }, } @@ -511,7 +531,10 @@ The image reference format is: resourceType/imageName Args: cobra.ExactArgs(1), Run: func(cmd *cobra.Command, args []string) { if workspace == "" { - err := fmt.Errorf("--workspace flag is required") + err := core.MarkExpectedError( + fmt.Errorf("--workspace flag is required"), + core.CLIErrorValidation, + ) fmt.Println(err) core.ExitWithError(err) } @@ -523,7 +546,10 @@ The image reference format is: resourceType/imageName } if tag != "" { - err := fmt.Errorf("sharing a specific tag is not supported, remove ':%s' from the reference", tag) + err := core.MarkExpectedError( + fmt.Errorf("sharing a specific tag is not supported, remove ':%s' from the reference", tag), + core.CLIErrorValidation, + ) fmt.Println(err) core.ExitWithError(err) } @@ -535,7 +561,7 @@ The image reference format is: resourceType/imageName path := fmt.Sprintf("images/%s/%s/share", resourceType, imageName) err = client.Post(ctx, path, body, nil) if err != nil { - err = fmt.Errorf("error sharing image %s/%s: %v", resourceType, imageName, err) + err = fmt.Errorf("error sharing image %s/%s: %w", resourceType, imageName, err) fmt.Println(err) core.ExitWithError(err) } @@ -567,7 +593,10 @@ The image reference format is: resourceType/imageName Args: cobra.ExactArgs(1), Run: func(cmd *cobra.Command, args []string) { if workspace == "" { - err := fmt.Errorf("--workspace flag is required") + err := core.MarkExpectedError( + fmt.Errorf("--workspace flag is required"), + core.CLIErrorValidation, + ) fmt.Println(err) core.ExitWithError(err) } @@ -579,7 +608,10 @@ The image reference format is: resourceType/imageName } if tag != "" { - err := fmt.Errorf("unsharing a specific tag is not supported, remove ':%s' from the reference", tag) + err := core.MarkExpectedError( + fmt.Errorf("unsharing a specific tag is not supported, remove ':%s' from the reference", tag), + core.CLIErrorValidation, + ) fmt.Println(err) core.ExitWithError(err) } @@ -590,7 +622,7 @@ The image reference format is: resourceType/imageName path := fmt.Sprintf("images/%s/%s/share/%s", resourceType, imageName, workspace) err = client.Delete(ctx, path, nil, nil) if err != nil { - err = fmt.Errorf("error unsharing image %s/%s: %v", resourceType, imageName, err) + err = fmt.Errorf("error unsharing image %s/%s: %w", resourceType, imageName, err) fmt.Println(err) core.ExitWithError(err) } diff --git a/cli/login.go b/cli/login.go index 2515cfb2..666bc42b 100644 --- a/cli/login.go +++ b/cli/login.go @@ -110,7 +110,10 @@ func resolveLoginWorkspace(cmd *cobra.Command, args []string) (string, string, e flagWorkspace, flagChanged := explicitWorkspaceFlag(cmd) if positionalWorkspace != "" { if flagChanged && flagWorkspace != "" && flagWorkspace != positionalWorkspace { - return "", "", fmt.Errorf("workspace specified twice: positional workspace %q conflicts with --workspace %q", positionalWorkspace, flagWorkspace) + return "", "", core.MarkExpectedError( + fmt.Errorf("workspace specified twice: positional workspace %q conflicts with --workspace %q", positionalWorkspace, flagWorkspace), + core.CLIErrorValidation, + ) } return positionalWorkspace, "", nil } diff --git a/cli/logs.go b/cli/logs.go index f7b423dd..c750497d 100644 --- a/cli/logs.go +++ b/cli/logs.go @@ -55,7 +55,10 @@ func normalizeResourceType(resourceType string) (string, error) { return canonical, nil } - return "", fmt.Errorf("invalid resource type '%s'. Valid types: sandbox/sbx, job/j, agent/ag, function/fn/mcp", resourceType) + return "", core.MarkExpectedError( + fmt.Errorf("invalid resource type '%s'. Valid types: sandbox/sbx, job/j, agent/ag, function/fn/mcp", resourceType), + core.CLIErrorValidation, + ) } // parseTimeFlag parses a time string flag value @@ -79,7 +82,10 @@ func parseTimeFlag(timeStr string) (time.Time, error) { return time.Date(t.Year(), t.Month(), t.Day(), 12, 0, 0, 0, time.UTC), nil } - return time.Time{}, fmt.Errorf("invalid time format '%s'. Use RFC3339 format (e.g., 2006-01-02T15:04:05Z) or YYYY-MM-DD", timeStr) + return time.Time{}, core.MarkExpectedError( + fmt.Errorf("invalid time format '%s'. Use RFC3339 format (e.g., 2006-01-02T15:04:05Z) or YYYY-MM-DD", timeStr), + core.CLIErrorValidation, + ) } // validateTimeRange ensures the time range doesn't exceed 3 days @@ -88,11 +94,17 @@ func validateTimeRange(start, end time.Time) error { maxDuration := 3 * 24 * time.Hour // 3 days if duration > maxDuration { - return fmt.Errorf("time range exceeds maximum of 3 days (requested: %v)", duration) + return core.MarkExpectedError( + fmt.Errorf("time range exceeds maximum of 3 days (requested: %v)", duration), + core.CLIErrorValidation, + ) } if duration < 0 { - return fmt.Errorf("start time must be before end time") + return core.MarkExpectedError( + fmt.Errorf("start time must be before end time"), + core.CLIErrorValidation, + ) } return nil @@ -258,14 +270,14 @@ Examples: // Use explicit start and end times startTime, err = parseTimeFlag(startTimeStr) if err != nil { - err = fmt.Errorf("invalid start time: %v", err) + err = fmt.Errorf("invalid start time: %w", err) core.PrintError("logs", err) core.ExitWithError(err) } endTime, err = parseTimeFlag(endTimeStr) if err != nil { - err = fmt.Errorf("invalid end time: %v", err) + err = fmt.Errorf("invalid end time: %w", err) core.PrintError("logs", err) core.ExitWithError(err) } @@ -283,7 +295,7 @@ Examples: // Only start time provided startTime, err = parseTimeFlag(startTimeStr) if err != nil { - err = fmt.Errorf("invalid start time: %v", err) + err = fmt.Errorf("invalid start time: %w", err) core.PrintError("logs", err) core.ExitWithError(err) } diff --git a/cli/monitor/logs.go b/cli/monitor/logs.go index a3839a72..1fdde863 100644 --- a/cli/monitor/logs.go +++ b/cli/monitor/logs.go @@ -223,7 +223,10 @@ func (w *BuildLogWatcher) fetchBuildLogs(offset int) ([]bufferedLogEntry, error) _, ok := response[w.resourceName] if !ok { - return nil, fmt.Errorf("resource %s not found", w.resourceName) + return nil, core.MarkExpectedError( + fmt.Errorf("resource %s not found", w.resourceName), + core.CLIErrorNotFound, + ) } // Extract log entries with parsed timestamps for proper ordering diff --git a/cli/push.go b/cli/push.go index 7f8e3801..474de755 100644 --- a/cli/push.go +++ b/cli/push.go @@ -2,6 +2,7 @@ package cli import ( "context" + "errors" "fmt" "net/http" "os" @@ -151,7 +152,10 @@ For private registries, supply credentials via --registry-cred or --docker-confi if resourceType == "" { if noTTY { core.PrintError("Push", fmt.Errorf("resource type is required. Specify it with --type (-t) flag or set 'type' in blaxel.toml")) - core.ExitWithError(fmt.Errorf("resource type is required")) + core.ExitWithError(core.MarkExpectedError( + fmt.Errorf("resource type is required"), + core.CLIErrorValidation, + )) } // Interactive prompt for resource type var selected string @@ -179,7 +183,10 @@ For private registries, supply credentials via --registry-cred or --docker-confi validTypes := map[string]bool{"agent": true, "function": true, "sandbox": true, "job": true} if !validTypes[resourceType] { core.PrintError("Push", fmt.Errorf("invalid resource type %q: must be one of sandbox, agent, job, function", resourceType)) - core.ExitWithError(fmt.Errorf("invalid resource type")) + core.ExitWithError(core.MarkExpectedError( + fmt.Errorf("invalid resource type"), + core.CLIErrorValidation, + )) } // Parse timeout early to fail fast before expensive upload @@ -192,7 +199,10 @@ For private registries, supply credentials via --registry-cred or --docker-confi } if parsed <= 0 { core.PrintError("Push", fmt.Errorf("timeout must be a positive duration, got %q", timeoutStr)) - core.ExitWithError(fmt.Errorf("invalid timeout")) + core.ExitWithError(core.MarkExpectedError( + fmt.Errorf("invalid timeout"), + core.CLIErrorValidation, + )) } buildTimeout = parsed } @@ -448,9 +458,15 @@ func watchBuildLogsNonInteractive(resourceType, name string, noTTY bool, buildTi for { select { case <-ctx.Done(): - return fmt.Errorf("build monitoring cancelled") + return core.MarkExpectedError( + fmt.Errorf("build monitoring cancelled"), + core.CLIErrorOperational, + ) case <-timeout: - return fmt.Errorf("build timed out after %s", buildTimeout) + return core.MarkExpectedError( + fmt.Errorf("build timed out after %s", buildTimeout), + core.CLIErrorOperational, + ) case <-ticker.C: // Check if the image exists in the registry (build completed) status, err := getImageBuildStatus(resourceType, name) @@ -467,7 +483,10 @@ func watchBuildLogsNonInteractive(resourceType, name string, noTTY bool, buildTi if status == "failed" { logWatcher.Stop() time.Sleep(1 * time.Second) - return fmt.Errorf("image build failed") + return core.MarkExpectedError( + fmt.Errorf("image build failed"), + core.CLIErrorOperational, + ) } } } @@ -496,8 +515,7 @@ func getImageBuildStatus(resourceType, name string) (string, error) { } err := client.Get(ctx, path, nil, &result) if err != nil { - errStr := err.Error() - if strings.Contains(errStr, "404") || strings.Contains(errStr, "not found") { + if isAPIStatus(err, http.StatusNotFound) { return "", nil // Not found yet, build may still be in progress } return "", err @@ -513,6 +531,11 @@ func getImageBuildStatus(resourceType, name string) (string, error) { } } +func isAPIStatus(err error, statusCode int) bool { + var apiErr *blaxel.Error + return errors.As(err, &apiErr) && apiErr.StatusCode == statusCode +} + func imageRef(resourceType, name string) string { if resourceType != "" { return resourceType + "/" + name diff --git a/cli/push_test.go b/cli/push_test.go index cc8ed8c3..34cbc650 100644 --- a/cli/push_test.go +++ b/cli/push_test.go @@ -1,11 +1,21 @@ package cli import ( + "errors" + "fmt" + "net/http" "testing" + blaxel "github.com/blaxel-ai/sdk-go" "github.com/stretchr/testify/assert" ) +func TestIsAPIStatusUsesTypedErrorIdentity(t *testing.T) { + assert.True(t, isAPIStatus(fmt.Errorf("image lookup: %w", &blaxel.Error{StatusCode: http.StatusNotFound}), http.StatusNotFound)) + assert.False(t, isAPIStatus(errors.New("internal cache entry not found (404)"), http.StatusNotFound)) + assert.False(t, isAPIStatus(&blaxel.Error{StatusCode: http.StatusInternalServerError}, http.StatusNotFound)) +} + func TestImageRefToName(t *testing.T) { tests := []struct { name string diff --git a/cli/run.go b/cli/run.go index 7e654c65..68b94934 100644 --- a/cli/run.go +++ b/cli/run.go @@ -150,7 +150,10 @@ This is useful for testing specific endpoints or non-standard API calls.`, bl run sbx my-sandbox --path /process --data '{"command": "python script.py", "waitForCompletion": true}'`, Run: func(cmd *cobra.Command, args []string) { if len(args) == 0 || len(args) == 1 { - err := fmt.Errorf("resource type and name are required") + err := core.MarkExpectedError( + fmt.Errorf("resource type and name are required"), + core.CLIErrorUsage, + ) core.PrintError("Run", err) core.ExitWithError(err) } @@ -167,7 +170,10 @@ This is useful for testing specific endpoints or non-standard API calls.`, for _, header := range headerFlags { parts := strings.SplitN(header, ":", 2) if len(parts) != 2 { - err := fmt.Errorf("invalid header format '%s'. Must be 'Key: Value'", header) + err := core.MarkExpectedError( + fmt.Errorf("invalid header format '%s'. Must be 'Key: Value'", header), + core.CLIErrorValidation, + ) core.PrintError("Run", err) core.ExitWithError(err) } @@ -188,12 +194,12 @@ This is useful for testing specific endpoints or non-standard API calls.`, var yamlData interface{} if err := yaml.Unmarshal(fileContent, &yamlData); err != nil { core.PrintError("Run", fmt.Errorf("error parsing YAML file: %w", err)) - core.ExitWithError(err) + core.ExitWithError(core.MarkExpectedError(err, core.CLIErrorValidation)) } jsonBytes, err := json.Marshal(yamlData) if err != nil { core.PrintError("Run", fmt.Errorf("error converting YAML to JSON: %w", err)) - core.ExitWithError(err) + core.ExitWithError(core.MarkExpectedError(err, core.CLIErrorValidation)) } data = string(jsonBytes) } else { @@ -284,7 +290,10 @@ This is useful for testing specific endpoints or non-standard API calls.`, ) if err != nil { if ctx.Err() == context.DeadlineExceeded { - err = fmt.Errorf("request timed out after %ds", timeout) + err = core.MarkExpectedError( + fmt.Errorf("request timed out after %ds", timeout), + core.CLIErrorOperational, + ) } else { err = fmt.Errorf("error making request: %w", err) } @@ -325,7 +334,10 @@ This is useful for testing specific endpoints or non-standard API calls.`, }) if err != nil { if ctx.Err() == context.DeadlineExceeded { - err = fmt.Errorf("request timed out after %ds", timeout) + err = core.MarkExpectedError( + fmt.Errorf("request timed out after %ds", timeout), + core.CLIErrorOperational, + ) } core.PrintError("Run", fmt.Errorf("error reading stream: %w", err)) core.ExitWithError(err) @@ -446,9 +458,12 @@ func validateInlineRunDataJSON(data, resourceType, path string) error { var raw json.RawMessage if err := json.Unmarshal([]byte(data), &raw); err != nil { - return fmt.Errorf( - "invalid JSON passed to --data: %v. For sandbox /process payloads with nested quotes, backslashes, or newlines, write the JSON to a file and pass it via --file to avoid shell-escaping collisions", - err, + return core.MarkExpectedError( + fmt.Errorf( + "invalid JSON passed to --data: %w. For sandbox /process payloads with nested quotes, backslashes, or newlines, write the JSON to a file and pass it via --file to avoid shell-escaping collisions", + err, + ), + core.CLIErrorValidation, ) } return nil @@ -571,7 +586,10 @@ func runJobLocally(data string, folder string, config core.Config, concurrent in batch := Batch{} err := json.Unmarshal([]byte(data), &batch) if err != nil { - err = fmt.Errorf("invalid JSON: %w", err) + err = core.MarkExpectedError( + fmt.Errorf("invalid JSON: %w", err), + core.CLIErrorValidation, + ) core.PrintError("Run", err) core.ExitWithError(err) } diff --git a/cli/serve.go b/cli/serve.go index 29fecec1..76fd6b9b 100644 --- a/cli/serve.go +++ b/cli/serve.go @@ -213,7 +213,10 @@ Workflow: core.Print("echo '[entrypoint]\\nprod = \"your-command\"' > blaxel.toml") } } - core.ExitWithError(fmt.Errorf("cannot start server: no entrypoint configured and no language detected")) + core.ExitWithError(core.MarkExpectedError( + fmt.Errorf("cannot start server: no entrypoint configured and no language detected"), + core.CLIErrorValidation, + )) } } diff --git a/cli/server/commands.go b/cli/server/commands.go index aec2026d..cfd45969 100644 --- a/cli/server/commands.go +++ b/cli/server/commands.go @@ -19,7 +19,7 @@ func FindRootCmd(port int, host string, hotreload bool, folder string, config co Envs: GetServerEnvironment(port, host, hotreload, config), }) if err != nil { - return nil, fmt.Errorf("error finding root cmd: %v", err) + return nil, fmt.Errorf("error finding root cmd: %w", err) } return exec.Command("sh", "-c", strings.Join(rootCmd, " ")), nil } @@ -67,21 +67,27 @@ func FindRootCmdAsString(cfg RootCmdConfig) ([]string, error) { return findGoRootCmdAsString(cfg) default: if cfg.Hotreload { - return nil, fmt.Errorf("no dev entrypoint configured and language not supported") + return nil, core.MarkExpectedError( + fmt.Errorf("no dev entrypoint configured and language not supported"), + core.CLIErrorValidation, + ) } - return nil, fmt.Errorf("no prod entrypoint configured and language not supported") + return nil, core.MarkExpectedError( + fmt.Errorf("no prod entrypoint configured and language not supported"), + core.CLIErrorValidation, + ) } } func FindJobCommand(task map[string]interface{}, folder string, config core.Config) (*exec.Cmd, error) { rootCmd, err := FindRootCmd(0, "localhost", false, folder, config) if err != nil { - return nil, fmt.Errorf("error finding root cmd: %v", err) + return nil, fmt.Errorf("error finding root cmd: %w", err) } for arg := range task { jsonencoded, err := json.Marshal(task[arg]) if err != nil { - return nil, fmt.Errorf("error marshalling task: %v", err) + return nil, fmt.Errorf("error marshalling task: %w", err) } lastArg := rootCmd.Args[len(rootCmd.Args)-1] lastArg = strings.Join([]string{lastArg, "--" + arg, string(jsonencoded)}, " ") diff --git a/cli/server/commands_go.go b/cli/server/commands_go.go index 686229dc..72a21311 100644 --- a/cli/server/commands_go.go +++ b/cli/server/commands_go.go @@ -85,7 +85,10 @@ func findGoRootCmdAsString(cfg RootCmdConfig) ([]string, error) { if entryFile != "" { return []string{"go", "run", goRunTargetFromEntryFile(entryFile)}, nil } - return nil, fmt.Errorf("entrypoint not found in config") + return nil, core.MarkExpectedError( + fmt.Errorf("entrypoint not found in config"), + core.CLIErrorNotFound, + ) } func goRunTargetFromEntryFile(entryFile string) string { diff --git a/cli/server/commands_python.go b/cli/server/commands_python.go index d6121247..9489ec12 100644 --- a/cli/server/commands_python.go +++ b/cli/server/commands_python.go @@ -114,7 +114,10 @@ func findPythonRootCmdAsString(cfg RootCmdConfig) ([]string, error) { file := FindPythonEntryFile(cfg.Folder) if file == "" { - return nil, fmt.Errorf("app.py or main.py not found in current directory") + return nil, core.MarkExpectedError( + fmt.Errorf("app.py or main.py not found in current directory"), + core.CLIErrorNotFound, + ) } venv := ".venv" diff --git a/cli/server/commands_ts.go b/cli/server/commands_ts.go index a20d20b1..6d63c4bc 100644 --- a/cli/server/commands_ts.go +++ b/cli/server/commands_ts.go @@ -95,22 +95,28 @@ func StartTypescriptServer(port int, host string, hotreload bool, folder string, func getPackageJson(folder string) (PackageJson, error) { currentDir, err := os.Getwd() if err != nil { - return PackageJson{}, fmt.Errorf("error getting current directory: %v", err) + return PackageJson{}, fmt.Errorf("error getting current directory: %w", err) } packageJsonPath := filepath.Join(currentDir, folder, "package.json") if _, err := os.Stat(packageJsonPath); err == nil { packageJson, err := os.ReadFile(packageJsonPath) if err != nil { - return PackageJson{}, fmt.Errorf("error reading package.json: %v", err) + return PackageJson{}, fmt.Errorf("error reading package.json: %w", err) } var packageJsonObj PackageJson err = json.Unmarshal(packageJson, &packageJsonObj) if err != nil { - return PackageJson{}, fmt.Errorf("error unmarshalling package.json: %v", err) + return PackageJson{}, core.MarkExpectedError( + fmt.Errorf("error unmarshalling package.json: %w", err), + core.CLIErrorValidation, + ) } return packageJsonObj, nil } - return PackageJson{}, fmt.Errorf("package.json not found in current directory") + return PackageJson{}, core.MarkExpectedError( + fmt.Errorf("package.json not found in current directory"), + core.CLIErrorNotFound, + ) } func findTSPackageManagerLockFile() string { @@ -215,5 +221,8 @@ func findTSRootCmdAsString(config RootCmdConfig) ([]string, error) { return []string{nodeExec, file}, nil } } - return nil, fmt.Errorf("index.js, index.ts, app.js or app.ts not found in current directory") + return nil, core.MarkExpectedError( + fmt.Errorf("index.js, index.ts, app.js or app.ts not found in current directory"), + core.CLIErrorNotFound, + ) } diff --git a/cli/server/serve_package.go b/cli/server/serve_package.go index bdd543b4..1f5d7430 100644 --- a/cli/server/serve_package.go +++ b/cli/server/serve_package.go @@ -146,7 +146,7 @@ func getServeCommands(port int, host string, hotreload bool, config core.Config, usedPorts := make(map[int]bool) pwd, err := os.Getwd() if err != nil { - return nil, fmt.Errorf("error getting current directory: %v", err) + return nil, fmt.Errorf("error getting current directory: %w", err) } colors := []string{"red", "green", "blue", "yellow", "purple", "cyan", "white"} command := PackageCommand{ diff --git a/cli/token.go b/cli/token.go index 89ae4c06..de03a6db 100644 --- a/cli/token.go +++ b/cli/token.go @@ -68,7 +68,10 @@ export TOKEN=$(bl token) // Validate workspace if workspace == "" { - err := fmt.Errorf("no workspace specified. Use 'bl login ' to authenticate") + err := core.MarkExpectedError( + fmt.Errorf("no workspace specified. Use 'bl login ' to authenticate"), + core.CLIErrorAuthentication, + ) core.PrintError("token", err) core.ExitWithError(err) } @@ -95,7 +98,10 @@ export TOKEN=$(bl token) core.ExitWithError(err) } if !credentials.IsValid() { - err := fmt.Errorf("no valid credentials found for workspace '%s'. Please run 'bl login %s'", workspace, workspace) + err := core.MarkExpectedError( + fmt.Errorf("no valid credentials found for workspace '%s'. Please run 'bl login %s'", workspace, workspace), + core.CLIErrorAuthentication, + ) core.PrintError("token", err) core.ExitWithError(err) } From e1a0716e6078cfa68f8d8cc4345ff66dfa831cc3 Mon Sep 17 00:00:00 2001 From: Michael Stolarz Date: Tue, 21 Jul 2026 10:27:25 -0700 Subject: [PATCH 3/3] fix(cli): retry transient mounted-drive reads (ENG-4049) --- cli/drive.go | 201 ++++++++++++++++++++-- cli/drive_retry_test.go | 373 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 556 insertions(+), 18 deletions(-) create mode 100644 cli/drive_retry_test.go diff --git a/cli/drive.go b/cli/drive.go index 3d6c331c..a51489fe 100644 --- a/cli/drive.go +++ b/cli/drive.go @@ -9,6 +9,7 @@ import ( "net/http" "net/url" "strings" + "time" blaxel "github.com/blaxel-ai/sdk-go" "github.com/blaxel-ai/toolkit/cli/core" @@ -71,6 +72,45 @@ type sandboxAPIError struct { Error string `json:"error"` } +// sandboxGatewayErrorResponse is the typed error envelope emitted by the +// Blaxel gateway. It is deliberately separate from sandboxAPIError because +// workload-owned APIs may still return the legacy {"error": "..."} shape. +type sandboxGatewayErrorResponse struct { + Error sandboxGatewayError `json:"error"` +} + +type sandboxGatewayError struct { + Code string `json:"code"` + Message string `json:"message"` + Origin string `json:"origin"` + Retryable bool `json:"retryable"` + Action string `json:"action"` +} + +type sandboxReadResponse struct { + StatusCode int + Body []byte + PlatformWorkloadUnavailable bool +} + +type sandboxRetryPolicy struct { + InitialBackoff time.Duration + MaxBackoff time.Duration + TotalBudget time.Duration + HTTPClient *http.Client + Now func() time.Time + Sleep func(context.Context, time.Duration) error +} + +var defaultSandboxReadRetryPolicy = sandboxRetryPolicy{ + InitialBackoff: 500 * time.Millisecond, + MaxBackoff: 30 * time.Second, + TotalBudget: 60 * time.Second, + HTTPClient: http.DefaultClient, + Now: time.Now, + Sleep: sleepWithContext, +} + func DriveCmd() *cobra.Command { cmd := &cobra.Command{ Use: "drive", @@ -295,25 +335,17 @@ func DriveMountsCmd() *cobra.Command { sandboxURL, token := resolveSandbox(ctx, sandboxName) - resp, err := sandboxRequest(ctx, http.MethodGet, sandboxURL, "/drives/mount", token, nil) + resp, err := sandboxReadRequestWithRetry(ctx, sandboxURL, "/drives/mount", token) if err != nil { core.PrintError("Drive mounts", err) core.ExitWithError(err) } - defer func() { _ = resp.Body.Close() }() - - respBody, err := io.ReadAll(resp.Body) - if err != nil { - core.PrintError("Drive mounts", fmt.Errorf("failed to read response: %w", err)) - core.ExitWithError(err) - } - if resp.StatusCode != http.StatusOK { - handleSandboxAPIError(respBody, resp.StatusCode, "list mounted drives") + handleSandboxReadAPIError(resp, "list mounted drives") } var listResp driveListResponse - if err := json.Unmarshal(respBody, &listResp); err != nil { + if err := json.Unmarshal(resp.Body, &listResp); err != nil { core.PrintError("Drive mounts", fmt.Errorf("failed to parse response: %w", err)) core.ExitWithError(err) } @@ -590,6 +622,10 @@ func resolveSandbox(ctx context.Context, sandboxName string) (sandboxURL, token // sandboxRequest makes an authenticated HTTP request to the sandbox API. func sandboxRequest(ctx context.Context, method, sandboxURL, path, token string, body io.Reader) (*http.Response, error) { + return sandboxRequestWithClient(ctx, http.DefaultClient, method, sandboxURL, path, token, body) +} + +func sandboxRequestWithClient(ctx context.Context, client *http.Client, method, sandboxURL, path, token string, body io.Reader) (*http.Response, error) { url := strings.TrimSuffix(sandboxURL, "/") + path req, err := http.NewRequestWithContext(ctx, method, url, body) @@ -599,7 +635,7 @@ func sandboxRequest(ctx context.Context, method, sandboxURL, path, token string, req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) req.Header.Set("Content-Type", "application/json") - resp, err := http.DefaultClient.Do(req) + resp, err := client.Do(req) if err != nil { return nil, fmt.Errorf("request to sandbox failed: %w", err) } @@ -607,17 +643,146 @@ func sandboxRequest(ctx context.Context, method, sandboxURL, path, token string, return resp, nil } +// sandboxReadRequestWithRetry retries only the idempotent GET used to list +// mounted drives. Keeping the method out of this API makes it impossible for a +// caller to accidentally reuse this policy for a mutation. +func sandboxReadRequestWithRetry(ctx context.Context, sandboxURL, path, token string) (*sandboxReadResponse, error) { + return sandboxReadRequestWithRetryPolicy( + ctx, + sandboxURL, + path, + token, + defaultSandboxReadRetryPolicy, + func() { core.PrintInfo("Waiting for sandbox to become available...") }, + ) +} + +func sandboxReadRequestWithRetryPolicy( + ctx context.Context, + sandboxURL string, + path string, + token string, + policy sandboxRetryPolicy, + onFirstRetry func(), +) (*sandboxReadResponse, error) { + if policy.InitialBackoff <= 0 || policy.MaxBackoff <= 0 || policy.TotalBudget <= 0 || policy.HTTPClient == nil || policy.Now == nil || policy.Sleep == nil { + return nil, fmt.Errorf("invalid sandbox retry policy") + } + + startedAt := policy.Now() + backoff := policy.InitialBackoff + retried := false + + for { + resp, err := sandboxRequestWithClient(ctx, policy.HTTPClient, http.MethodGet, sandboxURL, path, token, nil) + if err != nil { + return nil, err + } + + respBody, readErr := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if readErr != nil { + return nil, fmt.Errorf("failed to read sandbox response: %w", readErr) + } + + platformWorkloadUnavailable := isRetryableWorkloadUnavailable(resp, respBody) + result := &sandboxReadResponse{ + StatusCode: resp.StatusCode, + Body: respBody, + PlatformWorkloadUnavailable: platformWorkloadUnavailable, + } + if !platformWorkloadUnavailable { + return result, nil + } + + remaining := policy.TotalBudget - policy.Now().Sub(startedAt) + if remaining <= 0 { + return result, nil + } + + delay := min(backoff, policy.MaxBackoff, remaining) + if !retried { + if onFirstRetry != nil { + onFirstRetry() + } + retried = true + } + + if err := policy.Sleep(ctx, delay); err != nil { + return nil, fmt.Errorf("sandbox retry interrupted: %w", err) + } + backoff = min(backoff*2, policy.MaxBackoff) + } +} + +func isRetryableWorkloadUnavailable(resp *http.Response, body []byte) bool { + if resp.StatusCode != http.StatusNotFound { + return false + } + + var gatewayErr sandboxGatewayErrorResponse + if err := json.Unmarshal(body, &gatewayErr); err != nil { + return false + } + + isPlatformError := strings.EqualFold(resp.Header.Get("X-Blaxel-Source"), "platform") || + strings.EqualFold(gatewayErr.Error.Origin, "platform") + return isPlatformError && + gatewayErr.Error.Code == "WORKLOAD_UNAVAILABLE" && + gatewayErr.Error.Retryable +} + +func sleepWithContext(ctx context.Context, delay time.Duration) error { + timer := time.NewTimer(delay) + defer timer.Stop() + + select { + case <-ctx.Done(): + return ctx.Err() + case <-timer.C: + return nil + } +} + // handleSandboxAPIError extracts an error message from a sandbox API response and exits. func handleSandboxAPIError(body []byte, statusCode int, operation string) { + err := core.MarkExpectedHTTPError(newSandboxAPIError(body, statusCode, operation), statusCode) + core.PrintError("Drive", err) + core.ExitWithError(err) +} + +func handleSandboxReadAPIError(response *sandboxReadResponse, operation string) { + err := classifySandboxReadAPIError(response, operation) + core.PrintError("Drive", err) + core.ExitWithError(err) +} + +func classifySandboxReadAPIError(response *sandboxReadResponse, operation string) error { + err := newSandboxAPIError(response.Body, response.StatusCode, operation) + if response.PlatformWorkloadUnavailable { + return err + } + return core.MarkExpectedHTTPError(err, response.StatusCode) +} + +func newSandboxAPIError(body []byte, statusCode int, operation string) error { + var gatewayErr sandboxGatewayErrorResponse + if err := json.Unmarshal(body, &gatewayErr); err == nil && gatewayErr.Error.Message != "" { + detail := gatewayErr.Error.Message + if gatewayErr.Error.Action != "" { + detail = fmt.Sprintf("%s. %s", strings.TrimSuffix(detail, "."), gatewayErr.Error.Action) + } + if gatewayErr.Error.Code != "" { + return fmt.Errorf("failed to %s (HTTP %d, %s): %s", operation, statusCode, gatewayErr.Error.Code, detail) + } + return fmt.Errorf("failed to %s (HTTP %d): %s", operation, statusCode, detail) + } + var apiErr sandboxAPIError if err := json.Unmarshal(body, &apiErr); err == nil && apiErr.Error != "" { - err := fmt.Errorf("failed to %s (HTTP %d): %s", operation, statusCode, apiErr.Error) - core.PrintError("Drive", err) - core.ExitWithError(core.MarkExpectedHTTPError(err, statusCode)) + return fmt.Errorf("failed to %s (HTTP %d): %s", operation, statusCode, apiErr.Error) } - err := fmt.Errorf("failed to %s (HTTP %d): %s", operation, statusCode, string(body)) - core.PrintError("Drive", err) - core.ExitWithError(core.MarkExpectedHTTPError(err, statusCode)) + return fmt.Errorf("failed to %s (HTTP %d): %s", operation, statusCode, string(body)) } // outputDriveData marshals the given data to JSON or YAML format and prints it. diff --git a/cli/drive_retry_test.go b/cli/drive_retry_test.go new file mode 100644 index 00000000..6061d906 --- /dev/null +++ b/cli/drive_retry_test.go @@ -0,0 +1,373 @@ +package cli + +import ( + "context" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/blaxel-ai/toolkit/cli/core" + "github.com/stretchr/testify/require" +) + +const retryableWorkloadUnavailableBody = `{ + "error": { + "code": "WORKLOAD_UNAVAILABLE", + "message": "The sandbox is temporarily unavailable", + "origin": "platform", + "retryable": true, + "action": "Retry with exponential backoff" + } +}` + +type fakeSandboxRetryClock struct { + now time.Time + sleeps []time.Duration +} + +func (c *fakeSandboxRetryClock) Now() time.Time { + return c.now +} + +func (c *fakeSandboxRetryClock) Sleep(_ context.Context, delay time.Duration) error { + c.sleeps = append(c.sleeps, delay) + c.now = c.now.Add(delay) + return nil +} + +func testSandboxRetryPolicy(clock *fakeSandboxRetryClock) sandboxRetryPolicy { + return sandboxRetryPolicy{ + InitialBackoff: 500 * time.Millisecond, + MaxBackoff: 30 * time.Second, + TotalBudget: 60 * time.Second, + HTTPClient: http.DefaultClient, + Now: clock.Now, + Sleep: clock.Sleep, + } +} + +func TestSandboxReadRequestRetriesSequentiallyUntilSuccess(t *testing.T) { + var calls atomic.Int32 + var active atomic.Int32 + var maxActive atomic.Int32 + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + current := active.Add(1) + defer active.Add(-1) + for { + previous := maxActive.Load() + if current <= previous || maxActive.CompareAndSwap(previous, current) { + break + } + } + + if r.Method != http.MethodGet { + t.Errorf("request method = %q, want %q", r.Method, http.MethodGet) + } + if got := r.Header.Get("Authorization"); got != "Bearer test-token" { + t.Errorf("Authorization header = %q, want %q", got, "Bearer test-token") + } + attempt := calls.Add(1) + if attempt < 3 { + w.Header().Set("X-Blaxel-Source", "platform") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(retryableWorkloadUnavailableBody)) + return + } + _, _ = w.Write([]byte(`{"mounts":[]}`)) + })) + defer server.Close() + + clock := &fakeSandboxRetryClock{now: time.Unix(0, 0)} + firstRetryCalls := 0 + response, err := sandboxReadRequestWithRetryPolicy( + context.Background(), + server.URL, + "/drives/mount", + "test-token", + testSandboxRetryPolicy(clock), + func() { firstRetryCalls++ }, + ) + + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode) + require.False(t, response.PlatformWorkloadUnavailable) + require.JSONEq(t, `{"mounts":[]}`, string(response.Body)) + require.EqualValues(t, 3, calls.Load()) + require.EqualValues(t, 1, maxActive.Load()) + require.Equal(t, []time.Duration{500 * time.Millisecond, time.Second}, clock.sleeps) + require.Equal(t, 1, firstRetryCalls) +} + +func TestRetryableWorkloadUnavailableAcceptsCanonicalProvenanceFallbacks(t *testing.T) { + tests := []struct { + name string + header string + body string + want bool + }{ + { + name: "canonical response header", + header: "platform", + body: `{"error":{"code":"WORKLOAD_UNAVAILABLE","retryable":true}}`, + want: true, + }, + { + name: "typed envelope origin", + body: retryableWorkloadUnavailableBody, + want: true, + }, + { + name: "no platform provenance", + body: `{"error":{"code":"WORKLOAD_UNAVAILABLE","retryable":true}}`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resp := &http.Response{ + StatusCode: http.StatusNotFound, + Header: make(http.Header), + } + if tt.header != "" { + resp.Header.Set("X-Blaxel-Source", tt.header) + } + + require.Equal(t, tt.want, isRetryableWorkloadUnavailable(resp, []byte(tt.body))) + }) + } +} + +func TestSandboxReadRequestDoesNotRetryUntrustedOrNonRetryableErrors(t *testing.T) { + tests := []struct { + name string + body string + header string + status int + }{ + { + name: "non-retryable gateway error", + body: `{"error":{"code":"WORKLOAD_NOT_FOUND","message":"missing","origin":"platform","retryable":false}}`, + }, + { + name: "workload-owned lookalike", + body: `{"error":{"code":"WORKLOAD_UNAVAILABLE","message":"not platform owned","retryable":true}}`, + }, + { + name: "platform header with wrong code", + body: `{"error":{"code":"BAD_REQUEST","message":"bad request","retryable":true}}`, + header: "platform", + }, + { + name: "retryable envelope with wrong status", + body: retryableWorkloadUnavailableBody, + header: "platform", + status: http.StatusInternalServerError, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + calls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + calls++ + if tt.header != "" { + w.Header().Set("X-Blaxel-Source", tt.header) + } + status := tt.status + if status == 0 { + status = http.StatusNotFound + } + w.WriteHeader(status) + _, _ = w.Write([]byte(tt.body)) + })) + defer server.Close() + + clock := &fakeSandboxRetryClock{now: time.Unix(0, 0)} + response, err := sandboxReadRequestWithRetryPolicy( + context.Background(), server.URL, "/drives/mount", "token", testSandboxRetryPolicy(clock), nil, + ) + + require.NoError(t, err) + wantStatus := tt.status + if wantStatus == 0 { + wantStatus = http.StatusNotFound + } + require.Equal(t, wantStatus, response.StatusCode) + require.Equal(t, 1, calls) + require.Empty(t, clock.sleeps) + require.True(t, core.IsExpectedCLIError(classifySandboxReadAPIError(response, "list mounted drives"))) + }) + } +} + +func TestSandboxReadRequestStopsAtRetryBudget(t *testing.T) { + calls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + calls++ + w.Header().Set("X-Blaxel-Source", "platform") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(retryableWorkloadUnavailableBody)) + })) + defer server.Close() + + clock := &fakeSandboxRetryClock{now: time.Unix(0, 0)} + response, err := sandboxReadRequestWithRetryPolicy( + context.Background(), server.URL, "/drives/mount", "token", testSandboxRetryPolicy(clock), nil, + ) + + require.NoError(t, err) + require.Equal(t, http.StatusNotFound, response.StatusCode) + require.True(t, response.PlatformWorkloadUnavailable) + require.Equal(t, 8, calls) + require.Equal(t, []time.Duration{ + 500 * time.Millisecond, + time.Second, + 2 * time.Second, + 4 * time.Second, + 8 * time.Second, + 16 * time.Second, + 28*time.Second + 500*time.Millisecond, + }, clock.sleeps) + require.Equal(t, 60*time.Second, clock.Now().Sub(time.Unix(0, 0))) + + finalErr := classifySandboxReadAPIError(response, "list mounted drives") + require.ErrorContains(t, finalErr, "HTTP 404, WORKLOAD_UNAVAILABLE") + require.ErrorContains(t, finalErr, "temporarily unavailable") + require.ErrorContains(t, finalErr, "Retry with exponential backoff") + require.False(t, core.IsExpectedCLIError(finalErr)) +} + +func TestSandboxReadRequestHonorsCancellationDuringBackoff(t *testing.T) { + calls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + calls++ + w.Header().Set("X-Blaxel-Source", "platform") + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(retryableWorkloadUnavailableBody)) + })) + defer server.Close() + + ctx, cancel := context.WithCancel(context.Background()) + clock := &fakeSandboxRetryClock{now: time.Unix(0, 0)} + policy := testSandboxRetryPolicy(clock) + policy.Sleep = sleepWithContext + response, err := sandboxReadRequestWithRetryPolicy( + ctx, + server.URL, + "/drives/mount", + "token", + policy, + cancel, + ) + + require.Nil(t, response) + require.ErrorIs(t, err, context.Canceled) + require.Equal(t, 1, calls) +} + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (fn roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { + return fn(req) +} + +func TestSandboxReadRequestDoesNotRetryTransportErrors(t *testing.T) { + transportErr := errors.New("connection reset") + var calls atomic.Int32 + client := &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) { + calls.Add(1) + return nil, transportErr + })} + + clock := &fakeSandboxRetryClock{now: time.Unix(0, 0)} + policy := testSandboxRetryPolicy(clock) + policy.HTTPClient = client + response, err := sandboxReadRequestWithRetryPolicy( + context.Background(), "https://sandbox.example", "/drives/mount", "token", policy, nil, + ) + + require.Nil(t, response) + require.ErrorIs(t, err, transportErr) + require.EqualValues(t, 1, calls.Load()) + require.Empty(t, clock.sleeps) +} + +type closeTrackingBody struct { + io.Reader + closed *atomic.Bool + err error +} + +func (b *closeTrackingBody) Close() error { + b.closed.Store(true) + return b.err +} + +func TestSandboxReadRequestClosesEveryResponse(t *testing.T) { + firstClosed := &atomic.Bool{} + secondClosed := &atomic.Bool{} + calls := 0 + client := &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + calls++ + require.Equal(t, http.MethodGet, req.Method) + if calls == 1 { + return &http.Response{ + StatusCode: http.StatusNotFound, + Header: http.Header{"X-Blaxel-Source": []string{"platform"}}, + Body: &closeTrackingBody{ + Reader: strings.NewReader(retryableWorkloadUnavailableBody), + closed: firstClosed, + }, + Request: req, + }, nil + } + return &http.Response{ + StatusCode: http.StatusOK, + Header: make(http.Header), + Body: &closeTrackingBody{ + Reader: strings.NewReader(`{"mounts":[]}`), + closed: secondClosed, + err: errors.New("close failed after complete read"), + }, + Request: req, + }, nil + })} + + clock := &fakeSandboxRetryClock{now: time.Unix(0, 0)} + policy := testSandboxRetryPolicy(clock) + policy.HTTPClient = client + response, err := sandboxReadRequestWithRetryPolicy( + context.Background(), "https://sandbox.example", "/drives/mount", "token", policy, nil, + ) + + require.NoError(t, err) + require.Equal(t, http.StatusOK, response.StatusCode) + require.True(t, firstClosed.Load()) + require.True(t, secondClosed.Load()) + require.Equal(t, 2, calls) +} + +func TestNewSandboxAPIErrorSupportsLegacyAndTypedBodies(t *testing.T) { + tests := []struct { + name string + body string + want string + }{ + {name: "legacy", body: `{"error":"not found"}`, want: "HTTP 404): not found"}, + {name: "typed", body: retryableWorkloadUnavailableBody, want: "HTTP 404, WORKLOAD_UNAVAILABLE"}, + {name: "unstructured", body: "upstream unavailable", want: "HTTP 404): upstream unavailable"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := newSandboxAPIError([]byte(tt.body), http.StatusNotFound, "list mounted drives") + require.ErrorContains(t, err, tt.want) + }) + } +}