diff --git a/.golangci.yml b/.golangci.yml index 2a16994f5..f9a51eae8 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -65,10 +65,23 @@ linters: - forbidigo # errs-typed-only enforced on paths already migrated to errs.NewXxxError. # Add a path when its migration is complete. - - path-except: (internal/auth/|internal/errcompat/|internal/errclass/|internal/client/|internal/cmdutil/factory\.go|cmd/auth/|cmd/config/|cmd/service/|shortcuts/common/mcp_client\.go|shortcuts/calendar/helpers\.go) + - path-except: (internal/auth/|internal/errcompat/|internal/errclass/|internal/client/|internal/cmdutil/factory\.go|cmd/auth/|cmd/config/|cmd/service/|shortcuts/common/mcp_client\.go|shortcuts/calendar/helpers\.go|shortcuts/drive/) text: errs-typed-only linters: - forbidigo + # errs-no-bare-wrap enforced on paths fully migrated to typed final + # errors. Scoped separately from errs-typed-only because cmd/auth/, + # cmd/config/ still have residual fmt.Errorf and must not be caught. + - path-except: (shortcuts/drive/|shortcuts/calendar/helpers\.go|shortcuts/common/mcp_client\.go) + text: errs-no-bare-wrap + linters: + - forbidigo + # errs-no-legacy-helper is drive-only: the shared helpers it bans are + # still used by other domains until their later migration phase. + - path-except: (shortcuts/drive/) + text: errs-no-legacy-helper + linters: + - forbidigo settings: depguard: @@ -94,6 +107,23 @@ linters: msg: >- [errs-typed-only] use errs.NewXxxError(...) builder (see errs/types.go). + # ── legacy shared error helpers banned on drive ── + # These helpers internally produce legacy output.Err* shapes, so they + # are invisible to the errs-typed-only ban above. Drive has migrated its + # calls to typed errs.* (drive-local driveInputStatError / driveSaveError); + # this prevents reintroduction. Other domains still use the shared + # helpers (migrated globally in a later phase), so this is drive-scoped. + - pattern: (common\.FlagErrorf|common\.WrapInputStatError|common\.WrapSaveErrorByCategory)\b + msg: >- + [errs-no-legacy-helper] these shared helpers emit legacy output.Err* + shapes. Use the typed errs.NewXxxError builders or the drive-local + driveInputStatError / driveSaveError helpers (shortcuts/drive/drive_errors.go). + # ── bare error wraps banned on fully-typed paths ── + - pattern: (fmt\.Errorf|errors\.New)\b + msg: >- + [errs-no-bare-wrap] final errors must be typed (errs.NewXxxError); + wrap a cause with .WithCause(err). Genuine intermediate wraps: + //nolint:forbidigo with a reason. # ── http: shortcuts must not construct raw HTTP requests ── # Bans request / client construction; constants (http.MethodPost, # http.StatusOK) and pure helpers (http.StatusText, http.Header) are diff --git a/cmd/root.go b/cmd/root.go index e86ff069e..af655a394 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -255,6 +255,13 @@ func handleRootError(f *cmdutil.Factory, err error) int { return typedExit } + // Partial-failure (batch / multi-status): the ok:false result envelope is + // already on stdout; set the exit code and write nothing to stderr. + var pfErr *output.PartialFailureError + if errors.As(err, &pfErr) { + return pfErr.Code + } + if exitErr := asExitError(err); exitErr != nil { if !exitErr.Raw { // Raw errors (e.g. from `api` command via output.MarkRaw) diff --git a/errs/ERROR_CONTRACT.md b/errs/ERROR_CONTRACT.md index db9169834..b13bea963 100644 --- a/errs/ERROR_CONTRACT.md +++ b/errs/ERROR_CONTRACT.md @@ -155,7 +155,30 @@ caller scripts. New code should not reach for `ErrBare` unless the command is genuinely a predicate. Anything carrying recoverable error content -belongs in a typed `*errs.XxxError`. +belongs in a typed `*errs.XxxError` — or, for a batch result, in the +partial-failure outcome below. + +### Partial failure (batch / multi-status) + +A batch command (e.g. `drive +push` / `+pull` / `+sync`) that processes +many items can finish in a third state, neither full success nor a single +error: some items succeeded and some failed. Its primary output is the +per-item result, so it does **not** belong in a `stderr` error envelope. + +Such a command returns `runtime.OutPartialFailure(data, meta)`, which: + +1. writes the full result to **stdout** as an `ok:false` envelope — the + summary and every per-item outcome (succeeded *and* failed) stay + machine-readable, exactly as a successful `Out(...)` would carry them, + but with `ok` honestly reporting failure; and +2. returns `*output.PartialFailureError`, a typed exit signal the + dispatcher maps to a non-zero exit code while writing nothing further + to `stderr`. + +This is distinct from `ErrBare` (a predicate's one-bit answer) and from a +typed `*errs.XxxError` (a `stderr` error envelope): a partial failure is a +*result*, reported on stdout, that also failed. Consumers branch on +`ok == false` and then read `data.summary` / `data.items[]`. ## Consumers diff --git a/errs/subtypes.go b/errs/subtypes.go index 21a9561b6..68a5dd377 100644 --- a/errs/subtypes.go +++ b/errs/subtypes.go @@ -12,7 +12,8 @@ const ( // CategoryValidation subtypes const ( - SubtypeInvalidArgument Subtype = "invalid_argument" // user-supplied flag / arg failed validation (gRPC INVALID_ARGUMENT alignment) + SubtypeInvalidArgument Subtype = "invalid_argument" // user-supplied flag / arg failed validation (gRPC INVALID_ARGUMENT alignment) + SubtypeFailedPrecondition Subtype = "failed_precondition" // request is valid but the system/resource state is not in the state required to execute; caller must change state (not retry) — e.g. ambiguous remote mapping (gRPC FAILED_PRECONDITION alignment) ) // CategoryAuthentication subtypes diff --git a/errs/types.go b/errs/types.go index e292098af..a253aa9e6 100644 --- a/errs/types.go +++ b/errs/types.go @@ -61,8 +61,22 @@ type TypedError interface { // it is intentionally not serialized. type ValidationError struct { Problem - Param string `json:"param,omitempty"` - Cause error `json:"-"` + Param string `json:"param,omitempty"` + Params []InvalidParam `json:"params,omitempty"` + Cause error `json:"-"` +} + +// InvalidParam is one structured validation diagnostic: the parameter that +// failed (Name) and why (Reason). It mirrors an RFC 7807 "invalid-params" +// item (RFC 7807 §3.1 extension members). +// +// The wire key on ValidationError is "params" rather than "invalid_params" +// because the enclosing envelope already carries type:"validation", so the +// "invalid" qualifier would be redundant on the wire. The Go type keeps the +// InvalidParam prefix because, at package level, the name must self-describe. +type InvalidParam struct { + Name string `json:"name"` + Reason string `json:"reason"` } // Unwrap exposes the wrapped cause so errors.Unwrap / errors.Is can traverse @@ -122,6 +136,11 @@ func (e *ValidationError) WithParam(param string) *ValidationError { return e } +func (e *ValidationError) WithParams(params ...InvalidParam) *ValidationError { + e.Params = append(e.Params, params...) + return e +} + func (e *ValidationError) WithCause(cause error) *ValidationError { e.Cause = cause return e diff --git a/errs/types_test.go b/errs/types_test.go index b27d83733..c59a862b1 100644 --- a/errs/types_test.go +++ b/errs/types_test.go @@ -558,6 +558,71 @@ func TestTypedError_UnwrapSymmetry(t *testing.T) { }) } +// TestValidationError_WithParams covers the structured-validation extension: +// WithParams appends InvalidParam items, the scalar Param setter is unaffected, +// and the wire shape nests {name, reason} under "params" (omitted when empty). +func TestValidationError_WithParams(t *testing.T) { + t.Run("appends and exposes fields", func(t *testing.T) { + e := errs.NewValidationError(errs.SubtypeInvalidArgument, "duplicate rel_path"). + WithParams(errs.InvalidParam{Name: "a.md", Reason: "duplicate"}) + if len(e.Params) != 1 { + t.Fatalf("len(Params) = %d, want 1", len(e.Params)) + } + if e.Params[0].Name != "a.md" { + t.Errorf("Params[0].Name = %q, want %q", e.Params[0].Name, "a.md") + } + if e.Params[0].Reason != "duplicate" { + t.Errorf("Params[0].Reason = %q, want %q", e.Params[0].Reason, "duplicate") + } + }) + + t.Run("appends across multiple calls and returns receiver", func(t *testing.T) { + e := errs.NewValidationError(errs.SubtypeInvalidArgument, "x") + returned := e.WithParams(errs.InvalidParam{Name: "a.md", Reason: "dup"}) + if returned != e { + t.Errorf("WithParams returned different pointer; want same as receiver") + } + e.WithParams( + errs.InvalidParam{Name: "b.md", Reason: "dup"}, + errs.InvalidParam{Name: "c.md", Reason: "dup"}, + ) + if len(e.Params) != 3 { + t.Fatalf("len(Params) = %d after two calls, want 3", len(e.Params)) + } + }) + + t.Run("wire shape nests name and reason under params", func(t *testing.T) { + e := errs.NewValidationError(errs.SubtypeInvalidArgument, "duplicate rel_path"). + WithParam("--rel-path"). + WithParams(errs.InvalidParam{Name: "a.md", Reason: "duplicate"}) + b, err := json.Marshal(e) + if err != nil { + t.Fatalf("marshal failed: %v", err) + } + got := string(b) + for _, want := range []string{ + `"type":"validation"`, + `"param":"--rel-path"`, + `"params":[{"name":"a.md","reason":"duplicate"}]`, + } { + if !strings.Contains(got, want) { + t.Errorf("missing %q in %s", want, got) + } + } + }) + + t.Run("empty Params omitted from wire", func(t *testing.T) { + e := errs.NewValidationError(errs.SubtypeInvalidArgument, "x") + b, err := json.Marshal(e) + if err != nil { + t.Fatalf("marshal failed: %v", err) + } + if strings.Contains(string(b), `"params"`) { + t.Errorf("empty Params should be omitted from wire; got %s", b) + } + }) +} + func TestBuilderSetter_DefensiveCopy(t *testing.T) { t.Run("WithMissingScopes clones input", func(t *testing.T) { scopes := []string{"docx:document", "im:message:send"} diff --git a/internal/errclass/classify.go b/internal/errclass/classify.go index 177d6952d..a0176b3ec 100644 --- a/internal/errclass/classify.go +++ b/internal/errclass/classify.go @@ -129,6 +129,7 @@ func BuildAPIError(resp map[string]any, cc ClassifyContext) error { Action: action, } case errs.CategoryAPI: + base.Hint = APIHint(base.Subtype) // "" for subtypes without a context-free default return &errs.APIError{Problem: base} default: // Fail closed: an unrecognized Category routes to InternalError @@ -231,6 +232,22 @@ func ConfigHint(subtype errs.Subtype) string { return "" } +// APIHint returns the canonical per-subtype recovery hint for a typed APIError +// emitted via BuildAPIError, for API subtypes whose recovery is context-free. +// Context-specific guidance (e.g. a command's flags, an API's own quota) is +// layered on by the caller after BuildAPIError returns and overrides this. +func APIHint(subtype errs.Subtype) string { + switch subtype { + case errs.SubtypeConflict: + return "retry later and avoid concurrent duplicate requests on the same resource" + case errs.SubtypeCrossTenant: + return "operate on source and target within the same tenant and region/unit" + case errs.SubtypeCrossBrand: + return "operate on source and target within the same brand environment" + } + return "" +} + func buildPermissionError(p errs.Problem, resp map[string]any, cc ClassifyContext) *errs.PermissionError { missing := extractMissingScopes(resp) identity := cc.Identity diff --git a/internal/errclass/codemeta_drive.go b/internal/errclass/codemeta_drive.go new file mode 100644 index 000000000..fe231633b --- /dev/null +++ b/internal/errclass/codemeta_drive.go @@ -0,0 +1,17 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package errclass + +import "github.com/larksuite/cli/errs" + +// driveCodeMeta holds drive/docs-service Lark code → CodeMeta mappings. +// Only codes whose meaning is verifiable from repo evidence are registered; +// ambiguous codes fall back to CategoryAPI via BuildAPIError. +// BuildAPIError consumes this map via mergeCodeMeta + LookupCodeMeta. +var driveCodeMeta = map[int]CodeMeta{ + 1061044: {Category: errs.CategoryAPI, Subtype: errs.SubtypeNotFound}, // parent folder does not exist (upload) + 1069302: {Category: errs.CategoryAPI, Subtype: errs.SubtypeInvalidParameters}, // comment endpoint "Invalid or missing parameters" +} + +func init() { mergeCodeMeta(driveCodeMeta, "drive") } diff --git a/internal/errclass/codemeta_drive_test.go b/internal/errclass/codemeta_drive_test.go new file mode 100644 index 000000000..a65a94f2c --- /dev/null +++ b/internal/errclass/codemeta_drive_test.go @@ -0,0 +1,43 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package errclass + +import ( + "fmt" + "testing" + + "github.com/larksuite/cli/errs" +) + +// TestLookupCodeMeta_DriveCodes pins each drive-service code registered via the +// codemeta_drive.go init() merge to its expected Category/Subtype/Retryable. +// Each case traces to repo evidence (see codemeta_drive.go comments). +func TestLookupCodeMeta_DriveCodes(t *testing.T) { + cases := []struct { + code int + wantCat errs.Category + wantSubtype errs.Subtype + wantRetry bool + }{ + // 1061044: upload with a nonexistent parent folder token. The drive E2E + // (tests_e2e/drive/2026_06_01_errs_migrate_drive_test.go) drives this + // producer via a nonexistent parent folder → referenced resource missing. + {1061044, errs.CategoryAPI, errs.SubtypeNotFound, false}, + // 1069302: comment endpoint's opaque "Invalid or missing parameters" + // (shortcuts/drive/drive_add_comment.go) → API-side parameter rejection. + {1069302, errs.CategoryAPI, errs.SubtypeInvalidParameters, false}, + } + for _, tc := range cases { + t.Run(fmt.Sprintf("%d", tc.code), func(t *testing.T) { + meta, ok := LookupCodeMeta(tc.code) + if !ok { + t.Fatalf("code %d not registered in codeMeta", tc.code) + } + if meta.Category != tc.wantCat || meta.Subtype != tc.wantSubtype || meta.Retryable != tc.wantRetry { + t.Errorf("code %d: got %+v, want Category=%v Subtype=%v Retryable=%v", + tc.code, meta, tc.wantCat, tc.wantSubtype, tc.wantRetry) + } + }) + } +} diff --git a/internal/output/errors.go b/internal/output/errors.go index 0ed2c44a0..69fcd8339 100644 --- a/internal/output/errors.go +++ b/internal/output/errors.go @@ -170,6 +170,28 @@ func ErrBare(code int) *ExitError { return &ExitError{Code: code} } +// PartialFailureError is the exit signal for a batch / multi-status command that +// has already written an ok:false result envelope to stdout. The per-item +// outcomes are the primary, machine-readable output and live on stdout, so the +// dispatcher sets only the exit code and writes nothing to stderr. +// +// It is deliberately distinct from ErrBare (the predicate silent-exit signal) +// so the predicate contract stays narrow, and from a typed *errs.XxxError +// (which owns the stderr error envelope): a partial failure is a result, not an +// error envelope. +type PartialFailureError struct { + Code int +} + +func (e *PartialFailureError) Error() string { + return fmt.Sprintf("partial failure (exit %d)", e.Code) +} + +// PartialFailure builds the partial-failure exit signal with the given code. +func PartialFailure(code int) *PartialFailureError { + return &PartialFailureError{Code: code} +} + // WriteTypedErrorEnvelope writes the JSON error envelope for a typed error. // Each typed error owns its wire shape via its own struct tags: Problem fields // are promoted to the top level through embedding, and extension fields diff --git a/internal/output/exitcode.go b/internal/output/exitcode.go index 6b8c2310b..953a31042 100644 --- a/internal/output/exitcode.go +++ b/internal/output/exitcode.go @@ -61,6 +61,10 @@ func ExitCodeOf(err error) int { if _, ok := errs.ProblemOf(err); ok { return ExitCodeForCategory(errs.CategoryOf(err)) } + var pfErr *PartialFailureError + if errors.As(err, &pfErr) { + return pfErr.Code + } var exitErr *ExitError if errors.As(err, &exitErr) { return exitErr.Code diff --git a/lint/errscontract/rule_no_legacy_envelope_literal.go b/lint/errscontract/rule_no_legacy_envelope_literal.go new file mode 100644 index 000000000..b995b27f2 --- /dev/null +++ b/lint/errscontract/rule_no_legacy_envelope_literal.go @@ -0,0 +1,146 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package errscontract + +import ( + "go/ast" + "go/parser" + "go/token" + "strings" +) + +// migratedEnvelopePaths lists the source-tree prefixes that have been migrated +// to the typed errs.* taxonomy. On these paths, constructing a legacy +// output.ExitError / output.ErrDetail envelope literal directly is forbidden — +// call sites must return a typed errs.* error instead. Future domains opt in by +// appending their path prefix here. +var migratedEnvelopePaths = []string{ + "shortcuts/drive/", +} + +// legacyOutputImportPath is the import path of the package that declares the +// legacy ExitError / ErrDetail envelope types. The rule resolves whatever local +// name (default or alias) this path is bound to in each file, so an aliased +// import cannot bypass the check. +const legacyOutputImportPath = "github.com/larksuite/cli/internal/output" + +// CheckNoLegacyEnvelopeLiteral flags direct construction of legacy +// output.ExitError / output.ErrDetail composite literals on migrated paths. +// forbidigo can ban identifiers but not composite literals, so this AST rule +// covers the gap left after a path is migrated to typed errs.* errors. +// +// Path-scoped to migratedEnvelopePaths (mirrors how CheckProblemEmbed restricts +// by path); skips _test.go fixtures. output.ErrBare(...) is a CallExpr, not a +// CompositeLit, so the predicate exit-signal helper is naturally not flagged. +func CheckNoLegacyEnvelopeLiteral(path, src string) []Violation { + if !isMigratedEnvelopePath(path) || strings.HasSuffix(path, "_test.go") { + return nil + } + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, path, src, parser.ParseComments) + if err != nil { + return nil + } + // Resolve the local name(s) bound to the legacy output import path. A file + // may bind it as the default `output`, an alias (`legacy "...output"`), or a + // dot-import (qualifier becomes ""), in which case ExitError/ErrDetail appear + // as bare unqualified idents. + localNames, dotImported := resolveLegacyOutputNames(file) + var out []Violation + ast.Inspect(file, func(n ast.Node) bool { + lit, ok := n.(*ast.CompositeLit) + if !ok { + return true + } + if name, ok := legacyEnvelopeTypeName(lit.Type, localNames, dotImported); ok { + out = append(out, Violation{ + Rule: "no_legacy_envelope_literal", + Action: ActionReject, + File: path, + Line: fset.Position(lit.Pos()).Line, + Message: "direct construction of legacy output." + name + " is forbidden on migrated paths; return a typed errs.* error (output.ErrBare remains allowed for predicate exit signals)", + Suggestion: "replace the &output." + name + "{...} literal with a typed errs.* constructor " + + "(e.g. errs.NewValidationError / errs.NewAPIError / errs.NewNetworkError)", + }) + } + return true + }) + return out +} + +// isMigratedEnvelopePath reports whether path falls under any migrated path +// prefix in migratedEnvelopePaths. +func isMigratedEnvelopePath(path string) bool { + p := strings.ReplaceAll(path, "\\", "/") + for _, prefix := range migratedEnvelopePaths { + if strings.HasPrefix(p, prefix) || strings.Contains(p, "/"+prefix) { + return true + } + } + return false +} + +// resolveLegacyOutputNames walks the file's import declarations and returns the +// set of local names bound to legacyOutputImportPath, plus whether the path was +// dot-imported. Default imports bind the package's own name ("output"); aliased +// imports bind the alias; dot-imports bind names into the file scope. +func resolveLegacyOutputNames(file *ast.File) (map[string]struct{}, bool) { + names := make(map[string]struct{}) + dotImported := false + for _, imp := range file.Imports { + if imp.Path == nil { + continue + } + p := strings.Trim(imp.Path.Value, "`\"") + if p != legacyOutputImportPath { + continue + } + switch { + case imp.Name == nil: + // Default import: local name is the package name "output". + names["output"] = struct{}{} + case imp.Name.Name == ".": + dotImported = true + case imp.Name.Name == "_": + // Blank import cannot reference the types; ignore. + default: + names[imp.Name.Name] = struct{}{} + } + } + return names, dotImported +} + +// legacyEnvelopeTypeName reports whether a composite-literal Type names the +// legacy ExitError / ErrDetail envelope and returns the bare type name. It +// matches a qualified selector (pkg.ExitError) when pkg is one of the resolved +// local names for the legacy output import, and — when the package was +// dot-imported — also matches a bare unqualified ExitError / ErrDetail ident. +func legacyEnvelopeTypeName(expr ast.Expr, localNames map[string]struct{}, dotImported bool) (string, bool) { + if sel, ok := expr.(*ast.SelectorExpr); ok { + x, ok := sel.X.(*ast.Ident) + if !ok || sel.Sel == nil { + return "", false + } + if _, bound := localNames[x.Name]; !bound { + return "", false + } + return matchLegacyEnvelopeName(sel.Sel.Name) + } + if dotImported { + if ident, ok := expr.(*ast.Ident); ok { + return matchLegacyEnvelopeName(ident.Name) + } + } + return "", false +} + +// matchLegacyEnvelopeName returns the name when it is one of the legacy +// envelope type names. +func matchLegacyEnvelopeName(name string) (string, bool) { + switch name { + case "ExitError", "ErrDetail": + return name, true + } + return "", false +} diff --git a/lint/errscontract/rule_no_legacy_runtime_api_call.go b/lint/errscontract/rule_no_legacy_runtime_api_call.go new file mode 100644 index 000000000..517087142 --- /dev/null +++ b/lint/errscontract/rule_no_legacy_runtime_api_call.go @@ -0,0 +1,73 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package errscontract + +import ( + "go/ast" + "go/parser" + "go/token" + "strings" +) + +// CheckNoLegacyRuntimeAPICall flags calls to the runtime's legacy +// auto-classifying API helpers (CallAPI / DoAPIJSON / DoAPIJSONWithLogID) on +// migrated paths. Those helpers route failures through common.HandleApiResult / +// doAPIJSON, which emit a legacy output.ExitError "api_error" envelope and +// downgrade an already-typed network / auth boundary error into an API error. +// forbidigo's errs-typed-only ban does not see them because they are method +// calls, not output.Err* identifiers — this AST rule covers that gap. +// +// Migrated code must call a typed API wrapper (e.g. drive's driveCallAPI) or use +// runtime.DoAPI + errclass.BuildAPIError directly, so failures classify into +// typed errs.* errors. +// +// Path-scoped to migratedEnvelopePaths; skips _test.go fixtures. A typed wrapper +// like driveCallAPI is an unqualified call (*ast.Ident), not a selector, so it +// is not matched. runtime.DoAPI / runtime.RawAPI are intentionally not listed: +// they return the raw response for the caller to classify and do not emit a +// legacy envelope themselves. +func CheckNoLegacyRuntimeAPICall(path, src string) []Violation { + if !isMigratedEnvelopePath(path) || strings.HasSuffix(path, "_test.go") { + return nil + } + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, path, src, parser.ParseComments) + if err != nil { + return nil + } + var out []Violation + ast.Inspect(file, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok || sel.Sel == nil { + return true + } + if name, ok := matchLegacyRuntimeAPIMethod(sel.Sel.Name); ok { + out = append(out, Violation{ + Rule: "no_legacy_runtime_api_call", + Action: ActionReject, + File: path, + Line: fset.Position(call.Pos()).Line, + Message: "runtime." + name + " emits a legacy output.ExitError api_error envelope and downgrades typed network/auth boundary errors; it is forbidden on migrated paths", + Suggestion: "call the domain's typed API wrapper (e.g. driveCallAPI) or runtime.DoAPI + errclass.BuildAPIError " + + "so failures classify into typed errs.* errors", + }) + } + return true + }) + return out +} + +// matchLegacyRuntimeAPIMethod returns the name when it is one of the runtime's +// legacy auto-classifying API helper methods. +func matchLegacyRuntimeAPIMethod(name string) (string, bool) { + switch name { + case "CallAPI", "DoAPIJSON", "DoAPIJSONWithLogID": + return name, true + } + return "", false +} diff --git a/lint/errscontract/rules_test.go b/lint/errscontract/rules_test.go index ddb77ed28..9beed919d 100644 --- a/lint/errscontract/rules_test.go +++ b/lint/errscontract/rules_test.go @@ -593,3 +593,287 @@ func FooRegisterServiceMapBar(name string, _ interface{}) {} t.Errorf("message must name the offending call: %s", v[0].Message) } } + +// (F) direct legacy output.ExitError / output.ErrDetail literals on migrated +// paths → REJECT; output.ErrBare(...) calls and non-migrated paths pass. + +func TestCheckNoLegacyEnvelopeLiteral_RejectsExitErrorLiteralOnDrivePath(t *testing.T) { + src := `package drive + +import "github.com/larksuite/cli/internal/output" + +func boom() error { + return &output.ExitError{Code: 1} +} +` + v := CheckNoLegacyEnvelopeLiteral("shortcuts/drive/drive_export.go", src) + if len(v) != 1 { + t.Fatalf("expected 1 violation, got %d: %+v", len(v), v) + } + if v[0].Action != ActionReject { + t.Errorf("action = %q, want REJECT", v[0].Action) + } + if !strings.Contains(v[0].Message, "ExitError") { + t.Errorf("message should name the legacy type: %s", v[0].Message) + } +} + +func TestCheckNoLegacyEnvelopeLiteral_RejectsErrDetailLiteralOnDrivePath(t *testing.T) { + src := `package drive + +import "github.com/larksuite/cli/internal/output" + +func boom() *output.ErrDetail { + return &output.ErrDetail{Code: 7} +} +` + v := CheckNoLegacyEnvelopeLiteral("shortcuts/drive/drive_export_common.go", src) + if len(v) != 1 { + t.Fatalf("expected 1 violation, got %d: %+v", len(v), v) + } + if !strings.Contains(v[0].Message, "ErrDetail") { + t.Errorf("message should name the legacy type: %s", v[0].Message) + } +} + +func TestCheckNoLegacyEnvelopeLiteral_AllowsErrBareCallOnDrivePath(t *testing.T) { + // output.ErrBare(...) is a CallExpr, not a CompositeLit — must NOT fire. + src := `package drive + +import "github.com/larksuite/cli/internal/output" + +func boom() error { + return output.ErrBare(output.ExitAPI) +} +` + v := CheckNoLegacyEnvelopeLiteral("shortcuts/drive/drive_export.go", src) + if len(v) != 0 { + t.Errorf("ErrBare call should pass, got: %+v", v) + } +} + +func TestCheckNoLegacyEnvelopeLiteral_IgnoresNonMigratedPath(t *testing.T) { + // Same offending literal, but outside the migrated path set → not flagged. + src := `package other + +import "github.com/larksuite/cli/internal/output" + +func boom() error { + return &output.ExitError{Code: 1} +} +` + v := CheckNoLegacyEnvelopeLiteral("shortcuts/calendar/foo.go", src) + if len(v) != 0 { + t.Errorf("non-migrated path should pass, got: %+v", v) + } +} + +func TestCheckNoLegacyEnvelopeLiteral_SkipsTestFiles(t *testing.T) { + src := `package drive + +import "github.com/larksuite/cli/internal/output" + +func boom() error { + return &output.ExitError{Code: 1} +} +` + v := CheckNoLegacyEnvelopeLiteral("shortcuts/drive/drive_export_test.go", src) + if len(v) != 0 { + t.Errorf("_test.go file should be skipped, got: %+v", v) + } +} + +// TestCheckNoLegacyEnvelopeLiteral_RejectsAliasedImport pins that an aliased +// import of internal/output cannot bypass the rule: the qualifier is resolved +// from the import declaration, not matched against the literal string "output". +func TestCheckNoLegacyEnvelopeLiteral_RejectsAliasedImport(t *testing.T) { + src := `package drive + +import legacy "github.com/larksuite/cli/internal/output" + +func boom() error { + return &legacy.ExitError{Code: 1} +} +` + v := CheckNoLegacyEnvelopeLiteral("shortcuts/drive/drive_export.go", src) + if len(v) != 1 { + t.Fatalf("expected 1 violation for aliased import, got %d: %+v", len(v), v) + } + if v[0].Action != ActionReject { + t.Errorf("action = %q, want REJECT", v[0].Action) + } + if !strings.Contains(v[0].Message, "ExitError") { + t.Errorf("message should name the legacy type: %s", v[0].Message) + } +} + +// TestCheckNoLegacyEnvelopeLiteral_NormalImportStillRejected guards against a +// regression where resolving by import path accidentally drops the default +// (non-aliased) `output` case. +func TestCheckNoLegacyEnvelopeLiteral_NormalImportStillRejected(t *testing.T) { + src := `package drive + +import "github.com/larksuite/cli/internal/output" + +func boom() error { + return &output.ExitError{Code: 1} +} +` + v := CheckNoLegacyEnvelopeLiteral("shortcuts/drive/drive_export.go", src) + if len(v) != 1 { + t.Fatalf("expected 1 violation for default import, got %d: %+v", len(v), v) + } +} + +// TestCheckNoLegacyEnvelopeLiteral_ErrBareAliasedStillAllowed: output.ErrBare is +// a CallExpr, not a composite literal — even under an alias it must not fire. +func TestCheckNoLegacyEnvelopeLiteral_ErrBareAliasedStillAllowed(t *testing.T) { + src := `package drive + +import legacy "github.com/larksuite/cli/internal/output" + +func boom() error { + return legacy.ErrBare(legacy.ExitAPI) +} +` + v := CheckNoLegacyEnvelopeLiteral("shortcuts/drive/drive_export.go", src) + if len(v) != 0 { + t.Errorf("ErrBare call should pass, got: %+v", v) + } +} + +// TestCheckNoLegacyEnvelopeLiteral_RejectsDotImport: a dot-import surfaces +// ExitError / ErrDetail as bare unqualified idents; the rule must still catch +// the composite literal. +func TestCheckNoLegacyEnvelopeLiteral_RejectsDotImport(t *testing.T) { + src := `package drive + +import . "github.com/larksuite/cli/internal/output" + +func boom() error { + return &ExitError{Code: 1} +} +` + v := CheckNoLegacyEnvelopeLiteral("shortcuts/drive/drive_export.go", src) + if len(v) != 1 { + t.Fatalf("expected 1 violation for dot-import, got %d: %+v", len(v), v) + } + if !strings.Contains(v[0].Message, "ExitError") { + t.Errorf("message should name the legacy type: %s", v[0].Message) + } +} + +// TestCheckNoLegacyEnvelopeLiteral_UnrelatedSelectorPasses: a same-named +// selector on an unrelated package (not the legacy output import path) must not +// trigger a false positive. +func TestCheckNoLegacyEnvelopeLiteral_UnrelatedSelectorPasses(t *testing.T) { + src := `package drive + +import "example.com/other/output" + +func boom() error { + return &output.ExitError{Code: 1} +} +` + v := CheckNoLegacyEnvelopeLiteral("shortcuts/drive/drive_export.go", src) + if len(v) != 0 { + t.Errorf("unrelated package selector must not fire, got: %+v", v) + } +} + +func TestCheckNoLegacyRuntimeAPICall_RejectsCallAPIOnDrivePath(t *testing.T) { + src := `package drive + +func boom(runtime *common.RuntimeContext) error { + _, err := runtime.CallAPI("POST", "/x", nil, nil) + return err +} +` + v := CheckNoLegacyRuntimeAPICall("shortcuts/drive/drive_create_folder.go", src) + if len(v) != 1 { + t.Fatalf("expected 1 violation, got %d: %+v", len(v), v) + } + if v[0].Action != ActionReject { + t.Errorf("action = %q, want REJECT", v[0].Action) + } + if !strings.Contains(v[0].Message, "CallAPI") { + t.Errorf("message should name the legacy method: %s", v[0].Message) + } +} + +func TestCheckNoLegacyRuntimeAPICall_RejectsDoAPIJSONWithLogIDOnDrivePath(t *testing.T) { + src := `package drive + +func boom(runtime *common.RuntimeContext) error { + _, err := runtime.DoAPIJSONWithLogID("POST", "/x", nil, nil) + return err +} +` + v := CheckNoLegacyRuntimeAPICall("shortcuts/drive/drive_export.go", src) + if len(v) != 1 { + t.Fatalf("expected 1 violation, got %d: %+v", len(v), v) + } + if !strings.Contains(v[0].Message, "DoAPIJSONWithLogID") { + t.Errorf("message should name the legacy method: %s", v[0].Message) + } +} + +func TestCheckNoLegacyRuntimeAPICall_AllowsTypedWrapperCall(t *testing.T) { + // driveCallAPI is an unqualified call (*ast.Ident), not a selector — must NOT fire. + src := `package drive + +func boom(runtime *common.RuntimeContext) error { + _, err := driveCallAPI(runtime, "POST", "/x", nil, nil) + return err +} +` + v := CheckNoLegacyRuntimeAPICall("shortcuts/drive/drive_create_folder.go", src) + if len(v) != 0 { + t.Errorf("typed wrapper call must not fire, got: %+v", v) + } +} + +func TestCheckNoLegacyRuntimeAPICall_AllowsRawAPIAndDoAPI(t *testing.T) { + // RawAPI / DoAPI return the raw response for the caller to classify and do + // not emit a legacy envelope — they are not banned. + src := `package drive + +func boom(runtime *common.RuntimeContext) error { + _, _ = runtime.RawAPI("POST", "/x", nil, nil) + _, err := runtime.DoAPI(nil) + return err +} +` + v := CheckNoLegacyRuntimeAPICall("shortcuts/drive/drive_api.go", src) + if len(v) != 0 { + t.Errorf("RawAPI / DoAPI must not fire, got: %+v", v) + } +} + +func TestCheckNoLegacyRuntimeAPICall_IgnoresNonMigratedPath(t *testing.T) { + src := `package im + +func boom(runtime *common.RuntimeContext) error { + _, err := runtime.CallAPI("POST", "/x", nil, nil) + return err +} +` + v := CheckNoLegacyRuntimeAPICall("shortcuts/im/im_send.go", src) + if len(v) != 0 { + t.Errorf("non-migrated path must not fire, got: %+v", v) + } +} + +func TestCheckNoLegacyRuntimeAPICall_SkipsTestFiles(t *testing.T) { + src := `package drive + +func boom(runtime *common.RuntimeContext) error { + _, err := runtime.CallAPI("POST", "/x", nil, nil) + return err +} +` + v := CheckNoLegacyRuntimeAPICall("shortcuts/drive/drive_create_folder_test.go", src) + if len(v) != 0 { + t.Errorf("test files must be skipped, got: %+v", v) + } +} diff --git a/lint/errscontract/scan.go b/lint/errscontract/scan.go index bb920d855..dd4c54821 100644 --- a/lint/errscontract/scan.go +++ b/lint/errscontract/scan.go @@ -106,6 +106,8 @@ func ScanRepo(root string) ([]Violation, error) { all = append(all, CheckNoRegistrar(rel, string(src))...) all = append(all, CheckAdHocSubtype(rel, string(src))...) all = append(all, CheckTypedErrorCompleteness(rel, string(src))...) + all = append(all, CheckNoLegacyEnvelopeLiteral(rel, string(src))...) + all = append(all, CheckNoLegacyRuntimeAPICall(rel, string(src))...) // Typed-error invariants — self-scope to errs/ + classify.go. all = append(all, CheckNilSafeError(rel, string(src))...) all = append(all, CheckUnwrapSymmetry(rel, string(src))...) diff --git a/shortcuts/common/call_api_typed_test.go b/shortcuts/common/call_api_typed_test.go new file mode 100644 index 000000000..40925e029 --- /dev/null +++ b/shortcuts/common/call_api_typed_test.go @@ -0,0 +1,200 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package common + +import ( + "context" + "errors" + "net/http" + "testing" + + "github.com/spf13/cobra" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/httpmock" +) + +func newCallAPITypedRuntime(t *testing.T) (*RuntimeContext, *httpmock.Registry) { + t.Helper() + cfg := &core.CliConfig{Brand: core.BrandFeishu, AppID: "cli_x"} + f, _, _, reg := cmdutil.TestFactory(t, cfg) + rt := TestNewRuntimeContextForAPI(context.Background(), &cobra.Command{Use: "+x"}, cfg, f, core.AsUser) + return rt, reg +} + +// TestCallAPITyped_HeaderOnlyLogID pins the P1 fix: when the server returns +// log_id only in the x-tt-logid response header (not in the JSON body), the +// typed error still carries it. The legacy runtime.CallAPI path (body-only) +// dropped it. +func TestCallAPITyped_HeaderOnlyLogID(t *testing.T) { + rt, reg := newCallAPITypedRuntime(t) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/x/y", + Headers: http.Header{ + "Content-Type": []string{"application/json"}, + "X-Tt-Logid": []string{"hdr-log-123"}, + }, + Body: map[string]interface{}{"code": float64(1061044), "msg": "boom"}, // no log_id in body + }) + + _, err := rt.CallAPITyped("POST", "/open-apis/x/y", nil, map[string]any{}) + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected a typed errs.* error, got %T: %v", err, err) + } + if p.LogID != "hdr-log-123" { + t.Errorf("LogID = %q, want %q (lifted from x-tt-logid header)", p.LogID, "hdr-log-123") + } +} + +// TestCallAPITyped_BodyLogID confirms body-level log_id still surfaces. +func TestCallAPITyped_BodyLogID(t *testing.T) { + rt, reg := newCallAPITypedRuntime(t) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/x/y", + Body: map[string]interface{}{"code": float64(1061044), "msg": "boom", "log_id": "body-log-9"}, + }) + + _, err := rt.CallAPITyped("POST", "/open-apis/x/y", nil, map[string]any{}) + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed error, got %T: %v", err, err) + } + if p.LogID != "body-log-9" { + t.Errorf("LogID = %q, want body-log-9", p.LogID) + } +} + +// TestCallAPITyped_Success returns the data object on code 0, and does not leak +// the header log_id into the success payload (log_id surfacing is error-path +// only — success output stays identical to the legacy CallAPI). +func TestCallAPITyped_Success(t *testing.T) { + rt, reg := newCallAPITypedRuntime(t) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/x/y", + Headers: http.Header{ + "Content-Type": []string{"application/json"}, + "X-Tt-Logid": []string{"hdr-log-ok"}, + }, + Body: map[string]interface{}{"code": float64(0), "data": map[string]interface{}{"token": "tok1"}}, + }) + + data, err := rt.CallAPITyped("POST", "/open-apis/x/y", nil, map[string]any{}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if data["token"] != "tok1" { + t.Errorf("data[token] = %v, want tok1", data["token"]) + } + if _, leaked := data["log_id"]; leaked { + t.Errorf("success data must not carry log_id, got: %v", data) + } +} + +// TestAPIClassifyContext verifies the classify context is built from the +// runtime: Brand / AppID from config, Identity from the resolved caller, and +// LarkCmd from the running command path. +func TestAPIClassifyContext(t *testing.T) { + cfg := &core.CliConfig{Brand: core.BrandLark, AppID: "cli_x"} + rt := TestNewRuntimeContextWithIdentity(&cobra.Command{Use: "+upload"}, cfg, core.AsUser) + + cc := rt.APIClassifyContext() + if cc.Brand != "lark" { + t.Errorf("Brand = %q, want lark", cc.Brand) + } + if cc.AppID != "cli_x" { + t.Errorf("AppID = %q, want cli_x", cc.AppID) + } + if cc.Identity != "user" { + t.Errorf("Identity = %q, want user", cc.Identity) + } + if cc.LarkCmd != "+upload" { + t.Errorf("LarkCmd = %q, want +upload", cc.LarkCmd) + } + + bot := TestNewRuntimeContextWithIdentity(&cobra.Command{Use: "+push"}, &core.CliConfig{Brand: core.BrandFeishu, AppID: "y"}, core.AsBot) + if got := bot.APIClassifyContext().Identity; got != "bot" { + t.Errorf("bot Identity = %q, want bot", got) + } +} + +// TestCallAPITyped_NonJSON5xx pins that a non-JSON HTTP 5xx (e.g. a gateway 502 +// text/html page) is a retryable network/server_error carrying the header +// log_id — not a mis-parsed internal/invalid_response. +func TestCallAPITyped_NonJSON5xx(t *testing.T) { + rt, reg := newCallAPITypedRuntime(t) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/x/y", + Status: 502, + Headers: http.Header{ + "Content-Type": []string{"text/html"}, + "X-Tt-Logid": []string{"hdr-502"}, + }, + RawBody: []byte("502 Bad Gateway"), + }) + + _, err := rt.CallAPITyped("POST", "/open-apis/x/y", nil, map[string]any{}) + var netErr *errs.NetworkError + if !errors.As(err, &netErr) { + t.Fatalf("expected *errs.NetworkError for non-JSON 5xx, got %T: %v", err, err) + } + if netErr.Subtype != errs.SubtypeNetworkServer { + t.Errorf("subtype = %q, want %q", netErr.Subtype, errs.SubtypeNetworkServer) + } + if !netErr.Retryable { + t.Error("5xx network error must be retryable") + } + if netErr.LogID != "hdr-502" { + t.Errorf("LogID = %q, want hdr-502 (from header)", netErr.LogID) + } +} + +// TestCallAPITyped_5xxNoContentType pins that a 5xx with no Content-Type (which +// the body-only parse would mis-classify as invalid_response) is still a +// retryable network/server_error. +func TestCallAPITyped_5xxNoContentType(t *testing.T) { + rt, reg := newCallAPITypedRuntime(t) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/x/y", + Status: 503, + Headers: http.Header{}, // explicitly no Content-Type header + RawBody: []byte("service unavailable"), + }) + + _, err := rt.CallAPITyped("POST", "/open-apis/x/y", nil, map[string]any{}) + var netErr *errs.NetworkError + if !errors.As(err, &netErr) || netErr.Subtype != errs.SubtypeNetworkServer { + t.Fatalf("expected retryable network/server_error, got %T: %v", err, err) + } + if !netErr.Retryable { + t.Error("5xx network error must be retryable") + } +} + +// TestCallAPITyped_NonObjectJSON pins that a top-level non-object JSON body +// (e.g. "[]") is rejected as an invalid response, never a silent success ack. +func TestCallAPITyped_NonObjectJSON(t *testing.T) { + rt, reg := newCallAPITypedRuntime(t) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/x/y", + RawBody: []byte("[]"), + }) + + _, err := rt.CallAPITyped("POST", "/open-apis/x/y", nil, map[string]any{}) + var intErr *errs.InternalError + if !errors.As(err, &intErr) { + t.Fatalf("expected *errs.InternalError for non-object JSON, got %T: %v", err, err) + } + if intErr.Subtype != errs.SubtypeInvalidResponse { + t.Errorf("subtype = %q, want %q", intErr.Subtype, errs.SubtypeInvalidResponse) + } +} diff --git a/shortcuts/common/runner.go b/shortcuts/common/runner.go index 2a795d550..7b023e6e9 100644 --- a/shortcuts/common/runner.go +++ b/shortcuts/common/runner.go @@ -26,6 +26,7 @@ import ( "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/credential" + "github.com/larksuite/cli/internal/errclass" "github.com/larksuite/cli/internal/i18n" "github.com/larksuite/cli/internal/output" "github.com/spf13/cobra" @@ -233,6 +234,133 @@ func (ctx *RuntimeContext) CallAPI(method, url string, params map[string]interfa return HandleApiResult(result, err, "API call failed") } +// CallAPITyped is the typed-only replacement for CallAPI: it performs the same +// SDK request (buildRequest → APIClient.DoAPI → DoSDKRequest, identical +// transport and query model to CallAPI) and returns the "data" object, but +// classifies failures into typed errs.* errors via errclass.BuildAPIError. +// +// A transport / auth error from the client boundary is already typed and passes +// through unchanged; a non-zero API response code is classified into a typed +// error carrying subtype / code / log_id. Unlike CallAPI it never emits a legacy +// output.ExitError envelope, and never downgrades a typed network/auth error. +// +// It lifts x-tt-logid from the response header (which the body-only parse drops) +// so log_id surfaces on the typed error even when the server returns it only in +// the header. +func (ctx *RuntimeContext) CallAPITyped(method, url string, params map[string]interface{}, data interface{}) (map[string]interface{}, error) { + ac, err := ctx.getAPIClient() + if err != nil { + return nil, typedOrInternal(err) + } + resp, err := ac.DoAPI(ctx.ctx, ctx.buildRequest(method, url, params, data)) + if err != nil { + return nil, typedOrInternal(err) + } + return ctx.ClassifyAPIResponse(resp) +} + +// ClassifyAPIResponse turns a raw *larkcore.ApiResp into the "data" object or a +// typed errs.* error. It is the shared response classifier for typed API paths +// — used by CallAPITyped and by callers that drive the request themselves +// (e.g. file upload via DoAPI). It: +// +// 1. parses the JSON body; an unparseable body on an HTTP error status (a +// gateway 5xx text/html page, an empty body, a missing Content-Type) is +// classified by status — 5xx → retryable network/server_error, 404 → +// not_found, other 4xx → api error — not a misleading invalid-response +// internal error; +// 2. rejects a top-level non-object JSON ([], null, scalar) as an +// invalid-response internal error — never a silent success ack; +// 3. lifts x-tt-logid from the response header onto the typed error so log_id +// surfaces even when the body omits it; +// 4. classifies a non-zero API code via errclass.BuildAPIError, and treats any +// HTTP error status that parsed to code==0 as a status error. +// +// The success "data" object is returned untouched. On a non-zero API code the +// data is returned alongside the typed error, since the response can still +// carry fields a caller needs on failure (e.g. the file_token an overwrite +// returned, for token-stability handling). +func (ctx *RuntimeContext) ClassifyAPIResponse(resp *larkcore.ApiResp) (map[string]interface{}, error) { + logID, _ := logIDFromHeader(resp)["log_id"].(string) + + result, parseErr := client.ParseJSONResponse(resp) + if parseErr != nil { + if resp.StatusCode >= 400 { + return nil, httpStatusError(resp.StatusCode, resp.RawBody, logID) + } + return nil, client.WrapJSONResponseParseError(parseErr, resp.RawBody) + } + resultMap, ok := result.(map[string]interface{}) + if !ok { + e := errs.NewInternalError(errs.SubtypeInvalidResponse, "API returned a non-object JSON response") + if logID != "" { + e = e.WithLogID(logID) + } + return nil, e + } + if logID != "" { + if _, present := resultMap["log_id"]; !present { + resultMap["log_id"] = logID + } + } + out, _ := resultMap["data"].(map[string]interface{}) + if apiErr := errclass.BuildAPIError(resultMap, ctx.APIClassifyContext()); apiErr != nil { + return out, apiErr + } + if resp.StatusCode >= 400 { + return out, httpStatusError(resp.StatusCode, resp.RawBody, logID) + } + return out, nil +} + +// httpStatusError classifies an HTTP error status whose body is not a usable +// API envelope: 5xx → retryable network/server_error, 404 → not_found, other +// 4xx → api error. The x-tt-logid (when present) is attached for diagnosis. +func httpStatusError(status int, rawBody []byte, logID string) error { + body := TruncateStr(strings.TrimSpace(string(rawBody)), 500) + if status >= 500 { + e := errs.NewNetworkError(errs.SubtypeNetworkServer, "HTTP %d: %s", status, body).WithCode(status).WithRetryable() + if logID != "" { + e = e.WithLogID(logID) + } + return e + } + subtype := errs.SubtypeUnknown + if status == http.StatusNotFound { + subtype = errs.SubtypeNotFound + } + e := errs.NewAPIError(subtype, "HTTP %d: %s", status, body).WithCode(status) + if logID != "" { + e = e.WithLogID(logID) + } + return e +} + +// typedOrInternal passes an already-typed errs.* error through unchanged and +// lifts a still-untyped one to a typed internal error, so CallAPITyped never +// returns a bare/legacy error. +func typedOrInternal(err error) error { + if _, ok := errs.ProblemOf(err); ok { + return err + } + return errs.WrapInternal(err) +} + +// APIClassifyContext builds the errclass.ClassifyContext for the running command +// from the runtime config and resolved identity. +func (ctx *RuntimeContext) APIClassifyContext() errclass.ClassifyContext { + larkCmd := "" + if ctx.Cmd != nil { + larkCmd = strings.TrimPrefix(ctx.Cmd.CommandPath(), "lark ") + } + return errclass.ClassifyContext{ + Brand: string(ctx.Config.Brand), + AppID: ctx.Config.AppID, + Identity: string(ctx.As()), + LarkCmd: larkCmd, + } +} + // Deprecated: RawAPI uses an internal HTTP wrapper with limited control over request/response. // Prefer DoAPI for new code — it calls the Lark SDK directly and supports file upload/download options. // @@ -552,28 +680,47 @@ func (ctx *RuntimeContext) ValidatePath(path string) error { // Out prints a success JSON envelope to stdout. func (ctx *RuntimeContext) Out(data interface{}, meta *output.Meta) { - ctx.emit(data, meta, false) + ctx.emit(data, meta, false, true) } // OutRaw prints a success JSON envelope to stdout with HTML escaping disabled. // Use this instead of Out when the data contains XML/HTML content (e.g. document bodies) // that should be preserved as-is in JSON output. func (ctx *RuntimeContext) OutRaw(data interface{}, meta *output.Meta) { - ctx.emit(data, meta, true) + ctx.emit(data, meta, true, true) +} + +// OutPartialFailure writes an ok:false multi-status result envelope to stdout +// and returns the partial-failure exit signal. Use it for batch operations +// where some items failed but the per-item outcomes are the primary output: +// the full result (summary + per-item statuses) stays machine-readable on +// stdout, the process exits non-zero, and nothing is written to stderr. +// +// It is the typed alternative to `Out(...)` + `output.ErrBare(...)` — the +// envelope's ok field honestly reports failure instead of a misleading +// ok:true, and the exit signal is distinct from the predicate-only ErrBare. +func (ctx *RuntimeContext) OutPartialFailure(data interface{}, meta *output.Meta) error { + ctx.emit(data, meta, false, false) + if ctx.outputErr != nil { + return ctx.outputErr + } + return output.PartialFailure(output.ExitAPI) } -// emit is the shared success-path emitter. raw=true disables JSON HTML escaping so -// XML/HTML payloads (e.g. DocxXML bodies) are preserved verbatim; otherwise behavior +// emit is the shared stdout envelope emitter; ok sets the envelope's ok field +// (true for success, false for a partial-failure result). raw=true disables JSON +// HTML escaping so XML/HTML payloads (e.g. DocxXML bodies) are preserved +// verbatim; otherwise behavior // is identical — content-safety scanning and race-safe first-error capture via // outputErrOnce apply in both modes. -func (ctx *RuntimeContext) emit(data interface{}, meta *output.Meta, raw bool) { +func (ctx *RuntimeContext) emit(data interface{}, meta *output.Meta, raw, ok bool) { scanResult := output.ScanForSafety(ctx.Cmd.CommandPath(), data, ctx.IO().ErrOut) if scanResult.Blocked { ctx.outputErrOnce.Do(func() { ctx.outputErr = scanResult.BlockErr }) return } - env := output.Envelope{OK: true, Identity: string(ctx.As()), Data: data, Meta: meta, Notice: output.GetNotice()} + env := output.Envelope{OK: ok, Identity: string(ctx.As()), Data: data, Meta: meta, Notice: output.GetNotice()} if scanResult.Alert != nil { env.ContentSafetyAlert = scanResult.Alert } diff --git a/shortcuts/common/runner_partial_failure_test.go b/shortcuts/common/runner_partial_failure_test.go new file mode 100644 index 000000000..1be71c255 --- /dev/null +++ b/shortcuts/common/runner_partial_failure_test.go @@ -0,0 +1,63 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package common + +import ( + "context" + "encoding/json" + "errors" + "testing" + + "github.com/spf13/cobra" + + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/output" +) + +// TestOutPartialFailure pins the batch / multi-status contract: the result +// rides on stdout as an ok:false envelope (carrying the full payload), and the +// returned error is the typed partial-failure exit signal (ExitAPI), distinct +// from the predicate-only ErrBare. +func TestOutPartialFailure(t *testing.T) { + cfg := &core.CliConfig{Brand: core.BrandFeishu, AppID: "cli_x"} + f, stdout, _, _ := cmdutil.TestFactory(t, cfg) + rt := TestNewRuntimeContextForAPI(context.Background(), &cobra.Command{Use: "+push"}, cfg, f, core.AsUser) + + payload := map[string]interface{}{ + "summary": map[string]interface{}{"uploaded": 1, "failed": 1}, + "items": []map[string]interface{}{ + {"rel_path": "a.txt", "action": "uploaded"}, + {"rel_path": "b.txt", "action": "failed", "error": "boom"}, + }, + } + + err := rt.OutPartialFailure(payload, nil) + + // 1) typed partial-failure exit signal + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("expected *output.PartialFailureError, got %T: %v", err, err) + } + if pfErr.Code != output.ExitAPI { + t.Errorf("exit code = %d, want %d (ExitAPI)", pfErr.Code, output.ExitAPI) + } + + // 2) stdout envelope reports ok:false but still carries the full payload + // (both the succeeded and failed items) — consistent with a success Out(). + var env struct { + OK bool `json:"ok"` + Data map[string]interface{} `json:"data"` + } + if err := json.Unmarshal(stdout.Bytes(), &env); err != nil { + t.Fatalf("unmarshal stdout envelope: %v\nstdout: %s", err, stdout.String()) + } + if env.OK { + t.Errorf("ok must be false on partial failure, got ok:true\nstdout: %s", stdout.String()) + } + items, _ := env.Data["items"].([]interface{}) + if len(items) != 2 { + t.Fatalf("both succeeded and failed items must ride on stdout, got %d items\nstdout: %s", len(items), stdout.String()) + } +} diff --git a/shortcuts/drive/drive_add_comment.go b/shortcuts/drive/drive_add_comment.go index b40ca959d..337ed27b3 100644 --- a/shortcuts/drive/drive_add_comment.go +++ b/shortcuts/drive/drive_add_comment.go @@ -11,7 +11,7 @@ import ( "strings" "unicode/utf8" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -152,13 +152,13 @@ var DriveAddComment = common.Shortcut{ if docRef.Kind == "sheet" { blockID := strings.TrimSpace(runtime.Str("block-id")) if blockID == "" { - return output.ErrValidation("--block-id is required for sheet comments (format: !, e.g. a281f9!D6)") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--block-id is required for sheet comments (format: !, e.g. a281f9!D6)").WithParam("--block-id") } if _, err := parseSheetCellRef(blockID); err != nil { return err } if runtime.Bool("full-comment") || strings.TrimSpace(runtime.Str("selection-with-ellipsis")) != "" { - return output.ErrValidation("--full-comment and --selection-with-ellipsis are not applicable for sheet comments; use --block-id with ! format") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--full-comment and --selection-with-ellipsis are not applicable for sheet comments; use --block-id with ! format") } return nil } @@ -167,20 +167,20 @@ var DriveAddComment = common.Shortcut{ return err } if runtime.Bool("full-comment") { - return output.ErrValidation("--full-comment is not applicable for slide comments; use --block-id !") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--full-comment is not applicable for slide comments; use --block-id !") } if strings.TrimSpace(runtime.Str("selection-with-ellipsis")) != "" { - return output.ErrValidation("--selection-with-ellipsis is not applicable for slide comments; use --block-id !") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--selection-with-ellipsis is not applicable for slide comments; use --block-id !") } return nil } selection := runtime.Str("selection-with-ellipsis") blockID := strings.TrimSpace(runtime.Str("block-id")) if strings.TrimSpace(selection) != "" && blockID != "" { - return output.ErrValidation("--selection-with-ellipsis and --block-id are mutually exclusive") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--selection-with-ellipsis and --block-id are mutually exclusive") } if runtime.Bool("full-comment") && (strings.TrimSpace(selection) != "" || blockID != "") { - return output.ErrValidation("--full-comment cannot be used with --selection-with-ellipsis or --block-id") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--full-comment cannot be used with --selection-with-ellipsis or --block-id") } mode := resolveCommentMode(runtime.Bool("full-comment"), selection, blockID) @@ -188,7 +188,7 @@ var DriveAddComment = common.Shortcut{ return validateFileCommentMode(mode, "") } if mode == commentModeLocal && docRef.Kind == "doc" { - return output.ErrValidation("local comments only support docx, sheet, and slides; old doc format only supports full comments") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "local comments only support docx, sheet, and slides; old doc format only supports full comments") } return nil @@ -398,7 +398,7 @@ var DriveAddComment = common.Shortcut{ } blockID = match.AnchorBlockID if strings.TrimSpace(blockID) == "" { - return output.Errorf(output.ExitAPI, "api_error", "locate-doc response missing anchor_block_id") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "locate-doc response missing anchor_block_id") } selectedMatch = idx fmt.Fprintf(runtime.IO().ErrOut, "Locate-doc matched %d block(s); using match #%d (%s)\n", len(locateResult.Matches), idx, blockID) @@ -418,7 +418,7 @@ var DriveAddComment = common.Shortcut{ fmt.Fprintf(runtime.IO().ErrOut, "Creating full comment in %s\n", common.MaskToken(target.FileToken)) } - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "POST", requestPath, nil, @@ -473,7 +473,7 @@ func resolveCommentMode(explicitFullComment bool, selection, blockID string) com func parseCommentDocRef(input, docType string) (commentDocRef, error) { raw := strings.TrimSpace(input) if raw == "" { - return commentDocRef{}, output.ErrValidation("--doc cannot be empty") + return commentDocRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "--doc cannot be empty").WithParam("--doc") } if token, ok := extractURLToken(raw, "/wiki/"); ok { @@ -495,16 +495,16 @@ func parseCommentDocRef(input, docType string) (commentDocRef, error) { return commentDocRef{Kind: "doc", Token: token}, nil } if strings.Contains(raw, "://") { - return commentDocRef{}, output.ErrValidation("unsupported --doc input %q: use a doc/docx/file/sheet/slides URL, a token with --type, or a wiki URL that resolves to doc/docx/file/sheet/slides", raw) + return commentDocRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported --doc input %q: use a doc/docx/file/sheet/slides URL, a token with --type, or a wiki URL that resolves to doc/docx/file/sheet/slides", raw).WithParam("--doc") } if strings.ContainsAny(raw, "/?#") { - return commentDocRef{}, output.ErrValidation("unsupported --doc input %q: use a token with --type, or a wiki URL", raw) + return commentDocRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported --doc input %q: use a token with --type, or a wiki URL", raw).WithParam("--doc") } // Bare token: --type is required. docType = strings.TrimSpace(docType) if docType == "" { - return commentDocRef{}, output.ErrValidation("--type is required when --doc is a bare token (allowed values: doc, docx, file, sheet, slides)") + return commentDocRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "--type is required when --doc is a bare token (allowed values: doc, docx, file, sheet, slides)").WithParam("--type") } return commentDocRef{Kind: docType, Token: raw}, nil } @@ -519,7 +519,7 @@ func resolveCommentTarget(ctx context.Context, runtime *common.RuntimeContext, i if mode == commentModeLocal { switch docRef.Kind { case "doc": - return resolvedCommentTarget{}, output.ErrValidation("local comments only support docx, sheet, and slides; old doc format only supports full comments") + return resolvedCommentTarget{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "local comments only support docx, sheet, and slides; old doc format only supports full comments") case "file": if err := validateFileCommentMode(mode, ""); err != nil { return resolvedCommentTarget{}, err @@ -535,7 +535,7 @@ func resolveCommentTarget(ctx context.Context, runtime *common.RuntimeContext, i } fmt.Fprintf(runtime.IO().ErrOut, "Resolving wiki node: %s\n", common.MaskToken(docRef.Token)) - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "GET", "/open-apis/wiki/v2/spaces/get_node", map[string]interface{}{"token": docRef.Token}, @@ -549,13 +549,13 @@ func resolveCommentTarget(ctx context.Context, runtime *common.RuntimeContext, i objType := common.GetString(node, "obj_type") objToken := common.GetString(node, "obj_token") if objType == "" || objToken == "" { - return resolvedCommentTarget{}, output.Errorf(output.ExitAPI, "api_error", "wiki get_node returned incomplete node data") + return resolvedCommentTarget{}, errs.NewInternalError(errs.SubtypeInvalidResponse, "wiki get_node returned incomplete node data") } if objType == "slides" && mode == commentModeFull { - return resolvedCommentTarget{}, output.ErrValidation("wiki resolved to %q, but slide comments require --block-id !; --full-comment is not applicable", objType) + return resolvedCommentTarget{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "wiki resolved to %q, but slide comments require --block-id !; --full-comment is not applicable", objType) } if objType == "slides" && strings.TrimSpace(runtime.Str("selection-with-ellipsis")) != "" { - return resolvedCommentTarget{}, output.ErrValidation("wiki resolved to %q, but --selection-with-ellipsis is not applicable for slide comments; use --block-id !", objType) + return resolvedCommentTarget{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "wiki resolved to %q, but --selection-with-ellipsis is not applicable for slide comments; use --block-id !", objType) } if objType == "sheet" { // Sheet comments are handled via the sheet fast path in Execute. @@ -592,10 +592,10 @@ func resolveCommentTarget(ctx context.Context, runtime *common.RuntimeContext, i }, nil } if mode == commentModeLocal && objType != "docx" { - return resolvedCommentTarget{}, output.ErrValidation("wiki resolved to %q, but local comments only support docx, sheet, and slides; for sheet use --block-id !, for slides use --block-id !", objType) + return resolvedCommentTarget{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "wiki resolved to %q, but local comments only support docx, sheet, and slides; for sheet use --block-id !, for slides use --block-id !", objType) } if mode == commentModeFull && objType != "docx" && objType != "doc" { - return resolvedCommentTarget{}, output.ErrValidation("wiki resolved to %q, but comments only support doc/docx/file/sheet/slides", objType) + return resolvedCommentTarget{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "wiki resolved to %q, but comments only support doc/docx/file/sheet/slides", objType) } fmt.Fprintf(runtime.IO().ErrOut, "Resolved wiki to %s: %s\n", objType, common.MaskToken(objToken)) @@ -663,16 +663,14 @@ func parseLocateDocResult(result map[string]interface{}) locateDocResult { func selectLocateMatch(result locateDocResult) (locateDocMatch, int, error) { if len(result.Matches) == 0 { - return locateDocMatch{}, 0, output.ErrValidation("locate-doc did not find any matching block") + return locateDocMatch{}, 0, errs.NewValidationError(errs.SubtypeInvalidArgument, "locate-doc did not find any matching block").WithParam("--selection-with-ellipsis") } if len(result.Matches) > 1 { - return locateDocMatch{}, 0, output.ErrWithHint( - output.ExitValidation, - "ambiguous_match", - fmt.Sprintf("locate-doc matched %d blocks:\n%s", len(result.Matches), formatLocateCandidates(result.Matches)), - "narrow --selection-with-ellipsis until only one block matches", - ) + return locateDocMatch{}, 0, errs.NewValidationError(errs.SubtypeInvalidArgument, + "locate-doc matched %d blocks:\n%s", len(result.Matches), formatLocateCandidates(result.Matches)). + WithHint("narrow --selection-with-ellipsis until only one block matches"). + WithParam("--selection-with-ellipsis") } return result.Matches[0], 1, nil @@ -705,15 +703,15 @@ func summarizeLocateMatch(match locateDocMatch) string { func parseCommentReplyElements(raw string) ([]map[string]interface{}, error) { if strings.TrimSpace(raw) == "" { - return nil, output.ErrValidation("--content cannot be empty") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--content cannot be empty").WithParam("--content") } var inputs []commentReplyElementInput if err := json.Unmarshal([]byte(raw), &inputs); err != nil { - return nil, output.ErrValidation("--content is not valid JSON: %s\nexample: --content '[{\"type\":\"text\",\"text\":\"文本信息\"}]'", err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--content is not valid JSON: %s\nexample: --content '[{\"type\":\"text\",\"text\":\"文本信息\"}]'", err).WithParam("--content") } if len(inputs) == 0 { - return nil, output.ErrValidation("--content must contain at least one reply element") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--content must contain at least one reply element").WithParam("--content") } replyElements := make([]map[string]interface{}, 0, len(inputs)) @@ -724,7 +722,7 @@ func parseCommentReplyElements(raw string) ([]map[string]interface{}, error) { switch elementType { case "text": if strings.TrimSpace(input.Text) == "" { - return nil, output.ErrValidation("--content element #%d type=text requires non-empty text", index) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--content element #%d type=text requires non-empty text", index).WithParam("--content") } // Measure the raw rune count of the user input — that is what // the server actually counts. byte width and post-escape form @@ -734,13 +732,11 @@ func parseCommentReplyElements(raw string) ([]map[string]interface{}, error) { runes := utf8.RuneCountInString(input.Text) totalRunes += runes if totalRunes > maxCommentTotalRunes { - return nil, output.ErrWithHint( - output.ExitValidation, - "text_too_long", - fmt.Sprintf("--content reply_elements text totals %d characters at element #%d (this element: %d); the server caps the combined length at %d characters across ALL reply_elements", - totalRunes, index, runes, maxCommentTotalRunes), - fmt.Sprintf("shorten the comment so the combined text across all reply_elements fits within %d characters. The server enforces this cap on the TOTAL — splitting one long element into multiple smaller text elements does NOT help (they all add up against the same %d-rune budget). Server returns an opaque [1069302] on overflow, so this check is pre-flight; no escape transform changes the count (server reads raw runes).", maxCommentTotalRunes, maxCommentTotalRunes), - ) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, + "--content reply_elements text totals %d characters at element #%d (this element: %d); the server caps the combined length at %d characters across ALL reply_elements", + totalRunes, index, runes, maxCommentTotalRunes). + WithHint("shorten the comment so the combined text across all reply_elements fits within %d characters. The server enforces this cap on the TOTAL — splitting one long element into multiple smaller text elements does NOT help (they all add up against the same %d-rune budget). Server returns an opaque [1069302] on overflow, so this check is pre-flight; no escape transform changes the count (server reads raw runes).", maxCommentTotalRunes, maxCommentTotalRunes). + WithParam("--content") } // Escape '<' and '>' so the rendered comment displays them as // literal characters instead of being interpreted as markup @@ -754,7 +750,7 @@ func parseCommentReplyElements(raw string) ([]map[string]interface{}, error) { case "mention_user": mentionUser := firstNonEmptyString(input.MentionUser, input.Text) if mentionUser == "" { - return nil, output.ErrValidation("--content element #%d type=mention_user requires text or mention_user", index) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--content element #%d type=mention_user requires text or mention_user", index).WithParam("--content") } replyElements = append(replyElements, map[string]interface{}{ "type": "mention_user", @@ -763,14 +759,14 @@ func parseCommentReplyElements(raw string) ([]map[string]interface{}, error) { case "link": link := firstNonEmptyString(input.Link, input.Text) if link == "" { - return nil, output.ErrValidation("--content element #%d type=link requires text or link", index) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--content element #%d type=link requires text or link", index).WithParam("--content") } replyElements = append(replyElements, map[string]interface{}{ "type": "link", "link": link, }) default: - return nil, output.ErrValidation("--content element #%d has unsupported type %q; allowed values: text, mention_user, link", index, input.Type) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--content element #%d has unsupported type %q; allowed values: text, mention_user, link", index, input.Type).WithParam("--content") } } @@ -827,17 +823,17 @@ func anchorBlockIDForDryRun(blockID string) string { func parseSlidesBlockRef(blockID string) (string, string, error) { blockID = strings.TrimSpace(blockID) if blockID == "" { - return "", "", output.ErrValidation("slide comments require --block-id in ! format") + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "slide comments require --block-id in ! format").WithParam("--block-id") } parts := strings.SplitN(blockID, "!", 2) if len(parts) != 2 { - return "", "", output.ErrValidation("slide --block-id must be ! (e.g. shape!bPq), got %q", blockID) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "slide --block-id must be ! (e.g. shape!bPq), got %q", blockID).WithParam("--block-id") } parsedType := strings.TrimSpace(parts[0]) parsedID := strings.TrimSpace(parts[1]) if parsedType == "" || parsedID == "" { - return "", "", output.ErrValidation("slide --block-id must be ! (e.g. shape!bPq), got %q", blockID) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "slide --block-id must be ! (e.g. shape!bPq), got %q", blockID).WithParam("--block-id") } return parsedID, parsedType, nil } @@ -865,7 +861,7 @@ func firstPresentValue(m map[string]interface{}, keys ...string) interface{} { func parseSheetCellRef(input string) (*sheetAnchor, error) { parts := strings.SplitN(input, "!", 2) if len(parts) != 2 || parts[0] == "" || parts[1] == "" { - return nil, output.ErrValidation("--block-id for sheet must be ! (e.g. a281f9!D6), got %q", input) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--block-id for sheet must be ! (e.g. a281f9!D6), got %q", input).WithParam("--block-id") } sheetID := parts[0] cell := strings.TrimSpace(parts[1]) @@ -876,7 +872,7 @@ func parseSheetCellRef(input string) (*sheetAnchor, error) { i++ } if i == 0 || i >= len(cell) { - return nil, output.ErrValidation("--block-id cell reference %q is invalid (expected e.g. D6)", cell) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--block-id cell reference %q is invalid (expected e.g. D6)", cell).WithParam("--block-id") } colStr := strings.ToUpper(cell[:i]) rowStr := cell[i:] @@ -890,7 +886,7 @@ func parseSheetCellRef(input string) (*sheetAnchor, error) { row, err := strconv.Atoi(rowStr) if err != nil || row < 1 { - return nil, output.ErrValidation("--block-id row %q is invalid (must be >= 1)", rowStr) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--block-id row %q is invalid (must be >= 1)", rowStr).WithParam("--block-id") } row-- // convert to 0-based @@ -898,7 +894,7 @@ func parseSheetCellRef(input string) (*sheetAnchor, error) { } func fetchCommentTargetFileTitle(runtime *common.RuntimeContext, fileToken string) (string, error) { - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "POST", "/open-apis/drive/v1/metas/batch_query", nil, @@ -917,11 +913,11 @@ func fetchCommentTargetFileTitle(runtime *common.RuntimeContext, fileToken strin metas := common.GetSlice(data, "metas") if len(metas) == 0 { - return "", output.Errorf(output.ExitAPI, "api_error", "drive metas.batch_query returned no metadata for file %s", common.MaskToken(fileToken)) + return "", errs.NewInternalError(errs.SubtypeInvalidResponse, "drive metas.batch_query returned no metadata for file %s", common.MaskToken(fileToken)) } meta, ok := metas[0].(map[string]interface{}) if !ok { - return "", output.Errorf(output.ExitAPI, "api_error", "drive metas.batch_query returned unexpected metadata format for file %s", common.MaskToken(fileToken)) + return "", errs.NewInternalError(errs.SubtypeInvalidResponse, "drive metas.batch_query returned unexpected metadata format for file %s", common.MaskToken(fileToken)) } return common.GetString(meta, "title"), nil } @@ -936,23 +932,19 @@ func ensureSupportedFileCommentTarget(runtime *common.RuntimeContext, fileToken return title, extension, nil } if strings.TrimSpace(title) == "" { - return "", "", output.ErrWithHint( - output.ExitValidation, - "unsupported_file_comment_type", - "drive +add-comment does not support comments for this Drive file type yet; the file metadata did not return a title", - "file comments currently support full comments only for these extensions: "+supportedFileCommentExtensionsText(), - ) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, + "drive +add-comment does not support comments for this Drive file type yet; the file metadata did not return a title"). + WithHint("file comments currently support full comments only for these extensions: " + supportedFileCommentExtensionsText()). + WithParam("--doc") } extensionLabel := extension if extensionLabel == "" { extensionLabel = "no extension" } - return "", "", output.ErrWithHint( - output.ExitValidation, - "unsupported_file_comment_type", - fmt.Sprintf("drive +add-comment does not support comments for this Drive file type yet; got %q (%s)", title, extensionLabel), - "file comments currently support full comments only for these extensions: "+supportedFileCommentExtensionsText(), - ) + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, + "drive +add-comment does not support comments for this Drive file type yet; got %q (%s)", title, extensionLabel). + WithHint("file comments currently support full comments only for these extensions: " + supportedFileCommentExtensionsText()). + WithParam("--doc") } func fileCommentExtension(title string) string { @@ -993,9 +985,9 @@ func validateFileCommentMode(mode commentMode, resolvedObjType string) error { return nil } if resolvedObjType != "" { - return output.ErrValidation("wiki resolved to %q, but file comments only support full comments; omit --block-id and --selection-with-ellipsis", resolvedObjType) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "wiki resolved to %q, but file comments only support full comments; omit --block-id and --selection-with-ellipsis", resolvedObjType) } - return output.ErrValidation("file comments only support full comments; omit --block-id and --selection-with-ellipsis") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "file comments only support full comments; omit --block-id and --selection-with-ellipsis") } func executeSheetComment(runtime *common.RuntimeContext, docRef commentDocRef) error { @@ -1006,7 +998,7 @@ func executeSheetComment(runtime *common.RuntimeContext, docRef commentDocRef) e blockID := strings.TrimSpace(runtime.Str("block-id")) if blockID == "" { - return output.ErrValidation("--block-id is required for sheet comments (format: !, e.g. a281f9!D6)") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--block-id is required for sheet comments (format: !, e.g. a281f9!D6)").WithParam("--block-id") } anchor, err := parseSheetCellRef(blockID) if err != nil { @@ -1019,7 +1011,7 @@ func executeSheetComment(runtime *common.RuntimeContext, docRef commentDocRef) e fmt.Fprintf(runtime.IO().ErrOut, "Creating sheet comment in %s (sheet=%s, col=%d, row=%d)\n", common.MaskToken(docRef.Token), anchor.SheetID, anchor.Col, anchor.Row) - data, err := runtime.CallAPI("POST", requestPath, nil, requestBody) + data, err := runtime.CallAPITyped("POST", requestPath, nil, requestBody) if err != nil { return err } @@ -1054,7 +1046,7 @@ func executeFileComment(runtime *common.RuntimeContext, target resolvedCommentTa fmt.Fprintf(runtime.IO().ErrOut, "Creating file comment in %s (%s)\n", common.MaskToken(target.FileToken), extension) - data, err := runtime.CallAPI("POST", requestPath, nil, requestBody) + data, err := runtime.CallAPITyped("POST", requestPath, nil, requestBody) if err != nil { return err } @@ -1097,7 +1089,7 @@ func executeSlidesComment(runtime *common.RuntimeContext, docRef commentDocRef) fmt.Fprintf(runtime.IO().ErrOut, "Creating slide block comment in %s (block_id=%s, slide_block_type=%s)\n", common.MaskToken(docRef.Token), blockID, slideBlockType) - data, err := runtime.CallAPI("POST", requestPath, nil, requestBody) + data, err := runtime.CallAPITyped("POST", requestPath, nil, requestBody) if err != nil { return err } diff --git a/shortcuts/drive/drive_add_comment_test.go b/shortcuts/drive/drive_add_comment_test.go index 7875893eb..5d39d5d34 100644 --- a/shortcuts/drive/drive_add_comment_test.go +++ b/shortcuts/drive/drive_add_comment_test.go @@ -9,11 +9,32 @@ import ( "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/httpmock" - "github.com/larksuite/cli/internal/output" ) +// assertContentValidationHint asserts err is a typed *errs.ValidationError +// carrying SubtypeInvalidArgument, Param "--content", and a Hint containing +// the given substring. The over-cap message now flows through a typed +// ValidationError instead of the legacy *output.ExitError.Detail shape. +func assertContentValidationHint(t *testing.T, err error, wantHint string) { + t.Helper() + var valErr *errs.ValidationError + if !errors.As(err, &valErr) { + t.Fatalf("expected *errs.ValidationError, got %T (%v)", err, err) + } + if valErr.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("Subtype = %q, want %q", valErr.Subtype, errs.SubtypeInvalidArgument) + } + if valErr.Param != "--content" { + t.Errorf("Param = %q, want %q", valErr.Param, "--content") + } + if !strings.Contains(valErr.Hint, wantHint) { + t.Errorf("expected hint substring %q, got %q", wantHint, valErr.Hint) + } +} + func decodeJSONMap(t *testing.T, raw string) map[string]interface{} { t.Helper() @@ -421,14 +442,8 @@ func TestParseCommentReplyElementsTextLength(t *testing.T) { t.Fatalf("expected error containing %q, got %q", tt.wantErr, err.Error()) } if tt.wantHint != "" { - // Hint lives on ExitError.Detail.Hint, not err.Error(). - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected ExitError with Detail, got %T (%v)", err, err) - } - if !strings.Contains(exitErr.Detail.Hint, tt.wantHint) { - t.Errorf("expected hint substring %q, got %q", tt.wantHint, exitErr.Detail.Hint) - } + // Hint lives on the typed ValidationError, not err.Error(). + assertContentValidationHint(t, err, tt.wantHint) } return } @@ -458,11 +473,11 @@ func TestParseCommentReplyElementsHintForbidsSplitAdvice(t *testing.T) { if err == nil { t.Fatal("expected over-cap error, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected ExitError with Detail, got %T (%v)", err, err) + var valErr *errs.ValidationError + if !errors.As(err, &valErr) { + t.Fatalf("expected *errs.ValidationError, got %T (%v)", err, err) } - hint := exitErr.Detail.Hint + hint := valErr.Hint // The hint must explicitly call out that splitting does NOT help. if !strings.Contains(hint, "does NOT help") { diff --git a/shortcuts/drive/drive_apply_permission.go b/shortcuts/drive/drive_apply_permission.go index dff8a8474..2fd76b37b 100644 --- a/shortcuts/drive/drive_apply_permission.go +++ b/shortcuts/drive/drive_apply_permission.go @@ -8,7 +8,7 @@ import ( "fmt" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -44,7 +44,7 @@ var permApplyURLMarkers = []struct { func resolvePermApplyTarget(raw, explicitType string) (token, docType string, err error) { raw = strings.TrimSpace(raw) if raw == "" { - return "", "", output.ErrValidation("--token is required") + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--token is required").WithParam("--token") } if strings.Contains(raw, "://") { @@ -58,10 +58,10 @@ func resolvePermApplyTarget(raw, explicitType string) (token, docType string, er } } if token == "" { - return "", "", output.ErrValidation( + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "could not infer token from URL %q: supported paths are /docx/, /sheets/, /base/, /bitable/, /file/, /wiki/, /doc/, /mindnote/, /slides/. Pass a bare token with --type instead if the URL shape is unusual", raw, - ) + ).WithParam("--token") } } else { token = raw @@ -71,10 +71,10 @@ func resolvePermApplyTarget(raw, explicitType string) (token, docType string, er docType = explicitType } if docType == "" { - return "", "", output.ErrValidation( + return "", "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--type is required when --token is a bare token; accepted values: %s", strings.Join(permApplyTypes, ", "), - ) + ).WithParam("--type") } return token, docType, nil } @@ -125,7 +125,7 @@ var DriveApplyPermission = common.Shortcut{ fmt.Fprintf(runtime.IO().ErrOut, "Requesting %s access on %s %s...\n", runtime.Str("perm"), docType, common.MaskToken(token)) - data, err := runtime.CallAPI("POST", + data, err := runtime.CallAPITyped("POST", fmt.Sprintf("/open-apis/drive/v1/permissions/%s/members/apply", validate.EncodePathSegment(token)), map[string]interface{}{"type": docType}, body, diff --git a/shortcuts/drive/drive_create_folder.go b/shortcuts/drive/drive_create_folder.go index 75c50aee6..38507d491 100644 --- a/shortcuts/drive/drive_create_folder.go +++ b/shortcuts/drive/drive_create_folder.go @@ -8,7 +8,7 @@ import ( "fmt" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -72,7 +72,7 @@ var DriveCreateFolder = common.Shortcut{ } fmt.Fprintf(runtime.IO().ErrOut, "Creating folder %q in %s...\n", spec.Name, target) - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "POST", "/open-apis/drive/v1/files/create_folder", nil, @@ -84,7 +84,7 @@ var DriveCreateFolder = common.Shortcut{ folderToken := common.GetString(data, "token") if folderToken == "" { - return output.Errorf(output.ExitAPI, "api_error", "drive create_folder succeeded but returned no folder token (data.token)") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "drive create_folder succeeded but returned no folder token (data.token)") } out := map[string]interface{}{ "created": true, @@ -108,14 +108,14 @@ var DriveCreateFolder = common.Shortcut{ func validateDriveCreateFolderSpec(spec driveCreateFolderSpec) error { if spec.Name == "" { - return output.ErrValidation("--name must not be empty") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--name must not be empty").WithParam("--name") } if nameBytes := len([]byte(spec.Name)); nameBytes > 256 { - return output.ErrValidation("--name exceeds the maximum of 256 bytes (got %d)", nameBytes) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--name exceeds the maximum of 256 bytes (got %d)", nameBytes).WithParam("--name") } if spec.FolderToken != "" { if err := validate.ResourceName(spec.FolderToken, "--folder-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--folder-token") } } return nil diff --git a/shortcuts/drive/drive_create_shortcut.go b/shortcuts/drive/drive_create_shortcut.go index 92d0c5dc5..b79499631 100644 --- a/shortcuts/drive/drive_create_shortcut.go +++ b/shortcuts/drive/drive_create_shortcut.go @@ -8,7 +8,7 @@ import ( "fmt" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -84,7 +84,7 @@ var DriveCreateShortcut = common.Shortcut{ common.MaskToken(spec.FolderToken), ) - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "POST", "/open-apis/drive/v1/files/create_shortcut", nil, @@ -118,19 +118,19 @@ var DriveCreateShortcut = common.Shortcut{ // validateDriveCreateShortcutSpec validates shortcut creation inputs before API execution. func validateDriveCreateShortcutSpec(spec driveCreateShortcutSpec) error { if err := validate.ResourceName(spec.FileToken, "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--file-token") } if err := validate.ResourceName(spec.FolderToken, "--folder-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--folder-token") } if spec.FileType == "wiki" { - return output.ErrValidation("unsupported file type: wiki. This shortcut only supports Drive file tokens; wiki documents must be resolved to their underlying file token first") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported file type: wiki. This shortcut only supports Drive file tokens; wiki documents must be resolved to their underlying file token first").WithParam("--type") } if spec.FileType == "folder" { - return output.ErrValidation("unsupported file type: folder. The create_shortcut API only supports Drive files, not folders") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported file type: folder. The create_shortcut API only supports Drive files, not folders").WithParam("--type") } if !driveCreateShortcutAllowedTypes[spec.FileType] { - return output.ErrValidation("unsupported file type: %s. Supported types: file, docx, bitable, doc, sheet, mindnote, slides", spec.FileType) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported file type: %s. Supported types: file, docx, bitable, doc, sheet, mindnote, slides", spec.FileType).WithParam("--type") } return nil } diff --git a/shortcuts/drive/drive_create_shortcut_test.go b/shortcuts/drive/drive_create_shortcut_test.go index 942e079d8..14d83048d 100644 --- a/shortcuts/drive/drive_create_shortcut_test.go +++ b/shortcuts/drive/drive_create_shortcut_test.go @@ -12,6 +12,7 @@ import ( "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/httpmock" "github.com/larksuite/cli/internal/output" @@ -312,24 +313,24 @@ func TestDriveCreateShortcutClassifiesKnownAPIConstraints(t *testing.T) { t.Fatal("expected API error, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured exit error, got %v", err) + var apiErr *errs.APIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected *errs.APIError, got %T (%v)", err, err) } - if exitErr.Code != output.ExitAPI { - t.Fatalf("exit code = %d, want %d", exitErr.Code, output.ExitAPI) + if output.ExitCodeOf(err) != output.ExitAPI { + t.Fatalf("exit code = %d, want %d", output.ExitCodeOf(err), output.ExitAPI) } - if exitErr.Detail.Type != tt.wantType { - t.Fatalf("type = %q, want %q", exitErr.Detail.Type, tt.wantType) + if string(apiErr.Subtype) != tt.wantType { + t.Fatalf("subtype = %q, want %q", apiErr.Subtype, tt.wantType) } - if exitErr.Detail.Code != tt.code { - t.Fatalf("detail code = %d, want %d", exitErr.Detail.Code, tt.code) + if apiErr.Code != tt.code { + t.Fatalf("code = %d, want %d", apiErr.Code, tt.code) } - if !strings.Contains(exitErr.Detail.Message, tt.wantMsgPart) { - t.Fatalf("message = %q, want substring %q", exitErr.Detail.Message, tt.wantMsgPart) + if !strings.Contains(apiErr.Message, tt.wantMsgPart) { + t.Fatalf("message = %q, want substring %q", apiErr.Message, tt.wantMsgPart) } - if !strings.Contains(exitErr.Detail.Hint, tt.wantHint) { - t.Fatalf("hint = %q, want substring %q", exitErr.Detail.Hint, tt.wantHint) + if !strings.Contains(apiErr.Hint, tt.wantHint) { + t.Fatalf("hint = %q, want substring %q", apiErr.Hint, tt.wantHint) } }) } diff --git a/shortcuts/drive/drive_delete.go b/shortcuts/drive/drive_delete.go index 98a331e6e..d9190a793 100644 --- a/shortcuts/drive/drive_delete.go +++ b/shortcuts/drive/drive_delete.go @@ -8,7 +8,7 @@ import ( "fmt" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -81,7 +81,7 @@ var DriveDelete = common.Shortcut{ fmt.Fprintf(runtime.IO().ErrOut, "Deleting %s %s...\n", spec.FileType, common.MaskToken(spec.FileToken)) - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "DELETE", fmt.Sprintf("/open-apis/drive/v1/files/%s", validate.EncodePathSegment(spec.FileToken)), map[string]interface{}{"type": spec.FileType}, @@ -94,7 +94,7 @@ var DriveDelete = common.Shortcut{ if spec.FileType == "folder" { taskID := common.GetString(data, "task_id") if taskID == "" { - return output.Errorf(output.ExitAPI, "api_error", "delete folder returned no task_id") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "delete folder returned no task_id") } fmt.Fprintf(runtime.IO().ErrOut, "Folder delete is async, polling task %s...\n", taskID) @@ -136,13 +136,13 @@ var DriveDelete = common.Shortcut{ func validateDriveDeleteSpec(spec driveDeleteSpec) error { if err := validate.ResourceName(spec.FileToken, "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--file-token") } if spec.FileType == "wiki" { - return output.ErrValidation("unsupported file type: wiki. This shortcut only supports Drive files and folders; wiki documents are not supported") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported file type: wiki. This shortcut only supports Drive files and folders; wiki documents are not supported").WithParam("--type") } if !driveDeleteAllowedTypes[spec.FileType] { - return output.ErrValidation("unsupported file type: %s. Supported types: file, docx, bitable, doc, sheet, mindnote, folder, shortcut, slides", spec.FileType) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported file type: %s. Supported types: file, docx, bitable, doc, sheet, mindnote, folder, shortcut, slides", spec.FileType).WithParam("--type") } return nil } diff --git a/shortcuts/drive/drive_download.go b/shortcuts/drive/drive_download.go index 13d96d619..2a29e16a3 100644 --- a/shortcuts/drive/drive_download.go +++ b/shortcuts/drive/drive_download.go @@ -10,8 +10,8 @@ import ( larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -44,7 +44,7 @@ var DriveDownload = common.Shortcut{ overwrite := runtime.Bool("overwrite") if err := validate.ResourceName(fileToken, "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--file-token") } if outputPath == "" { @@ -53,10 +53,10 @@ var DriveDownload = common.Shortcut{ // Early path validation + overwrite check if _, resolveErr := runtime.ResolveSavePath(outputPath); resolveErr != nil { - return output.ErrValidation("unsafe output path: %s", resolveErr) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", resolveErr).WithParam("--output") } if _, statErr := runtime.FileIO().Stat(outputPath); statErr == nil && !overwrite { - return output.ErrValidation("output file already exists: %s (use --overwrite to replace)", outputPath) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "output file already exists: %s (use --overwrite to replace)", outputPath).WithParam("--output") } fmt.Fprintf(runtime.IO().ErrOut, "Downloading: %s\n", common.MaskToken(fileToken)) @@ -66,7 +66,7 @@ var DriveDownload = common.Shortcut{ ApiPath: fmt.Sprintf("/open-apis/drive/v1/files/%s/download", validate.EncodePathSegment(fileToken)), }) if err != nil { - return output.ErrNetwork("download failed: %s", err) + return wrapDriveNetworkErr(err, "download failed: %s", err) } defer resp.Body.Close() @@ -75,7 +75,7 @@ var DriveDownload = common.Shortcut{ ContentLength: resp.ContentLength, }, resp.Body) if err != nil { - return common.WrapSaveErrorByCategory(err, "io") + return driveSaveError(err) } savedPath, _ := runtime.ResolveSavePath(outputPath) diff --git a/shortcuts/drive/drive_duplicate_remote_test.go b/shortcuts/drive/drive_duplicate_remote_test.go index 6c8a391f6..574c1c1bb 100644 --- a/shortcuts/drive/drive_duplicate_remote_test.go +++ b/shortcuts/drive/drive_duplicate_remote_test.go @@ -17,9 +17,9 @@ import ( "testing" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/httpmock" - "github.com/larksuite/cli/internal/output" ) const ( @@ -823,64 +823,37 @@ func registerDownload(reg *httpmock.Registry, fileToken, body string) { func assertDuplicateRemotePathError(t *testing.T, err error, relPath string, tokens ...string) { t.Helper() if err == nil { - t.Fatal("expected duplicate_remote_path error, got nil") + t.Fatal("expected duplicate rel_path validation error, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("expected *output.ExitError, got %T: %v", err, err) + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) } - if exitErr.Code != output.ExitAPI { - t.Fatalf("exit code = %d, want %d", exitErr.Code, output.ExitAPI) + if validationErr.Subtype != errs.SubtypeFailedPrecondition { + t.Fatalf("subtype = %q, want %q", validationErr.Subtype, errs.SubtypeFailedPrecondition) } - if exitErr.Detail == nil || exitErr.Detail.Type != "duplicate_remote_path" { - t.Fatalf("error detail = %#v, want duplicate_remote_path", exitErr.Detail) + if validationErr.Hint == "" { + t.Fatal("duplicate validation error should carry a recovery hint so AI consumers know the next action") } - detailMap, ok := exitErr.Detail.Detail.(map[string]interface{}) - if !ok { - t.Fatalf("duplicate detail type = %T, want map[string]interface{}", exitErr.Detail.Detail) + if len(validationErr.Params) == 0 { + t.Fatal("duplicate validation error should carry at least one param") } - duplicates, ok := detailMap["duplicates_remote"].([]driveDuplicateRemotePath) - if !ok { - t.Fatalf("duplicate detail duplicates_remote type = %T, want []driveDuplicateRemotePath", detailMap["duplicates_remote"]) - } - if len(duplicates) == 0 { - t.Fatal("duplicate detail should include at least one rel_path group") - } - if _, hasLegacyFilesKey := detailMap["files"]; hasLegacyFilesKey { - t.Fatalf("duplicate detail should not expose legacy files key: %#v", detailMap) - } - var matched bool - for _, duplicate := range duplicates { - if duplicate.RelPath != relPath { - continue + var matched *errs.InvalidParam + for i := range validationErr.Params { + if validationErr.Params[i].Name == relPath { + matched = &validationErr.Params[i] + break } - matched = true - if len(duplicate.Entries) != len(tokens) { - t.Fatalf("duplicate entry count = %d, want %d for rel_path %q", len(duplicate.Entries), len(tokens), relPath) - } - for i, token := range tokens { - if duplicate.Entries[i].FileToken != token { - t.Fatalf("duplicate entry %d file_token = %q, want %q", i, duplicate.Entries[i].FileToken, token) - } - if duplicate.Entries[i].Type == "" { - t.Fatalf("duplicate entry %d missing type for rel_path %q", i, relPath) - } - } - } - if !matched { - t.Fatalf("duplicate detail missing rel_path group %q: %#v", relPath, duplicates) } - raw, marshalErr := json.Marshal(exitErr.Detail.Detail) - if marshalErr != nil { - t.Fatalf("marshal detail: %v", marshalErr) + if matched == nil { + t.Fatalf("duplicate params missing rel_path group %q: %#v", relPath, validationErr.Params) } - text := string(raw) - if !strings.Contains(text, relPath) { - t.Fatalf("duplicate detail missing rel_path %q: %s", relPath, text) + if matched.Reason == "" { + t.Fatalf("duplicate param for rel_path %q missing reason", relPath) } for _, token := range tokens { - if !strings.Contains(text, token) { - t.Fatalf("duplicate detail missing token %q: %s", token, text) + if !strings.Contains(matched.Reason, token) { + t.Fatalf("duplicate param reason missing token %q: %s", token, matched.Reason) } } } diff --git a/shortcuts/drive/drive_errors.go b/shortcuts/drive/drive_errors.go new file mode 100644 index 000000000..4184d158b --- /dev/null +++ b/shortcuts/drive/drive_errors.go @@ -0,0 +1,89 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package drive + +import ( + "errors" + "strings" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/extension/fileio" + "github.com/larksuite/cli/internal/output" +) + +// wrapDriveNetworkErr returns err unchanged when it is already a typed errs.* +// error (preserving its subtype / code / log_id from the runtime boundary), +// and only wraps a raw, unclassified error as a transport-level network error. +func wrapDriveNetworkErr(err error, format string, args ...any) error { + if _, ok := errs.ProblemOf(err); ok { + return err + } + return errs.NewNetworkError(errs.SubtypeNetworkTransport, format, args...).WithCause(err) +} + +// driveInputStatError maps a FileIO.Stat/Open error for input file validation +// to a typed validation error: +// - Path validation failures → "unsafe file path: ..." +// - Other errors → "cannot read file: ..." +func driveInputStatError(err error) error { + if err == nil { + return nil + } + if errors.Is(err, fileio.ErrPathValidation) { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe file path: %s", err).WithCause(err) + } + return errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot read file: %s", err).WithCause(err) +} + +// driveSaveError maps a FileIO.Save error to a typed error. Path validation +// failures are validation errors (exit code 2); mkdir / write failures are +// internal file-I/O errors (exit code 5). +func driveSaveError(err error) error { + if err == nil { + return nil + } + var me *fileio.MkdirError + switch { + case errors.Is(err, fileio.ErrPathValidation): + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", err).WithCause(err) + case errors.As(err, &me): + return errs.NewInternalError(errs.SubtypeFileIO, "cannot create parent directory: %s", err).WithCause(err) + default: + return errs.NewInternalError(errs.SubtypeFileIO, "cannot create file: %s", err).WithCause(err) + } +} + +// appendDriveExportRecoveryHint attaches a recovery hint to err while preserving +// its original classification (typed subtype/code or legacy detail), only falling +// back to a typed internal error when err is unclassified. +func appendDriveExportRecoveryHint(err error, hint string) error { + if err == nil { + return nil + } + // An already-typed error keeps its own category/subtype/code/log_id + // (per ERROR_CONTRACT.md "propagate typed errors unchanged"); we only + // append the recovery hint. p points at the embedded Problem, so the + // mutation is reflected in the returned err. + if p, ok := errs.ProblemOf(err); ok { + if strings.TrimSpace(p.Hint) != "" { + p.Hint = p.Hint + "\n" + hint + } else { + p.Hint = hint + } + return err + } + // Legacy *output.ExitError fallback: preserve the original error's + // class/exit code by appending the hint in place rather than downgrading + // to api/server_error. + var exitErr *output.ExitError + if errors.As(err, &exitErr) && exitErr.Detail != nil { + if strings.TrimSpace(exitErr.Detail.Hint) != "" { + exitErr.Detail.Hint = exitErr.Detail.Hint + "\n" + hint + } else { + exitErr.Detail.Hint = hint + } + return err + } + return errs.NewInternalError(errs.SubtypeSDKError, "%s", err.Error()).WithHint(hint).WithCause(err) +} diff --git a/shortcuts/drive/drive_export.go b/shortcuts/drive/drive_export.go index e0031fdb0..86e7c99a5 100644 --- a/shortcuts/drive/drive_export.go +++ b/shortcuts/drive/drive_export.go @@ -5,13 +5,12 @@ package drive import ( "context" - "errors" "fmt" "path/filepath" "strings" "time" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -107,7 +106,7 @@ var DriveExport = common.Shortcut{ if spec.FileExtension == "markdown" { fmt.Fprintf(runtime.IO().ErrOut, "Exporting docx as markdown: %s\n", common.MaskToken(spec.Token)) apiPath := fmt.Sprintf("/open-apis/docs_ai/v1/documents/%s/fetch", validate.EncodePathSegment(spec.Token)) - data, err := runtime.DoAPIJSONWithLogID( + data, err := runtime.CallAPITyped( "POST", apiPath, nil, @@ -122,11 +121,11 @@ var DriveExport = common.Shortcut{ // Extract content from the V2 response: data.document.content doc, ok := data["document"].(map[string]interface{}) if !ok { - return output.Errorf(output.ExitAPI, "api_error", "invalid markdown fetch response: missing document object") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "invalid markdown fetch response: missing document object") } content, ok := doc["content"].(string) if !ok { - return output.Errorf(output.ExitAPI, "api_error", "invalid markdown fetch response: missing document.content") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "invalid markdown fetch response: missing document.content") } fileName := preferredFileName @@ -207,11 +206,7 @@ var DriveExport = common.Shortcut{ status.FileToken, recoveryCommand, ) - var exitErr *output.ExitError - if errors.As(err, &exitErr) && exitErr.Detail != nil { - return output.ErrWithHint(exitErr.Code, exitErr.Detail.Type, exitErr.Detail.Message, hint) - } - return output.ErrWithHint(output.ExitAPI, "api_error", err.Error(), hint) + return appendDriveExportRecoveryHint(err, hint) } out["ticket"] = ticket out["doc_type"] = spec.DocType @@ -225,7 +220,7 @@ var DriveExport = common.Shortcut{ if msg == "" { msg = status.StatusLabel() } - return output.Errorf(output.ExitAPI, "api_error", "export task failed: %s (ticket=%s)", msg, ticket) + return errs.NewAPIError(errs.SubtypeServerError, "export task failed: %s (ticket=%s)", msg, ticket) } fmt.Fprintf(runtime.IO().ErrOut, "Export status %d/%d: %s\n", attempt, driveExportPollAttempts, status.StatusLabel()) @@ -238,14 +233,7 @@ var DriveExport = common.Shortcut{ ticket, nextCommand, ) - var exitErr *output.ExitError - if errors.As(lastPollErr, &exitErr) && exitErr.Detail != nil { - if strings.TrimSpace(exitErr.Detail.Hint) != "" { - hint = exitErr.Detail.Hint + "\n" + hint - } - return output.ErrWithHint(exitErr.Code, exitErr.Detail.Type, exitErr.Detail.Message, hint) - } - return output.ErrWithHint(output.ExitAPI, "api_error", lastPollErr.Error(), hint) + return appendDriveExportRecoveryHint(lastPollErr, hint) } failed := false diff --git a/shortcuts/drive/drive_export_common.go b/shortcuts/drive/drive_export_common.go index 529c01b0e..187ac3e73 100644 --- a/shortcuts/drive/drive_export_common.go +++ b/shortcuts/drive/drive_export_common.go @@ -15,9 +15,9 @@ import ( larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" "github.com/larksuite/cli/internal/client" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -127,48 +127,48 @@ func (s driveExportStatus) StatusLabel() string { // backend request is sent. func validateDriveExportSpec(spec driveExportSpec) error { if err := validate.ResourceName(spec.Token, "--token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--token") } switch spec.DocType { case "doc", "docx", "sheet", "bitable", "slides": default: - return output.ErrValidation("invalid --doc-type %q: allowed values are doc, docx, sheet, bitable, slides", spec.DocType) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --doc-type %q: allowed values are doc, docx, sheet, bitable, slides", spec.DocType).WithParam("--doc-type") } switch spec.FileExtension { case "docx", "pdf", "xlsx", "csv", "markdown", "base", "pptx": default: - return output.ErrValidation("invalid --file-extension %q: allowed values are docx, pdf, xlsx, csv, markdown, base, pptx", spec.FileExtension) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --file-extension %q: allowed values are docx, pdf, xlsx, csv, markdown, base, pptx", spec.FileExtension).WithParam("--file-extension") } if spec.FileExtension == "markdown" && spec.DocType != "docx" { - return output.ErrValidation("--file-extension markdown only supports --doc-type docx") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--file-extension markdown only supports --doc-type docx") } if spec.FileExtension == "base" && spec.DocType != "bitable" { - return output.ErrValidation("--file-extension base only supports --doc-type bitable") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--file-extension base only supports --doc-type bitable") } if spec.FileExtension == "pptx" && spec.DocType != "slides" { - return output.ErrValidation("--file-extension pptx only supports --doc-type slides") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--file-extension pptx only supports --doc-type slides") } if spec.DocType == "slides" && spec.FileExtension != "pptx" && spec.FileExtension != "pdf" { - return output.ErrValidation("--doc-type slides only supports --file-extension pptx or pdf") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--doc-type slides only supports --file-extension pptx or pdf") } if strings.TrimSpace(spec.SubID) != "" { if spec.FileExtension != "csv" || (spec.DocType != "sheet" && spec.DocType != "bitable") { - return output.ErrValidation("--sub-id is only used when exporting sheet/bitable as csv") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--sub-id is only used when exporting sheet/bitable as csv").WithParam("--sub-id") } if err := validate.ResourceName(spec.SubID, "--sub-id"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--sub-id") } } if spec.FileExtension == "csv" && (spec.DocType == "sheet" || spec.DocType == "bitable") && strings.TrimSpace(spec.SubID) == "" { - return output.ErrValidation("--sub-id is required when exporting sheet/bitable as csv") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--sub-id is required when exporting sheet/bitable as csv").WithParam("--sub-id") } return nil @@ -186,14 +186,14 @@ func createDriveExportTask(runtime *common.RuntimeContext, spec driveExportSpec) body["sub_id"] = spec.SubID } - data, err := runtime.CallAPI("POST", "/open-apis/drive/v1/export_tasks", nil, body) + data, err := runtime.CallAPITyped("POST", "/open-apis/drive/v1/export_tasks", nil, body) if err != nil { return "", err } ticket := common.GetString(data, "ticket") if ticket == "" { - return "", output.Errorf(output.ExitAPI, "api_error", "export task created but ticket is missing") + return "", errs.NewInternalError(errs.SubtypeInvalidResponse, "export task created but ticket is missing") } return ticket, nil } @@ -201,7 +201,7 @@ func createDriveExportTask(runtime *common.RuntimeContext, spec driveExportSpec) // getDriveExportStatus fetches the current backend state for a previously // created export task. func getDriveExportStatus(runtime *common.RuntimeContext, token, ticket string) (driveExportStatus, error) { - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "GET", fmt.Sprintf("/open-apis/drive/v1/export_tasks/%s", validate.EncodePathSegment(ticket)), map[string]interface{}{"token": token}, @@ -251,12 +251,12 @@ func saveContentToOutputDir(fio fileio.FileIO, outputDir, fileName string, paylo // Overwrite check via FileIO.Stat if !overwrite { if _, statErr := fio.Stat(target); statErr == nil { - return "", output.ErrValidation("output file already exists: %s (use --overwrite to replace)", target) + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "output file already exists: %s (use --overwrite to replace)", target) } } if _, err := fio.Save(target, fileio.SaveOptions{}, bytes.NewReader(payload)); err != nil { - return "", common.WrapSaveErrorByCategory(err, "io") + return "", driveSaveError(err) } resolvedPath, _ := fio.ResolvePath(target) if resolvedPath == "" { @@ -269,7 +269,7 @@ func saveContentToOutputDir(fio fileio.FileIO, outputDir, fileName string, paylo // file name, and returns metadata about the saved file. func downloadDriveExportFile(ctx context.Context, runtime *common.RuntimeContext, fileToken, outputDir, preferredName string, overwrite bool) (map[string]interface{}, error) { if err := validate.ResourceName(fileToken, "--file-token"); err != nil { - return nil, output.ErrValidation("%s", err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--file-token") } apiResp, err := runtime.DoAPI(&larkcore.ApiReq{ @@ -277,10 +277,24 @@ func downloadDriveExportFile(ctx context.Context, runtime *common.RuntimeContext ApiPath: fmt.Sprintf("/open-apis/drive/v1/export_tasks/file/%s/download", validate.EncodePathSegment(fileToken)), }, larkcore.WithFileDownload()) if err != nil { - return nil, output.ErrNetwork("download failed: %s", err) + return nil, wrapDriveNetworkErr(err, "download failed: %s", err) } if apiResp.StatusCode >= 400 { - return nil, output.ErrNetwork("download failed: HTTP %d: %s", apiResp.StatusCode, string(apiResp.RawBody)) + subtype := errs.SubtypeNetworkTransport + if apiResp.StatusCode >= 500 { + subtype = errs.SubtypeNetworkServer + } + e := errs.NewNetworkError(subtype, "download failed: HTTP %d: %s", apiResp.StatusCode, string(apiResp.RawBody)).WithCode(apiResp.StatusCode) + // Mirror internal/client streamLogID: fall back to the request-id header + // when log-id is absent so the diagnostic ID is still populated. + logID := strings.TrimSpace(apiResp.Header.Get(larkcore.HttpHeaderKeyLogId)) + if logID == "" { + logID = strings.TrimSpace(apiResp.Header.Get(larkcore.HttpHeaderKeyRequestId)) + } + if logID != "" { + e = e.WithLogID(logID) + } + return nil, e } fileName := strings.TrimSpace(preferredName) diff --git a/shortcuts/drive/drive_export_download.go b/shortcuts/drive/drive_export_download.go index 62ddd9220..daed6cad2 100644 --- a/shortcuts/drive/drive_export_download.go +++ b/shortcuts/drive/drive_export_download.go @@ -6,7 +6,7 @@ package drive import ( "context" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -30,7 +30,7 @@ var DriveExportDownload = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if err := validate.ResourceName(runtime.Str("file-token"), "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--file-token") } return nil }, diff --git a/shortcuts/drive/drive_export_test.go b/shortcuts/drive/drive_export_test.go index 74f94efdb..619c9d43d 100644 --- a/shortcuts/drive/drive_export_test.go +++ b/shortcuts/drive/drive_export_test.go @@ -13,6 +13,7 @@ import ( "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/httpmock" "github.com/larksuite/cli/internal/output" @@ -360,12 +361,18 @@ func TestDriveExportMarkdownRejectsMissingDocumentObject(t *testing.T) { t.Fatal("expected error for missing document object, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured exit error, got %v", err) + var intErr *errs.InternalError + if !errors.As(err, &intErr) { + t.Fatalf("expected *errs.InternalError, got %T", err) } - if !strings.Contains(exitErr.Detail.Message, "missing document object") { - t.Fatalf("error message = %q, want mention of missing document object", exitErr.Detail.Message) + if intErr.Subtype != errs.SubtypeInvalidResponse { + t.Fatalf("Subtype = %q, want %q", intErr.Subtype, errs.SubtypeInvalidResponse) + } + if !strings.Contains(intErr.Message, "missing document object") { + t.Fatalf("error message = %q, want mention of missing document object", intErr.Message) + } + if got := output.ExitCodeOf(err); got != output.ExitInternal { + t.Fatalf("exit code = %d, want %d", got, output.ExitInternal) } } @@ -396,12 +403,18 @@ func TestDriveExportMarkdownRejectsMissingDocumentContent(t *testing.T) { t.Fatal("expected error for missing document.content, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured exit error, got %v", err) + var intErr *errs.InternalError + if !errors.As(err, &intErr) { + t.Fatalf("expected *errs.InternalError, got %T", err) + } + if intErr.Subtype != errs.SubtypeInvalidResponse { + t.Fatalf("Subtype = %q, want %q", intErr.Subtype, errs.SubtypeInvalidResponse) } - if !strings.Contains(exitErr.Detail.Message, "missing document.content") { - t.Fatalf("error message = %q, want mention of missing document.content", exitErr.Detail.Message) + if !strings.Contains(intErr.Message, "missing document.content") { + t.Fatalf("error message = %q, want mention of missing document.content", intErr.Message) + } + if got := output.ExitCodeOf(err); got != output.ExitInternal { + t.Fatalf("exit code = %d, want %d", got, output.ExitInternal) } } @@ -688,21 +701,25 @@ func TestDriveExportReadyDownloadFailureIncludesRecoveryHint(t *testing.T) { t.Fatal("expected download recovery error, got nil") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured exit error, got %v", err) + // The download itself succeeds; the local "file already exists" failure is a + // validation error. The recovery-hint wrapper must preserve that typed class + // (exit 2) instead of downgrading it to api/server_error (exit 1), per + // ERROR_CONTRACT.md "propagate typed errors unchanged". + var valErr *errs.ValidationError + if !errors.As(err, &valErr) { + t.Fatalf("expected *errs.ValidationError (preserved class), got %T", err) } - if !strings.Contains(exitErr.Detail.Message, "already exists") { - t.Fatalf("message missing overwrite guidance: %q", exitErr.Detail.Message) + if !strings.Contains(valErr.Message, "already exists") { + t.Fatalf("message missing overwrite guidance: %q", valErr.Message) } - if !strings.Contains(exitErr.Detail.Hint, "ticket=tk_ready") { - t.Fatalf("hint missing ticket: %q", exitErr.Detail.Hint) + if !strings.Contains(valErr.Hint, "ticket=tk_ready") { + t.Fatalf("hint missing ticket: %q", valErr.Hint) } - if !strings.Contains(exitErr.Detail.Hint, "file_token=box_ready") { - t.Fatalf("hint missing file token: %q", exitErr.Detail.Hint) + if !strings.Contains(valErr.Hint, "file_token=box_ready") { + t.Fatalf("hint missing file token: %q", valErr.Hint) } - if !strings.Contains(exitErr.Detail.Hint, `lark-cli drive +export-download --file-token "box_ready" --file-name "report.pdf"`) { - t.Fatalf("hint missing recovery command: %q", exitErr.Detail.Hint) + if !strings.Contains(valErr.Hint, `lark-cli drive +export-download --file-token "box_ready" --file-name "report.pdf"`) { + t.Fatalf("hint missing recovery command: %q", valErr.Hint) } } @@ -856,18 +873,26 @@ func TestDriveExportPollErrorsReturnLastErrorWithRecoveryHint(t *testing.T) { t.Fatalf("stdout should stay empty on persistent poll error: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured exit error, got %v", err) + // The poll error is now a typed *errs.APIError (runtime.CallAPITyped). + // The recovery-hint wrapper must preserve that error's class and exit code + // (NOT downgrade it) and only append the recovery hint to the Problem in place. + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected a typed errs.* error, got %T (%v)", err, err) + } + // Lark code 999 is unknown to the classifier, so it maps to CategoryAPI → + // ExitAPI — the wrapper must keep that, not force a different exit code. + if output.ExitCodeOf(err) != output.ExitAPI { + t.Fatalf("exit code = %d, want preserved %d (ExitAPI)", output.ExitCodeOf(err), output.ExitAPI) } - if !strings.Contains(exitErr.Detail.Message, "temporary backend failure") { - t.Fatalf("message missing last poll error: %q", exitErr.Detail.Message) + if !strings.Contains(p.Message, "temporary backend failure") { + t.Fatalf("message missing last poll error: %q", p.Message) } - if !strings.Contains(exitErr.Detail.Hint, "ticket=tk_poll_fail") { - t.Fatalf("hint missing ticket: %q", exitErr.Detail.Hint) + if !strings.Contains(p.Hint, "ticket=tk_poll_fail") { + t.Fatalf("hint missing ticket: %q", p.Hint) } - if !strings.Contains(exitErr.Detail.Hint, "lark-cli drive +task_result --scenario export --ticket tk_poll_fail --file-token docx123") { - t.Fatalf("hint missing recovery command: %q", exitErr.Detail.Hint) + if !strings.Contains(p.Hint, "lark-cli drive +task_result --scenario export --ticket tk_poll_fail --file-token docx123") { + t.Fatalf("hint missing recovery command: %q", p.Hint) } } diff --git a/shortcuts/drive/drive_import.go b/shortcuts/drive/drive_import.go index a9f6e32f9..8c5c9a2c8 100644 --- a/shortcuts/drive/drive_import.go +++ b/shortcuts/drive/drive_import.go @@ -9,8 +9,8 @@ import ( "path/filepath" "strings" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -161,10 +161,10 @@ func preflightDriveImportFile(fio fileio.FileIO, spec *driveImportSpec) (int64, // and format-specific size limits before planning the upload path. info, err := fio.Stat(spec.FilePath) if err != nil { - return 0, common.WrapInputStatError(err) + return 0, driveInputStatError(err) } if !info.Mode().IsRegular() { - return 0, output.ErrValidation("file must be a regular file: %s", spec.FilePath) + return 0, errs.NewValidationError(errs.SubtypeInvalidArgument, "file must be a regular file: %s", spec.FilePath).WithParam("--file") } if err = validateDriveImportFileSize(spec.FilePath, spec.DocType, info.Size()); err != nil { return 0, err diff --git a/shortcuts/drive/drive_import_common.go b/shortcuts/drive/drive_import_common.go index e4abe0b84..525051359 100644 --- a/shortcuts/drive/drive_import_common.go +++ b/shortcuts/drive/drive_import_common.go @@ -11,7 +11,7 @@ import ( "strings" "time" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -95,7 +95,7 @@ func (s driveImportSpec) CreateTaskBody(fileToken string) map[string]interface{} func uploadMediaForImport(ctx context.Context, runtime *common.RuntimeContext, filePath, fileName, docType string) (string, error) { importInfo, err := runtime.FileIO().Stat(filePath) if err != nil { - return "", common.WrapInputStatError(err) + return "", driveInputStatError(err) } fileSize := importInfo.Size() @@ -142,7 +142,7 @@ func buildImportMediaExtra(filePath, docType string) (string, error) { "file_extension": strings.TrimPrefix(strings.ToLower(filepath.Ext(filePath)), "."), }) if err != nil { - return "", output.Errorf(output.ExitInternal, "json_error", "build upload extra failed: %v", err) + return "", errs.NewInternalError(errs.SubtypeUnknown, "build upload extra failed: %v", err).WithCause(err) } return string(extraBytes), nil } @@ -178,20 +178,20 @@ func validateDriveImportFileSize(filePath, docType string, fileSize int64) error ext := strings.TrimPrefix(strings.ToLower(filepath.Ext(filePath)), ".") if ext == "csv" { // CSV is the only source format whose limit depends on the target type. - return output.ErrValidation( + return errs.NewValidationError(errs.SubtypeInvalidArgument, "file %s exceeds %s import limit for .csv when importing as %s", common.FormatSize(fileSize), common.FormatSize(limit), docType, - ) + ).WithParam("--file") } - return output.ErrValidation( + return errs.NewValidationError(errs.SubtypeInvalidArgument, "file %s exceeds %s import limit for .%s", common.FormatSize(fileSize), common.FormatSize(limit), ext, - ) + ).WithParam("--file") } // validateDriveImportSpec enforces the CLI-level compatibility rules before any @@ -199,18 +199,18 @@ func validateDriveImportFileSize(filePath, docType string, fileSize int64) error func validateDriveImportSpec(spec driveImportSpec) error { ext := spec.FileExtension() if ext == "" { - return output.ErrValidation("file must have an extension (e.g. .md, .docx, .xlsx, .pptx)") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "file must have an extension (e.g. .md, .docx, .xlsx, .pptx)").WithParam("--file") } switch spec.DocType { case "docx", "sheet", "bitable", "slides": default: - return output.ErrValidation("unsupported target document type: %s. Supported types are: docx, sheet, bitable, slides", spec.DocType) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported target document type: %s. Supported types are: docx, sheet, bitable, slides", spec.DocType).WithParam("--type") } supportedTypes, ok := driveImportExtToDocTypes[ext] if !ok { - return output.ErrValidation("unsupported file extension: %s. Supported extensions are: docx, doc, txt, md, mark, markdown, html, xlsx, xls, csv, base, pptx", ext) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported file extension: %s. Supported extensions are: docx, doc, txt, md, mark, markdown, html, xlsx, xls, csv, base, pptx", ext).WithParam("--file") } typeAllowed := false @@ -236,21 +236,21 @@ func validateDriveImportSpec(spec driveImportSpec) error { default: hint = fmt.Sprintf(".%s files can only be imported as 'docx', not '%s'", ext, spec.DocType) } - return output.ErrValidation("file type mismatch: %s", hint) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "file type mismatch: %s", hint) } if strings.TrimSpace(spec.FolderToken) != "" { if err := validate.ResourceName(spec.FolderToken, "--folder-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--folder-token") } } if strings.TrimSpace(spec.TargetToken) != "" { if spec.DocType != "bitable" { - return output.ErrValidation("--target-token is only supported when --type is bitable") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-token is only supported when --type is bitable").WithParam("--target-token") } if err := validate.ResourceName(spec.TargetToken, "--target-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--target-token") } } @@ -308,14 +308,14 @@ func driveImportTaskResultCommand(ticket string) string { // createDriveImportTask creates the server-side import task after the media // upload has produced a reusable file token. func createDriveImportTask(runtime *common.RuntimeContext, spec driveImportSpec, fileToken string) (string, error) { - data, err := runtime.CallAPI("POST", "/open-apis/drive/v1/import_tasks", nil, spec.CreateTaskBody(fileToken)) + data, err := runtime.CallAPITyped("POST", "/open-apis/drive/v1/import_tasks", nil, spec.CreateTaskBody(fileToken)) if err != nil { return "", err } ticket := common.GetString(data, "ticket") if ticket == "" { - return "", output.Errorf(output.ExitAPI, "api_error", "no ticket returned from import_tasks") + return "", errs.NewInternalError(errs.SubtypeInvalidResponse, "no ticket returned from import_tasks") } return ticket, nil } @@ -323,10 +323,10 @@ func createDriveImportTask(runtime *common.RuntimeContext, spec driveImportSpec, // getDriveImportStatus fetches the current state of an import task by ticket. func getDriveImportStatus(runtime *common.RuntimeContext, ticket string) (driveImportStatus, error) { if err := validate.ResourceName(ticket, "--ticket"); err != nil { - return driveImportStatus{}, output.ErrValidation("%s", err) + return driveImportStatus{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--ticket") } - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "GET", fmt.Sprintf("/open-apis/drive/v1/import_tasks/%s", validate.EncodePathSegment(ticket)), nil, @@ -391,7 +391,7 @@ func pollDriveImportTask(runtime *common.RuntimeContext, ticket string) (driveIm if msg == "" { msg = status.StatusLabel() } - return status, false, output.Errorf(output.ExitAPI, "api_error", "import failed with status %d: %s", status.JobStatus, msg) + return status, false, errs.NewAPIError(errs.SubtypeServerError, "import failed with status %d: %s", status.JobStatus, msg) } } if !hadSuccessfulPoll && lastErr != nil { diff --git a/shortcuts/drive/drive_inspect.go b/shortcuts/drive/drive_inspect.go index 7941d6b48..e448311cf 100644 --- a/shortcuts/drive/drive_inspect.go +++ b/shortcuts/drive/drive_inspect.go @@ -9,7 +9,7 @@ import ( "io" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -37,18 +37,18 @@ var DriveInspect = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { raw := strings.TrimSpace(runtime.Str("url")) if raw == "" { - return output.ErrValidation("--url cannot be empty") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--url cannot be empty").WithParam("--url") } _, ok := common.ParseResourceURL(raw) if !ok { // Not a recognized URL pattern. if strings.Contains(raw, "://") { - return output.ErrValidation("unsupported --url %q: use a recognized Lark document URL or a bare token with --type", raw) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported --url %q: use a recognized Lark document URL or a bare token with --type", raw).WithParam("--url") } // Bare token: --type is required. if strings.TrimSpace(runtime.Str("type")) == "" { - return output.ErrValidation("--type is required when --url is a bare token (allowed: doc, docx, sheet, bitable, wiki, file, folder, mindnote, slides)") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--type is required when --url is a bare token (allowed: doc, docx, sheet, bitable, wiki, file, folder, mindnote, slides)").WithParam("--type") } } return nil @@ -111,7 +111,7 @@ var DriveInspect = common.Shortcut{ // Step 2: If type is "wiki", unwrap via get_node API. if docType == "wiki" { fmt.Fprintf(runtime.IO().ErrOut, "Inspecting wiki node: %s\n", common.MaskToken(docToken)) - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "GET", "/open-apis/wiki/v2/spaces/get_node", map[string]interface{}{"token": docToken}, @@ -128,7 +128,7 @@ var DriveInspect = common.Shortcut{ nodeToken := common.GetString(node, "node_token") if objType == "" || objToken == "" { - return output.Errorf(output.ExitAPI, "api_error", "wiki get_node returned incomplete node data (obj_type=%q, obj_token=%q)", objType, objToken) + return errs.NewInternalError(errs.SubtypeInvalidResponse, "wiki get_node returned incomplete node data (obj_type=%q, obj_token=%q)", objType, objToken) } wikiNode = map[string]interface{}{ diff --git a/shortcuts/drive/drive_io_test.go b/shortcuts/drive/drive_io_test.go index a1b7807e8..6391cbf09 100644 --- a/shortcuts/drive/drive_io_test.go +++ b/shortcuts/drive/drive_io_test.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "mime" "mime/multipart" "net/http" @@ -17,6 +18,7 @@ import ( "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/httpmock" @@ -1338,9 +1340,20 @@ func TestDriveUploadValidateRejectsConflictingTargets(t *testing.T) { runtime := common.TestNewRuntimeContextWithCtx(context.Background(), cmd, nil) err := DriveUpload.Validate(context.Background(), runtime) - if err == nil || !strings.Contains(err.Error(), "mutually exclusive") { + var verr *errs.ValidationError + if !errors.As(err, &verr) { + t.Fatalf("Validate() error = %T %v, want *errs.ValidationError", err, err) + } + if verr.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("subtype = %q, want %q", verr.Subtype, errs.SubtypeInvalidArgument) + } + if !strings.Contains(verr.Error(), "mutually exclusive") { t.Fatalf("Validate() error = %v, want mutually exclusive error", err) } + // Multi-flag conflict carries no single Param. + if verr.Param != "" { + t.Fatalf("Param = %q, want empty for multi-flag conflict", verr.Param) + } } func TestDriveUploadValidateRejectsExplicitEmptyWikiToken(t *testing.T) { @@ -1361,9 +1374,7 @@ func TestDriveUploadValidateRejectsExplicitEmptyWikiToken(t *testing.T) { runtime := common.TestNewRuntimeContextWithCtx(context.Background(), cmd, nil) err := DriveUpload.Validate(context.Background(), runtime) - if err == nil || !strings.Contains(err.Error(), "--wiki-token cannot be empty") { - t.Fatalf("Validate() error = %v, want empty wiki-token error", err) - } + assertDriveValidationParam(t, err, "--wiki-token", "--wiki-token cannot be empty") } func TestDriveUploadValidateRejectsExplicitEmptyFileToken(t *testing.T) { @@ -1384,9 +1395,7 @@ func TestDriveUploadValidateRejectsExplicitEmptyFileToken(t *testing.T) { runtime := common.TestNewRuntimeContextWithCtx(context.Background(), cmd, nil) err := DriveUpload.Validate(context.Background(), runtime) - if err == nil || !strings.Contains(err.Error(), "--file-token cannot be empty") { - t.Fatalf("Validate() error = %v, want empty file-token error", err) - } + assertDriveValidationParam(t, err, "--file-token", "--file-token cannot be empty") } func TestDriveUploadValidateRejectsExplicitEmptyFolderToken(t *testing.T) { @@ -1407,8 +1416,25 @@ func TestDriveUploadValidateRejectsExplicitEmptyFolderToken(t *testing.T) { runtime := common.TestNewRuntimeContextWithCtx(context.Background(), cmd, nil) err := DriveUpload.Validate(context.Background(), runtime) - if err == nil || !strings.Contains(err.Error(), "--folder-token cannot be empty") { - t.Fatalf("Validate() error = %v, want empty folder-token error", err) + assertDriveValidationParam(t, err, "--folder-token", "--folder-token cannot be empty") +} + +// assertDriveValidationParam asserts err is a typed *errs.ValidationError with +// SubtypeInvalidArgument, the given Param, and a message containing wantMsg. +func assertDriveValidationParam(t *testing.T, err error, wantParam, wantMsg string) { + t.Helper() + var verr *errs.ValidationError + if !errors.As(err, &verr) { + t.Fatalf("error = %T %v, want *errs.ValidationError", err, err) + } + if verr.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("subtype = %q, want %q", verr.Subtype, errs.SubtypeInvalidArgument) + } + if verr.Param != wantParam { + t.Fatalf("Param = %q, want %q", verr.Param, wantParam) + } + if !strings.Contains(verr.Error(), wantMsg) { + t.Fatalf("error = %q, want substring %q", verr.Error(), wantMsg) } } diff --git a/shortcuts/drive/drive_move.go b/shortcuts/drive/drive_move.go index 8eed0bcf3..d61ae1a7b 100644 --- a/shortcuts/drive/drive_move.go +++ b/shortcuts/drive/drive_move.go @@ -8,7 +8,7 @@ import ( "fmt" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -74,14 +74,14 @@ var DriveMove = common.Shortcut{ return err } if rootToken == "" { - return output.Errorf(output.ExitAPI, "api_error", "get root folder token failed, root folder is empty") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "get root folder token failed, root folder is empty") } spec.FolderToken = rootToken } fmt.Fprintf(runtime.IO().ErrOut, "Moving %s %s to folder %s...\n", spec.FileType, common.MaskToken(spec.FileToken), common.MaskToken(spec.FolderToken)) - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "POST", fmt.Sprintf("/open-apis/drive/v1/files/%s/move", validate.EncodePathSegment(spec.FileToken)), nil, @@ -95,7 +95,7 @@ var DriveMove = common.Shortcut{ if spec.FileType == "folder" { taskID := common.GetString(data, "task_id") if taskID == "" { - return output.Errorf(output.ExitAPI, "api_error", "move folder returned no task_id") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "move folder returned no task_id") } fmt.Fprintf(runtime.IO().ErrOut, "Folder move is async, polling task %s...\n", taskID) @@ -139,14 +139,14 @@ var DriveMove = common.Shortcut{ // getRootFolderToken resolves the caller's Drive root folder token so other // commands can safely use it as a default destination. func getRootFolderToken(ctx context.Context, runtime *common.RuntimeContext) (string, error) { - data, err := runtime.CallAPI("GET", "/open-apis/drive/explorer/v2/root_folder/meta", nil, nil) + data, err := runtime.CallAPITyped("GET", "/open-apis/drive/explorer/v2/root_folder/meta", nil, nil) if err != nil { return "", err } token := common.GetString(data, "token") if token == "" { - return "", output.Errorf(output.ExitAPI, "api_error", "root_folder/meta returned no token") + return "", errs.NewInternalError(errs.SubtypeInvalidResponse, "root_folder/meta returned no token") } return token, nil diff --git a/shortcuts/drive/drive_move_common.go b/shortcuts/drive/drive_move_common.go index d200d8cf5..175937e7d 100644 --- a/shortcuts/drive/drive_move_common.go +++ b/shortcuts/drive/drive_move_common.go @@ -8,7 +8,7 @@ import ( "strings" "time" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -47,15 +47,15 @@ func (s driveMoveSpec) RequestBody() map[string]interface{} { func validateDriveMoveSpec(spec driveMoveSpec) error { if err := validate.ResourceName(spec.FileToken, "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--file-token") } if strings.TrimSpace(spec.FolderToken) != "" { if err := validate.ResourceName(spec.FolderToken, "--folder-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--folder-token") } } if !driveMoveAllowedTypes[spec.FileType] { - return output.ErrValidation("unsupported file type: %s. Supported types: file, docx, bitable, doc, sheet, mindnote, folder, slides", spec.FileType) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported file type: %s. Supported types: file, docx, bitable, doc, sheet, mindnote, folder, slides", spec.FileType).WithParam("--type") } return nil } @@ -109,10 +109,10 @@ func driveTaskCheckParams(taskID string) map[string]interface{} { // folder move or delete task. func getDriveTaskCheckStatus(runtime *common.RuntimeContext, taskID string) (driveTaskCheckStatus, error) { if err := validate.ResourceName(taskID, "--task-id"); err != nil { - return driveTaskCheckStatus{}, output.ErrValidation("%s", err) + return driveTaskCheckStatus{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--task-id") } - data, err := runtime.CallAPI("GET", "/open-apis/drive/v1/files/task_check", driveTaskCheckParams(taskID), nil) + data, err := runtime.CallAPITyped("GET", "/open-apis/drive/v1/files/task_check", driveTaskCheckParams(taskID), nil) if err != nil { return driveTaskCheckStatus{}, err } @@ -163,7 +163,7 @@ func pollDriveTaskCheck(runtime *common.RuntimeContext, taskID string) (driveTas return status, true, nil } if status.Failed() { - return status, false, output.Errorf(output.ExitAPI, "api_error", "folder task failed") + return status, false, errs.NewAPIError(errs.SubtypeServerError, "folder task failed") } } diff --git a/shortcuts/drive/drive_pull.go b/shortcuts/drive/drive_pull.go index 04fb4509f..3fa27eda2 100644 --- a/shortcuts/drive/drive_pull.go +++ b/shortcuts/drive/drive_pull.go @@ -15,8 +15,8 @@ import ( larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -88,26 +88,26 @@ var DrivePull = common.Shortcut{ localDir := strings.TrimSpace(runtime.Str("local-dir")) folderToken := strings.TrimSpace(runtime.Str("folder-token")) if localDir == "" { - return common.FlagErrorf("--local-dir is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--local-dir is required").WithParam("--local-dir") } if folderToken == "" { - return common.FlagErrorf("--folder-token is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--folder-token is required").WithParam("--folder-token") } if err := validate.ResourceName(folderToken, "--folder-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--folder-token") } if _, err := validate.SafeLocalFlagPath("--local-dir", localDir); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--local-dir") } info, err := runtime.FileIO().Stat(localDir) if err != nil { - return common.WrapInputStatError(err) + return driveInputStatError(err) } if !info.IsDir() { - return output.ErrValidation("--local-dir is not a directory: %s", localDir) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--local-dir is not a directory: %s", localDir).WithParam("--local-dir") } if runtime.Bool("delete-local") && !runtime.Bool("yes") { - return output.ErrValidation("--delete-local requires --yes (high-risk: deletes local files absent from Drive)") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--delete-local requires --yes (high-risk: deletes local files absent from Drive)").WithParam("--yes") } return nil }, @@ -143,18 +143,18 @@ var DrivePull = common.Shortcut{ // remove the wrong files outside cwd. safeRoot, err := validate.SafeInputPath(localDir) if err != nil { - return output.ErrValidation("--local-dir: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--local-dir: %s", err).WithParam("--local-dir") } cwdCanonical, err := validate.SafeInputPath(".") if err != nil { - return output.ErrValidation("could not resolve cwd: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "could not resolve cwd: %s", err) } // rootRelToCwd is the localDir form FileIO.Save accepts (it // rejects absolute paths). For cwd itself it becomes ".", which // joins cleanly with the rel_paths returned by the lister. rootRelToCwd, err := filepath.Rel(cwdCanonical, safeRoot) if err != nil { - return output.ErrValidation("--local-dir resolves outside cwd: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--local-dir resolves outside cwd: %s", err).WithParam("--local-dir") } fmt.Fprintf(runtime.IO().ErrOut, "Listing Drive folder: %s\n", common.MaskToken(folderToken)) @@ -174,7 +174,7 @@ var DrivePull = common.Shortcut{ // treated as orphaned. remoteFiles, remotePaths, err := drivePullRemoteViews(entries, duplicateRemote) if err != nil { - return output.Errorf(output.ExitInternal, "internal", "%s", err) + return errs.WrapInternal(err) } var downloaded, skipped, failed, deletedLocal int @@ -293,26 +293,25 @@ var DrivePull = common.Shortcut{ // Item-level failures (download error, dir/file conflict, delete // error) must surface as a non-zero exit so AI / script callers // don't have to reach into summary.failed to detect a partial - // sync. The same structured payload rides along in error.detail - // so forensics aren't lost. When --delete-local was skipped - // because of an earlier download failure, callers see - // deleted_local=0 plus the download failure that aborted it, - // which is what makes the partial state self-explanatory. + // sync. On any failure the structured payload (summary + items + + // a "note" carrying the human guidance) is written to stdout as an + // ok:false result via OutPartialFailure, which also sets the exit + // code, so the per-item context is never lost. When --delete-local + // was skipped because + // of an earlier download failure, callers see deleted_local=0 + // plus the download failure that aborted it, which is what makes + // the partial state self-explanatory. if failed > 0 { - msg := fmt.Sprintf("%d item(s) failed during +pull; partial sync — re-run after resolving the failures", failed) + note := fmt.Sprintf("%d item(s) failed during +pull; partial sync — re-run after resolving the failures", failed) if deleteLocal && downloadFailed > 0 { - msg += " (--delete-local was skipped because the download pass had failures)" - } - return &output.ExitError{ - Code: output.ExitAPI, - Detail: &output.ErrDetail{ - Type: "partial_failure", - Message: msg, - Detail: payload, - }, + note += " (--delete-local was skipped because the download pass had failures)" } + payload["note"] = note } + if failed > 0 { + return runtime.OutPartialFailure(payload, nil) + } runtime.Out(payload, nil) return nil }, @@ -326,14 +325,14 @@ func drivePullDownload(ctx context.Context, runtime *common.RuntimeContext, file ApiPath: fmt.Sprintf("/open-apis/drive/v1/files/%s/download", validate.EncodePathSegment(fileToken)), }) if err != nil { - return output.ErrNetwork("download %s: %s", common.MaskToken(fileToken), err) + return wrapDriveNetworkErr(err, "download %s: %s", common.MaskToken(fileToken), err) } defer resp.Body.Close() if _, err := runtime.FileIO().Save(target, fileio.SaveOptions{ ContentType: resp.Header.Get("Content-Type"), ContentLength: resp.ContentLength, }, resp.Body); err != nil { - return common.WrapSaveErrorByCategory(err, "io") + return driveSaveError(err) } if err := drivePullApplyRemoteModifiedTime(target, remoteModifiedTime, runtime); err != nil { fmt.Fprintf(runtime.IO().ErrOut, "Downloaded %s but could not preserve remote modified_time: %s\n", target, err) @@ -350,10 +349,10 @@ func drivePullApplyRemoteModifiedTime(target, remoteModifiedTime string, runtime } resolved, err := runtime.FileIO().ResolvePath(target) if err != nil { - return output.ErrValidation("unsafe output path: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", err) } if err := drivePullChtimes(resolved, remoteTime, remoteTime); err != nil { - return output.Errorf(output.ExitInternal, "io", "cannot preserve remote modified_time on local file: %s", err) + return errs.NewInternalError(errs.SubtypeFileIO, "cannot preserve remote modified_time on local file: %s", err).WithCause(err) } return nil } @@ -437,7 +436,7 @@ func drivePullRemoteViews(entries []driveRemoteEntry, duplicateRemote string) (m remoteFiles[rel] = drivePullTarget{DownloadToken: chosen.FileToken, ItemFileToken: chosen.FileToken, ModifiedTime: chosen.ModifiedTime} remotePaths[rel] = struct{}{} default: - return nil, nil, fmt.Errorf("unsupported duplicate remote strategy %q", duplicateRemote) + return nil, nil, errs.NewInternalError(errs.SubtypeUnknown, "unsupported duplicate remote strategy %q", duplicateRemote) } } return remoteFiles, remotePaths, nil @@ -467,7 +466,7 @@ func drivePullWalkLocal(root string) ([]string, error) { return nil }) if err != nil { - return nil, output.Errorf(output.ExitInternal, "io", "walk %s: %s", root, err) + return nil, errs.NewInternalError(errs.SubtypeFileIO, "walk %s: %s", root, err).WithCause(err) } return paths, nil } diff --git a/shortcuts/drive/drive_pull_test.go b/shortcuts/drive/drive_pull_test.go index c47018481..5a6d6caa3 100644 --- a/shortcuts/drive/drive_pull_test.go +++ b/shortcuts/drive/drive_pull_test.go @@ -478,9 +478,9 @@ func TestDrivePullSkipsWhenSmartIgnoresRemoteSize(t *testing.T) { // already a directory locally. SafeOutputPath would refuse to overwrite // the directory at write time, but if --if-exists=skip silently swallows // the collision the caller sees "skipped" and assumes the mirror is -// in sync. The fix surfaces it as a structured `partial_failure` -// ExitError (non-zero exit + items[] in error.detail) under both skip -// and overwrite policies so callers can react via exit code. +// in sync. The fix surfaces it as a partial-failure (ok:false items[] payload +// on stdout + non-zero exit) under both skip and overwrite policies so callers +// can react via exit code. func TestDrivePullSurfacesDirectoryFileMirrorConflict(t *testing.T) { for _, policy := range []string{"overwrite", "skip"} { t.Run(policy, func(t *testing.T) { @@ -515,8 +515,8 @@ func TestDrivePullSurfacesDirectoryFileMirrorConflict(t *testing.T) { "--if-exists", policy, "--as", "bot", }, f, stdout) - detail := assertDrivePullPartialFailure(t, err) - summary, items := splitDrivePullDetail(t, detail) + assertDrivePullPartialFailure(t, err) + summary, items := splitDrivePullStdout(t, stdout.Bytes()) if got := summary["failed"]; got != float64(1) { t.Errorf("[%s] summary.failed = %v, want 1", policy, got) } @@ -529,9 +529,6 @@ func TestDrivePullSurfacesDirectoryFileMirrorConflict(t *testing.T) { if msg, _ := items[0]["error"].(string); !strings.Contains(msg, "is a directory") { t.Errorf("[%s] error message should mention the directory conflict, got: %q", policy, msg) } - if stdout.Len() != 0 { - t.Errorf("[%s] stdout should be empty on partial_failure, got: %s", policy, stdout.String()) - } }) } } @@ -900,8 +897,8 @@ func TestDrivePullDeleteLocalPreservesLocalFileShadowedByRemoteFolder(t *testing // TestDrivePullDeleteLocalCountsFailureInSummary pins the contract that // a failed delete shows up in summary.failed (not just in items[]) AND -// surfaces as a partial_failure ExitError so callers can detect the -// half-synced state via exit code. Before the fix, the delete_failed +// surfaces as a non-zero exit (partial-failure signal) so callers can detect +// the half-synced state via exit code. Before the fix, the delete_failed // branches appended an item but left `failed` at zero AND returned nil, // so the JSON envelope reported `ok=true`+`exit=0` even when the mirror // was incomplete. Setup forces os.Remove to fail by making the file's @@ -947,8 +944,8 @@ func TestDrivePullDeleteLocalCountsFailureInSummary(t *testing.T) { "--yes", "--as", "bot", }, f, stdout) - detail := assertDrivePullPartialFailure(t, err) - summary, items := splitDrivePullDetail(t, detail) + assertDrivePullPartialFailure(t, err) + summary, items := splitDrivePullStdout(t, stdout.Bytes()) if got := summary["failed"]; got != float64(1) { t.Errorf("summary.failed = %v, want 1 (delete_failed must increment failed)", got) } @@ -958,15 +955,12 @@ func TestDrivePullDeleteLocalCountsFailureInSummary(t *testing.T) { if len(items) != 1 || items[0]["action"] != "delete_failed" { t.Errorf("expected one items[] entry with action=delete_failed, got: %#v", items) } - if stdout.Len() != 0 { - t.Errorf("stdout should be empty on partial_failure, got: %s", stdout.String()) - } } // TestDrivePullDownloadFailureSkipsDeleteLocalAndExitsNonZero pins the // gating contract for --delete-local: when the download pass produced // any failure, the delete walk MUST be skipped entirely and the command -// MUST exit non-zero with type=partial_failure. The half-synced state +// MUST exit non-zero via the partial-failure signal. The half-synced state // where some Drive files are missing locally AND some local-only files // have been removed is never observable. func TestDrivePullDownloadFailureSkipsDeleteLocalAndExitsNonZero(t *testing.T) { @@ -1014,12 +1008,12 @@ func TestDrivePullDownloadFailureSkipsDeleteLocalAndExitsNonZero(t *testing.T) { "--yes", "--as", "bot", }, f, stdout) - exitErr := assertDrivePullPartialFailure(t, err) - if !strings.Contains(exitErr.Detail.Message, "--delete-local was skipped") { - t.Errorf("expected message to mention --delete-local skip, got: %q", exitErr.Detail.Message) + assertDrivePullPartialFailure(t, err) + if note := drivePullStdoutNote(t, stdout.Bytes()); !strings.Contains(note, "--delete-local was skipped") { + t.Errorf("expected note to mention --delete-local skip, got: %q", note) } - summary, items := splitDrivePullDetail(t, exitErr) + summary, items := splitDrivePullStdout(t, stdout.Bytes()) if got := summary["failed"]; got != float64(1) { t.Errorf("summary.failed = %v, want 1", got) } @@ -1036,9 +1030,6 @@ func TestDrivePullDownloadFailureSkipsDeleteLocalAndExitsNonZero(t *testing.T) { if _, statErr := os.Stat(stale); statErr != nil { t.Fatalf("stale.txt must survive when --delete-local is skipped after a download failure; stat err=%v", statErr) } - if stdout.Len() != 0 { - t.Errorf("stdout should be empty on partial_failure, got: %s", stdout.String()) - } } // TestDrivePullDeleteLocalDoesNotEscapeViaSymlinkParentRef is the @@ -1343,49 +1334,60 @@ func mustReadFile(t *testing.T, path, want string) { } } -// assertDrivePullPartialFailure asserts that err is the structured -// partial_failure ExitError +pull returns when any item-level failure -// happens, and returns the unwrapped *ExitError so the caller can drill -// into Detail.Detail without re-doing the type assertion. -func assertDrivePullPartialFailure(t *testing.T, err error) *output.ExitError { +// assertDrivePullPartialFailure asserts that err is the typed partial-failure +// exit signal +pull returns on any item-level failure. The structured +// {summary, items, note} payload rides on stdout as an ok:false envelope via +// runtime.OutPartialFailure (in alignment with +push/+sync), so this helper +// only checks the exit-code signal; callers read the payload from stdout via +// splitDrivePullStdout. +func assertDrivePullPartialFailure(t *testing.T, err error) { t.Helper() if err == nil { - t.Fatal("expected partial_failure ExitError, got nil") - } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("expected *output.ExitError, got %T: %v", err, err) - } - if exitErr.Code != output.ExitAPI { - t.Errorf("exit code = %d, want %d (ExitAPI)", exitErr.Code, output.ExitAPI) + t.Fatal("expected partial-failure exit signal, got nil") } - if exitErr.Detail == nil { - t.Fatalf("ExitError.Detail must be set on partial_failure") + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("expected *output.PartialFailureError, got %T: %v", err, err) } - if exitErr.Detail.Type != "partial_failure" { - t.Errorf("error.type = %q, want partial_failure", exitErr.Detail.Type) + if pfErr.Code != output.ExitAPI { + t.Errorf("exit code = %d, want %d (ExitAPI)", pfErr.Code, output.ExitAPI) } - return exitErr } -// splitDrivePullDetail extracts the {summary, items[]} payload from the -// ExitError detail. We round-trip through JSON so test assertions don't -// depend on the concrete map types the production code happens to use. -func splitDrivePullDetail(t *testing.T, exitErr *output.ExitError) (map[string]interface{}, []map[string]interface{}) { +// splitDrivePullStdout extracts the {summary, items[]} payload from the +// stdout envelope written by runtime.Out. We round-trip through JSON so test +// assertions don't depend on the concrete map types the production code +// happens to use. +func splitDrivePullStdout(t *testing.T, stdout []byte) (map[string]interface{}, []map[string]interface{}) { t.Helper() - raw, err := json.Marshal(exitErr.Detail.Detail) - if err != nil { - t.Fatalf("marshal detail: %v", err) + var envelope struct { + Data struct { + Summary map[string]interface{} `json:"summary"` + Items []map[string]interface{} `json:"items"` + } `json:"data"` + } + if err := json.Unmarshal(stdout, &envelope); err != nil { + t.Fatalf("unmarshal stdout: %v\nraw=%s", err, string(stdout)) } - var got struct { - Summary map[string]interface{} `json:"summary"` - Items []map[string]interface{} `json:"items"` + if envelope.Data.Summary == nil { + t.Fatalf("stdout missing data.summary; raw=%s", string(stdout)) } - if err := json.Unmarshal(raw, &got); err != nil { - t.Fatalf("unmarshal detail: %v\nraw=%s", err, string(raw)) + return envelope.Data.Summary, envelope.Data.Items +} + +// drivePullStdoutNote extracts the partial-failure "note" guidance from the +// stdout envelope. The human-readable note that used to live in the +// partial_failure ExitError message now rides on stdout alongside the +// summary + items payload. +func drivePullStdoutNote(t *testing.T, stdout []byte) string { + t.Helper() + var envelope struct { + Data struct { + Note string `json:"note"` + } `json:"data"` } - if got.Summary == nil { - t.Fatalf("error.detail missing summary; raw=%s", string(raw)) + if err := json.Unmarshal(stdout, &envelope); err != nil { + t.Fatalf("unmarshal stdout: %v\nraw=%s", err, string(stdout)) } - return got.Summary, got.Items + return envelope.Data.Note } diff --git a/shortcuts/drive/drive_push.go b/shortcuts/drive/drive_push.go index 78c34e67c..63bbf7c3d 100644 --- a/shortcuts/drive/drive_push.go +++ b/shortcuts/drive/drive_push.go @@ -5,7 +5,6 @@ package drive import ( "context" - "encoding/json" "errors" "fmt" "io" @@ -19,6 +18,7 @@ import ( larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" @@ -112,26 +112,26 @@ var DrivePush = common.Shortcut{ localDir := strings.TrimSpace(runtime.Str("local-dir")) folderToken := strings.TrimSpace(runtime.Str("folder-token")) if localDir == "" { - return common.FlagErrorf("--local-dir is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--local-dir is required").WithParam("--local-dir") } if folderToken == "" { - return common.FlagErrorf("--folder-token is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--folder-token is required").WithParam("--folder-token") } if err := validate.ResourceName(folderToken, "--folder-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--folder-token") } if _, err := validate.SafeLocalFlagPath("--local-dir", localDir); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--local-dir") } info, err := runtime.FileIO().Stat(localDir) if err != nil { - return common.WrapInputStatError(err) + return driveInputStatError(err) } if !info.IsDir() { - return output.ErrValidation("--local-dir is not a directory: %s", localDir) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--local-dir is not a directory: %s", localDir).WithParam("--local-dir") } if runtime.Bool("delete-remote") && !runtime.Bool("yes") { - return output.ErrValidation("--delete-remote requires --yes (high-risk: deletes Drive files absent locally)") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--delete-remote requires --yes (high-risk: deletes Drive files absent locally)").WithParam("--yes") } // Conditional scope pre-check: when --delete-remote --yes is set, the // run will issue DELETE /open-apis/drive/v1/files/ after the @@ -185,11 +185,11 @@ var DrivePush = common.Shortcut{ // FileIO.Open's SafeInputPath check still accepts. safeRoot, err := validate.SafeInputPath(localDir) if err != nil { - return output.ErrValidation("--local-dir: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--local-dir: %s", err).WithParam("--local-dir") } cwdCanonical, err := validate.SafeInputPath(".") if err != nil { - return output.ErrValidation("could not resolve cwd: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "could not resolve cwd: %s", err) } fmt.Fprintf(runtime.IO().ErrOut, "Walking local: %s\n", localDir) @@ -217,7 +217,7 @@ var DrivePush = common.Shortcut{ // reruns. remoteFiles, remoteFolders, remoteFileGroups, err := drivePushRemoteViews(entries, duplicateRemote) if err != nil { - return output.Errorf(output.ExitInternal, "internal", "%s", err) + return errs.WrapInternal(err) } var uploaded, skipped, failed, deletedRemote int @@ -374,7 +374,7 @@ var DrivePush = common.Shortcut{ } } - runtime.Out(map[string]interface{}{ + payload := map[string]interface{}{ "summary": map[string]interface{}{ "uploaded": uploaded, "skipped": skipped, @@ -382,15 +382,15 @@ var DrivePush = common.Shortcut{ "deleted_remote": deletedRemote, }, "items": items, - }, nil) - // Bump the exit code on any item-level failure (upload, overwrite, - // folder, or delete) so callers / scripts / agents can react. The - // summary + items[] envelope was just written to stdout via Out(), - // so ErrBare here only affects the exit code — the structured - // per-item context is still in the stdout JSON. + } + // On any item-level failure (upload, overwrite, folder, or delete) the + // command reports a partial failure: the summary + per-item items[] stay + // machine-readable on stdout (ok:false) and the process exits non-zero, + // so callers / scripts / agents can react. if failed > 0 { - return output.ErrBare(output.ExitAPI) + return runtime.OutPartialFailure(payload, nil) } + runtime.Out(payload, nil) return nil }, } @@ -466,7 +466,7 @@ func drivePushWalkLocal(root, cwdCanonical string) (map[string]drivePushLocalFil return nil }) if err != nil { - return nil, nil, output.Errorf(output.ExitInternal, "io", "walk %s: %s", root, err) + return nil, nil, errs.NewInternalError(errs.SubtypeFileIO, "walk %s: %s", root, err).WithCause(err) } dirs := make([]string, 0, len(dirsSet)) for d := range dirsSet { @@ -543,7 +543,7 @@ func drivePushRemoteViews(entries []driveRemoteEntry, duplicateRemote string) (m } remoteFiles[rel] = chosen default: - return nil, nil, nil, fmt.Errorf("unsupported duplicate remote strategy %q", duplicateRemote) + return nil, nil, nil, errs.NewInternalError(errs.SubtypeUnknown, "unsupported duplicate remote strategy %q", duplicateRemote) } } return remoteFiles, remoteFolders, fileGroups, nil @@ -567,7 +567,7 @@ func drivePushEnsureFolder(ctx context.Context, runtime *common.RuntimeContext, return "", err } - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "POST", "/open-apis/drive/v1/files/create_folder", nil, @@ -581,7 +581,7 @@ func drivePushEnsureFolder(ctx context.Context, runtime *common.RuntimeContext, } token := common.GetString(data, "token") if token == "" { - return "", output.Errorf(output.ExitAPI, "api_error", "create_folder for %q returned no folder token", relDir) + return "", errs.NewInternalError(errs.SubtypeInvalidResponse, "create_folder for %q returned no folder token", relDir) } folderCache[relDir] = token return token, nil @@ -617,7 +617,7 @@ func drivePushUploadFile(ctx context.Context, runtime *common.RuntimeContext, fi func drivePushUploadAll(_ context.Context, runtime *common.RuntimeContext, file drivePushLocalFile, existingToken, parentToken string) (string, string, error) { f, err := runtime.FileIO().Open(file.OpenPath) if err != nil { - return "", "", common.WrapInputStatError(err) + return "", "", driveInputStatError(err) } defer f.Close() @@ -644,27 +644,22 @@ func drivePushUploadAll(_ context.Context, runtime *common.RuntimeContext, file if errors.As(err, &exitErr) { return "", "", err } - return "", "", output.ErrNetwork("upload failed: %v", err) + return "", "", wrapDriveNetworkErr(err, "upload failed: %v", err) } - var result map[string]interface{} - if err := json.Unmarshal(apiResp.RawBody, &result); err != nil { - return "", "", output.Errorf(output.ExitAPI, "api_error", "upload failed: invalid response JSON: %v", err) - } - // Extract the token before the larkCode check: the backend can produce - // a partial-success response (code != 0 alongside a non-empty - // data.file_token) where bytes have already landed under that token. - // Returning "" here would force the caller to fall back to + // ClassifyAPIResponse returns the data even on a non-zero code, so the + // token is available on a partial-success response (code != 0 alongside a + // non-empty data.file_token) where bytes have already landed under that + // token. Returning "" would force the caller to fall back to // entry.FileToken and silently lose the token Drive actually used, // defeating the overwrite-error token-stability handling in Execute. - data, _ := result["data"].(map[string]interface{}) + data, err := runtime.ClassifyAPIResponse(apiResp) token := common.GetString(data, "file_token") - if larkCode := int(common.GetFloat(result, "code")); larkCode != 0 { - msg, _ := result["msg"].(string) - return token, "", output.ErrAPI(larkCode, fmt.Sprintf("upload failed: [%d] %s", larkCode, msg), result["error"]) + if err != nil { + return token, "", err } if token == "" { - return "", "", output.Errorf(output.ExitAPI, "api_error", "upload failed: no file_token returned") + return "", "", errs.NewInternalError(errs.SubtypeInvalidResponse, "upload failed: no file_token returned") } version := common.GetString(data, "version") if version == "" { @@ -677,7 +672,7 @@ func drivePushUploadAll(_ context.Context, runtime *common.RuntimeContext, file // deployed backend hasn't shipped the field yet we surface the gap // rather than report a phantom success — callers can downgrade to // --if-exists=skip in the meantime. - return token, "", output.Errorf(output.ExitAPI, "api_error", "overwrite for %q succeeded but no version was returned by upload_all", file.RelPath) + return token, "", errs.NewInternalError(errs.SubtypeInvalidResponse, "overwrite for %q succeeded but no version was returned by upload_all", file.RelPath) } return token, version, nil } @@ -692,7 +687,7 @@ func drivePushUploadMultipart(_ context.Context, runtime *common.RuntimeContext, if existingToken != "" { prepareBody["file_token"] = existingToken } - prepareResult, err := runtime.CallAPI("POST", "/open-apis/drive/v1/files/upload_prepare", nil, prepareBody) + prepareResult, err := runtime.CallAPITyped("POST", "/open-apis/drive/v1/files/upload_prepare", nil, prepareBody) if err != nil { return "", err } @@ -701,7 +696,7 @@ func drivePushUploadMultipart(_ context.Context, runtime *common.RuntimeContext, blockSize := int64(common.GetFloat(prepareResult, "block_size")) blockNum := int(common.GetFloat(prepareResult, "block_num")) if uploadID == "" || blockSize <= 0 || blockNum <= 0 { - return "", output.Errorf(output.ExitAPI, "api_error", + return "", errs.NewInternalError(errs.SubtypeInvalidResponse, "upload_prepare returned invalid data: upload_id=%q, block_size=%d, block_num=%d", uploadID, blockSize, blockNum) } @@ -717,7 +712,7 @@ func drivePushUploadMultipart(_ context.Context, runtime *common.RuntimeContext, // one Open + Close + path-validation per block). partFile, err := runtime.FileIO().Open(file.OpenPath) if err != nil { - return "", common.WrapInputStatError(err) + return "", driveInputStatError(err) } defer partFile.Close() @@ -744,21 +739,16 @@ func drivePushUploadMultipart(_ context.Context, runtime *common.RuntimeContext, if errors.As(doErr, &exitErr) { return "", doErr } - return "", output.ErrNetwork("upload part %d/%d failed: %v", seq+1, blockNum, doErr) + return "", wrapDriveNetworkErr(doErr, "upload part %d/%d failed: %v", seq+1, blockNum, doErr) } - var partResult map[string]interface{} - if err := json.Unmarshal(apiResp.RawBody, &partResult); err != nil { - return "", output.Errorf(output.ExitAPI, "api_error", "upload part %d/%d: invalid response JSON: %v", seq+1, blockNum, err) - } - if larkCode := int(common.GetFloat(partResult, "code")); larkCode != 0 { - msg, _ := partResult["msg"].(string) - return "", output.ErrAPI(larkCode, fmt.Sprintf("upload part %d/%d failed: [%d] %s", seq+1, blockNum, larkCode, msg), partResult["error"]) + if _, err := runtime.ClassifyAPIResponse(apiResp); err != nil { + return "", err } fmt.Fprintf(runtime.IO().ErrOut, " Block %d/%d uploaded (%s)\n", seq+1, blockNum, common.FormatSize(partSize)) } - finishResult, err := runtime.CallAPI("POST", "/open-apis/drive/v1/files/upload_finish", nil, map[string]interface{}{ + finishResult, err := runtime.CallAPITyped("POST", "/open-apis/drive/v1/files/upload_finish", nil, map[string]interface{}{ "upload_id": uploadID, "block_num": blockNum, }) @@ -767,7 +757,7 @@ func drivePushUploadMultipart(_ context.Context, runtime *common.RuntimeContext, } token := common.GetString(finishResult, "file_token") if token == "" { - return "", output.Errorf(output.ExitAPI, "api_error", "upload_finish succeeded but no file_token returned") + return "", errs.NewInternalError(errs.SubtypeInvalidResponse, "upload_finish succeeded but no file_token returned") } return token, nil } @@ -776,7 +766,7 @@ func drivePushUploadMultipart(_ context.Context, runtime *common.RuntimeContext, // never reached here because --delete-remote only iterates the type=file // subset of the remote listing. func drivePushDeleteFile(_ context.Context, runtime *common.RuntimeContext, fileToken string) error { - _, err := runtime.CallAPI( + _, err := runtime.CallAPITyped( "DELETE", fmt.Sprintf("/open-apis/drive/v1/files/%s", validate.EncodePathSegment(fileToken)), map[string]interface{}{"type": driveTypeFile}, diff --git a/shortcuts/drive/drive_push_test.go b/shortcuts/drive/drive_push_test.go index 3d5654ca2..fff10eebe 100644 --- a/shortcuts/drive/drive_push_test.go +++ b/shortcuts/drive/drive_push_test.go @@ -871,21 +871,19 @@ func TestDrivePushOverwriteWithoutVersionFails(t *testing.T) { "--if-exists", "overwrite", "--as", "bot", }, f, stdout) - // Item-level failures bump the exit code via output.ErrBare(ExitAPI), - // preserving the structured items[] envelope on stdout. Older behavior - // was to silently return nil; the assertion below pins the new contract. + // Item-level failures report a partial failure: an ok:false items[] + // envelope on stdout + a non-zero exit via the partial-failure signal. + // Older behavior was to silently return nil; the assertion below pins + // the new contract. if err == nil { t.Fatalf("expected non-zero exit on item-level failure, got nil\nstdout: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("expected *output.ExitError, got %T: %v", err, err) + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("expected *output.PartialFailureError, got %T: %v", err, err) } - if exitErr.Code != output.ExitAPI { - t.Errorf("expected ExitAPI (%d), got code=%d", output.ExitAPI, exitErr.Code) - } - if exitErr.Detail != nil { - t.Errorf("ErrBare should carry no Detail (the items[] envelope already covered the per-item error), got: %#v", exitErr.Detail) + if pfErr.Code != output.ExitAPI { + t.Errorf("expected ExitAPI (%d), got code=%d", output.ExitAPI, pfErr.Code) } out := stdout.String() @@ -959,12 +957,19 @@ func TestDrivePushOverwritePartialSuccessSurfacesReturnedToken(t *testing.T) { if err == nil { t.Fatalf("expected non-zero exit on item-level failure, got nil\nstdout: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Code != output.ExitAPI { - t.Fatalf("expected ExitAPI from output.ExitError, got %T %v", err, err) + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) || pfErr.Code != output.ExitAPI { + t.Fatalf("expected ExitAPI from *output.PartialFailureError, got %T %v", err, err) } out := stdout.String() + // Partial failure reports an ok:false result envelope on stdout (not a + // misleading ok:true) while still carrying BOTH the succeeded and failed + // items — consistent with the pre-change payload. The failed side is + // asserted via "failed": 1 and the succeeded side via tok_keep_partial. + if !strings.Contains(out, `"ok": false`) { + t.Errorf("partial failure must emit an ok:false result envelope, got: %s", out) + } if !strings.Contains(out, `"failed": 1`) { t.Errorf("expected failed=1, got: %s", out) } @@ -1042,9 +1047,9 @@ func TestDrivePushSkipsDeleteAfterUploadFailure(t *testing.T) { if err == nil { t.Fatalf("expected non-zero exit on overwrite failure, got nil\nstdout: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Code != output.ExitAPI { - t.Fatalf("expected ExitAPI ExitError, got %v", err) + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) || pfErr.Code != output.ExitAPI { + t.Fatalf("expected ExitAPI *output.PartialFailureError, got %v", err) } out := stdout.String() @@ -1065,7 +1070,7 @@ func TestDrivePushSkipsDeleteAfterUploadFailure(t *testing.T) { // TestDrivePushExitsZeroOnCleanRun pins the inverse: a successful run // with no failures must NOT bump the exit code. Without this the -// ErrBare-on-failure path could regress to "always non-zero" silently. +// partial-failure path could regress to "always non-zero" silently. func TestDrivePushExitsZeroOnCleanRun(t *testing.T) { f, stdout, _, reg := cmdutil.TestFactory(t, driveTestConfig()) diff --git a/shortcuts/drive/drive_search.go b/shortcuts/drive/drive_search.go index e1b618fe6..a17872512 100644 --- a/shortcuts/drive/drive_search.go +++ b/shortcuts/drive/drive_search.go @@ -6,7 +6,6 @@ package drive import ( "context" "encoding/json" - "errors" "fmt" "io" "math" @@ -15,6 +14,7 @@ import ( "strings" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -219,13 +219,13 @@ func readDriveSearchSpec(runtime *common.RuntimeContext) driveSearchSpec { // that depends on the combination of flag values. func buildDriveSearchRequest(spec driveSearchSpec, userOpenID string, now time.Time) (map[string]interface{}, []string, error) { if spec.Mine && len(spec.CreatorIDs) > 0 { - return nil, nil, output.ErrValidation("cannot combine --mine and --creator-ids") + return nil, nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot combine --mine and --creator-ids") } if len(spec.FolderTokens) > 0 && len(spec.SpaceIDs) > 0 { - return nil, nil, output.ErrValidation("cannot combine --folder-tokens and --space-ids; doc and wiki scoped search cannot be combined") + return nil, nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot combine --folder-tokens and --space-ids; doc and wiki scoped search cannot be combined") } if spec.Mine && userOpenID == "" { - return nil, nil, output.ErrValidation("--mine requires a logged-in user open_id, but none is configured; run `lark-cli auth login` or set user open_id in config") + return nil, nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--mine requires a logged-in user open_id, but none is configured; run `lark-cli auth login` or set user open_id in config").WithParam("--mine") } if err := validateDocTypes(spec.DocTypes); err != nil { @@ -337,7 +337,7 @@ func parseDriveSearchPageSize(raw string) (int, error) { } n, err := strconv.Atoi(raw) if err != nil { - return 0, output.ErrValidation("--page-size must be a number, got %q", raw) + return 0, errs.NewValidationError(errs.SubtypeInvalidArgument, "--page-size must be a number, got %q", raw).WithParam("--page-size") } if n <= 0 { return 15, nil @@ -355,23 +355,23 @@ func parseDriveSearchPageSize(raw string) (int, error) { func validateDriveSearchIDs(spec driveSearchSpec) error { for _, id := range spec.CreatorIDs { if _, err := common.ValidateUserID(id); err != nil { - return output.ErrValidation("--creator-ids %q: %s", id, err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--creator-ids %q: %s", id, err).WithParam("--creator-ids") } } if n := len(spec.ChatIDs); n > driveSearchMaxChatIDs { - return output.ErrValidation("--chat-ids: max %d values per request, got %d", driveSearchMaxChatIDs, n) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--chat-ids: max %d values per request, got %d", driveSearchMaxChatIDs, n).WithParam("--chat-ids") } for _, id := range spec.ChatIDs { if _, err := common.ValidateChatID(id); err != nil { - return output.ErrValidation("--chat-ids %q: %s", id, err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--chat-ids %q: %s", id, err).WithParam("--chat-ids") } } if n := len(spec.SharerIDs); n > driveSearchMaxSharerIDs { - return output.ErrValidation("--sharer-ids: max %d values per request, got %d", driveSearchMaxSharerIDs, n) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--sharer-ids: max %d values per request, got %d", driveSearchMaxSharerIDs, n).WithParam("--sharer-ids") } for _, id := range spec.SharerIDs { if _, err := common.ValidateUserID(id); err != nil { - return output.ErrValidation("--sharer-ids %q: %s", id, err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--sharer-ids %q: %s", id, err).WithParam("--sharer-ids") } } return nil @@ -382,7 +382,7 @@ func validateDocTypes(values []string) error { // values are already upper-cased by readDriveSearchSpec; compare as-is // so the filter we emit to the server matches what we validated. if _, ok := driveSearchDocTypeSet[v]; !ok { - return output.ErrValidation("--doc-types contains unknown value %q (allowed: doc,sheet,bitable,mindnote,file,wiki,docx,folder,catalog,slides,shortcut)", v) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--doc-types contains unknown value %q (allowed: doc,sheet,bitable,mindnote,file,wiki,docx,folder,catalog,slides,shortcut)", v).WithParam("--doc-types") } } return nil @@ -417,13 +417,13 @@ func clampOpenedTimeWindow(spec *driveSearchSpec, now time.Time) (string, error) } sinceUnix, err := parseTimeValue(spec.OpenedSince, now) if err != nil { - return "", output.ErrValidation("invalid --opened-since %q: %s", spec.OpenedSince, err) + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --opened-since %q: %s", spec.OpenedSince, err).WithParam("--opened-since") } var untilUnix int64 if spec.OpenedUntil != "" { untilUnix, err = parseTimeValue(spec.OpenedUntil, now) if err != nil { - return "", output.ErrValidation("invalid --opened-until %q: %s", spec.OpenedUntil, err) + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --opened-until %q: %s", spec.OpenedUntil, err).WithParam("--opened-until") } } else { untilUnix = now.Unix() @@ -440,7 +440,7 @@ func clampOpenedTimeWindow(spec *driveSearchSpec, now time.Time) (string, error) } maxSecs := int64(driveSearchMaxOpenedSpanDays) * 24 * 3600 if spanSecs > maxSecs { - return "", output.ErrValidation( + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "--opened-* window spans %d days, exceeds the %d-day (1-year) maximum; narrow the range or run multiple queries", spanSecs/86400, driveSearchMaxOpenedSpanDays, ) @@ -505,7 +505,7 @@ func buildTimeRangeFilter(key, since, until string, now time.Time) (map[string]i if since != "" { unix, err := parseTimeValue(since, now) if err != nil { - return nil, nil, output.ErrValidation("invalid --%s-since %q: %s", timeDimCLIName(key), since, err) + return nil, nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --%s-since %q: %s", timeDimCLIName(key), since, err).WithParam(fmt.Sprintf("--%s-since", timeDimCLIName(key))) } if hourAggregated && unix%3600 != 0 { snapped := floorHour(unix) @@ -517,7 +517,7 @@ func buildTimeRangeFilter(key, since, until string, now time.Time) (map[string]i if until != "" { unix, err := parseTimeValue(until, now) if err != nil { - return nil, nil, output.ErrValidation("invalid --%s-until %q: %s", timeDimCLIName(key), until, err) + return nil, nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --%s-until %q: %s", timeDimCLIName(key), until, err).WithParam(fmt.Sprintf("--%s-until", timeDimCLIName(key))) } if hourAggregated && unix%3600 != 0 { snapped := ceilHour(unix) @@ -571,7 +571,7 @@ var driveSearchRelativeRe = regexp.MustCompile(`^(\d+)([dmy])$`) func parseTimeValue(input string, now time.Time) (int64, error) { s := strings.TrimSpace(input) if s == "" { - return 0, fmt.Errorf("empty value") + return 0, fmt.Errorf("empty value") //nolint:forbidigo // intermediate parse helper; caller wraps into typed ValidationError } if m := driveSearchRelativeRe.FindStringSubmatch(s); m != nil { @@ -616,34 +616,27 @@ func parseTimeValue(input string, now time.Time) (int64, error) { } } - return 0, fmt.Errorf("expected relative (7d/1m/1y), date (YYYY-MM-DD[ HH:MM:SS]), RFC3339, or unix seconds") + return 0, fmt.Errorf("expected relative (7d/1m/1y), date (YYYY-MM-DD[ HH:MM:SS]), RFC3339, or unix seconds") //nolint:forbidigo // intermediate parse helper; caller wraps into typed ValidationError } func callDriveSearchAPI(runtime *common.RuntimeContext, reqBody map[string]interface{}) (map[string]interface{}, error) { - data, err := runtime.CallAPI("POST", "/open-apis/search/v2/doc_wiki/search", nil, reqBody) + data, err := runtime.CallAPITyped("POST", "/open-apis/search/v2/doc_wiki/search", nil, reqBody) if err != nil { return nil, enrichDriveSearchError(err) } return data, nil } -// enrichDriveSearchError adds a +search-specific hint for known opaque Lark -// codes; other errors pass through unchanged. +// enrichDriveSearchError adds a +search-specific hint for a known opaque Lark +// code; other errors pass through unchanged. The hint is appended in place on +// the typed Problem, preserving its category / subtype / code / log_id. func enrichDriveSearchError(err error) error { - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { + p, ok := errs.ProblemOf(err) + if !ok || p.Code != driveSearchErrUserNotVisible { return err } - if exitErr.Detail.Code != driveSearchErrUserNotVisible { - return err - } - detail := *exitErr.Detail - detail.Hint = "one or more open_ids in --creator-ids / --sharer-ids are outside this app's user-visibility scope (this is the app's contact visibility, not the search:docs:read API scope); ask an admin to grant the app visibility to those users in the developer console, or drop the unreachable open_ids" - return &output.ExitError{ - Code: exitErr.Code, - Detail: &detail, - Err: exitErr.Err, - } + p.Hint = "one or more open_ids in --creator-ids / --sharer-ids are outside this app's user-visibility scope (this is the app's contact visibility, not the search:docs:read API scope); ask an admin to grant the app visibility to those users in the developer console, or drop the unreachable open_ids" + return err } func cloneDriveSearchFilter(src map[string]interface{}) map[string]interface{} { diff --git a/shortcuts/drive/drive_search_test.go b/shortcuts/drive/drive_search_test.go index a26faf3e6..7e9721e02 100644 --- a/shortcuts/drive/drive_search_test.go +++ b/shortcuts/drive/drive_search_test.go @@ -13,6 +13,8 @@ import ( "testing" "time" + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/errclass" "github.com/larksuite/cli/internal/output" ) @@ -258,6 +260,19 @@ func TestValidateDriveSearchIDs(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "--creator-ids") { t.Fatalf("expected --creator-ids error, got: %v", err) } + var vErr *errs.ValidationError + if !errors.As(err, &vErr) { + t.Fatalf("expected *errs.ValidationError, got %T", err) + } + if vErr.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("Subtype = %q, want %q", vErr.Subtype, errs.SubtypeInvalidArgument) + } + if vErr.Param != "--creator-ids" { + t.Fatalf("Param = %q, want --creator-ids", vErr.Param) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Fatalf("exit code = %d, want ExitValidation (%d)", got, output.ExitValidation) + } }) t.Run("bad chat id format", func(t *testing.T) { @@ -625,51 +640,39 @@ func TestEnrichDriveSearchError(t *testing.T) { } }) - t.Run("ExitError without Detail passes through", func(t *testing.T) { - t.Parallel() - orig := &output.ExitError{Code: 1} - if got := enrichDriveSearchError(orig); got != orig { - t.Fatalf("ExitError without Detail should pass through unchanged") - } - }) - - t.Run("ExitError with non-matching code passes through", func(t *testing.T) { + t.Run("typed error with non-matching code passes through", func(t *testing.T) { t.Parallel() - orig := &output.ExitError{ - Code: 1, - Detail: &output.ErrDetail{Code: 12345, Message: "other"}, - } + orig := errclass.BuildAPIError( + map[string]any{"code": float64(12345), "msg": "other"}, + errclass.ClassifyContext{}, + ) if got := enrichDriveSearchError(orig); got != orig { t.Fatalf("non-matching code should pass through unchanged") } }) - t.Run("matching code rewrites Hint without mutating original", func(t *testing.T) { + t.Run("matching code decorates the typed error's hint in place", func(t *testing.T) { t.Parallel() - orig := &output.ExitError{ - Code: 1, - Detail: &output.ErrDetail{ - Code: driveSearchErrUserNotVisible, - Message: "[99992351] user not visible", - Hint: "", - }, - } + orig := errclass.BuildAPIError( + map[string]any{"code": float64(driveSearchErrUserNotVisible), "msg": "[99992351] user not visible"}, + errclass.ClassifyContext{}, + ) + // Terminal decoration of an upstream error: the hint is set in place on + // the existing typed Problem and that same error is returned (no new + // error is constructed). enriched := enrichDriveSearchError(orig) - eErr, ok := enriched.(*output.ExitError) - if !ok { - t.Fatalf("expected *output.ExitError, got %T", enriched) - } - if eErr == orig { - t.Fatal("should return a new ExitError, not mutate the original") + if enriched != orig { + t.Fatal("should decorate and return the upstream error, not construct a new one") } - if orig.Detail.Hint != "" { - t.Fatal("original Detail.Hint must remain unchanged") + p, ok := errs.ProblemOf(enriched) + if !ok { + t.Fatalf("expected a typed errs.* error, got %T", enriched) } - if !strings.Contains(eErr.Detail.Hint, "--creator-ids") { - t.Fatalf("hint should mention --creator-ids, got %q", eErr.Detail.Hint) + if !strings.Contains(p.Hint, "--creator-ids") { + t.Fatalf("hint should mention --creator-ids, got %q", p.Hint) } - if eErr.Detail.Message != orig.Detail.Message { - t.Fatalf("Message should be preserved, got %q", eErr.Detail.Message) + if p.Message != "[99992351] user not visible" { + t.Fatalf("Message should be preserved, got %q", p.Message) } }) } @@ -739,6 +742,18 @@ func TestBuildDriveSearchRequest(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "--mine") { t.Fatalf("expected exclusion error, got: %v", err) } + // Mutual-exclusion error: typed validation, but no single attributable + // flag, so Param stays empty. + var vErr *errs.ValidationError + if !errors.As(err, &vErr) { + t.Fatalf("expected *errs.ValidationError, got %T", err) + } + if vErr.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("Subtype = %q, want %q", vErr.Subtype, errs.SubtypeInvalidArgument) + } + if vErr.Param != "" { + t.Fatalf("Param = %q, want empty for mutual-exclusion error", vErr.Param) + } }) t.Run("--folder-tokens + --space-ids mutually exclusive", func(t *testing.T) { diff --git a/shortcuts/drive/drive_secure_label.go b/shortcuts/drive/drive_secure_label.go index 3d2dee1a6..a7b8f95db 100644 --- a/shortcuts/drive/drive_secure_label.go +++ b/shortcuts/drive/drive_secure_label.go @@ -7,7 +7,7 @@ import ( "context" "fmt" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -36,7 +36,7 @@ var DriveSecureLabelList = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { pageSize := runtime.Int("page-size") if pageSize < 1 || pageSize > 10 { - return output.ErrValidation("--page-size must be between 1 and 10") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--page-size must be between 1 and 10").WithParam("--page-size") } return nil }, @@ -47,7 +47,7 @@ var DriveSecureLabelList = common.Shortcut{ Params(buildSecureLabelListParams(runtime)) }, Execute: func(ctx context.Context, runtime *common.RuntimeContext) error { - data, err := runtime.CallAPI("GET", + data, err := runtime.CallAPITyped("GET", "/open-apis/drive/v2/my_secure_labels", buildSecureLabelListParams(runtime), nil, @@ -95,7 +95,7 @@ var DriveSecureLabelUpdate = common.Shortcut{ return err } body := map[string]interface{}{"id": runtime.Str("label-id")} - data, err := runtime.CallAPI("PATCH", + data, err := runtime.CallAPITyped("PATCH", fmt.Sprintf("/open-apis/drive/v2/files/%s/secure_label", validate.EncodePathSegment(token)), map[string]interface{}{"type": docType}, body, diff --git a/shortcuts/drive/drive_status.go b/shortcuts/drive/drive_status.go index b3e9470c0..45554f384 100644 --- a/shortcuts/drive/drive_status.go +++ b/shortcuts/drive/drive_status.go @@ -17,7 +17,7 @@ import ( larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -75,27 +75,27 @@ var DriveStatus = common.Shortcut{ localDir := strings.TrimSpace(runtime.Str("local-dir")) folderToken := strings.TrimSpace(runtime.Str("folder-token")) if localDir == "" { - return common.FlagErrorf("--local-dir is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--local-dir is required").WithParam("--local-dir") } if folderToken == "" { - return common.FlagErrorf("--folder-token is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--folder-token is required").WithParam("--folder-token") } if err := validate.ResourceName(folderToken, "--folder-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--folder-token") } // Path safety (absolute paths, traversal, symlink escape) is enforced // upfront by the framework helper so the error message references the // correct flag name; FileIO().Stat below would do the same check, but // surface --file in its hint. if _, err := validate.SafeLocalFlagPath("--local-dir", localDir); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--local-dir") } info, err := runtime.FileIO().Stat(localDir) if err != nil { - return common.WrapInputStatError(err) + return driveInputStatError(err) } if !info.IsDir() { - return output.ErrValidation("--local-dir is not a directory: %s", localDir) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--local-dir is not a directory: %s", localDir).WithParam("--local-dir") } // Conditional scope pre-check: quick mode only compares local mtime with // Drive modified_time, so it must not be blocked on the download grant. @@ -144,11 +144,11 @@ var DriveStatus = common.Shortcut{ // only possible under a Validate↔Execute race. safeRoot, err := validate.SafeInputPath(localDir) if err != nil { - return output.ErrValidation("--local-dir: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--local-dir: %s", err).WithParam("--local-dir") } cwdCanonical, err := validate.SafeInputPath(".") if err != nil { - return output.ErrValidation("could not resolve cwd: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "could not resolve cwd: %s", err) } fmt.Fprintf(runtime.IO().ErrOut, "Walking local: %s\n", localDir) @@ -263,7 +263,7 @@ func walkLocalForStatus(root, cwdCanonical string) (map[string]driveStatusLocalF return nil }) if err != nil { - return nil, output.Errorf(output.ExitInternal, "io", "walk %s: %s", root, err) + return nil, errs.NewInternalError(errs.SubtypeFileIO, "walk %s: %s", root, err).WithCause(err) } return files, nil } @@ -276,12 +276,12 @@ func driveStatusShouldTreatAsUnchangedQuick(remoteModified string, local time.Ti func hashLocalForStatus(runtime *common.RuntimeContext, path string) (string, error) { f, err := runtime.FileIO().Open(path) if err != nil { - return "", common.WrapInputStatError(err) + return "", driveInputStatError(err) } defer f.Close() h := sha256.New() if _, err := io.Copy(h, f); err != nil { - return "", output.Errorf(output.ExitInternal, "io", "hash %s: %s", path, err) + return "", errs.NewInternalError(errs.SubtypeFileIO, "hash %s: %s", path, err).WithCause(err) } return hex.EncodeToString(h.Sum(nil)), nil } @@ -292,12 +292,12 @@ func hashRemoteForStatus(ctx context.Context, runtime *common.RuntimeContext, fi ApiPath: fmt.Sprintf("/open-apis/drive/v1/files/%s/download", validate.EncodePathSegment(fileToken)), }) if err != nil { - return "", output.ErrNetwork("download %s: %s", common.MaskToken(fileToken), err) + return "", wrapDriveNetworkErr(err, "download %s: %s", common.MaskToken(fileToken), err) } defer resp.Body.Close() h := sha256.New() if _, err := io.Copy(h, resp.Body); err != nil { - return "", output.ErrNetwork("hash remote %s: %s", common.MaskToken(fileToken), err) + return "", wrapDriveNetworkErr(err, "hash remote %s: %s", common.MaskToken(fileToken), err) } return hex.EncodeToString(h.Sum(nil)), nil } diff --git a/shortcuts/drive/drive_status_test.go b/shortcuts/drive/drive_status_test.go index 1a9210cc1..d00498460 100644 --- a/shortcuts/drive/drive_status_test.go +++ b/shortcuts/drive/drive_status_test.go @@ -822,12 +822,15 @@ func TestWalkLocalForStatusMissingRootReturnsInternalError(t *testing.T) { if err == nil { t.Fatal("expected walkLocalForStatus() to fail for missing root") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) { - t.Fatalf("expected structured ExitError, got %T", err) + var internalErr *errs.InternalError + if !errors.As(err, &internalErr) { + t.Fatalf("expected *errs.InternalError, got %T", err) } - if exitErr.Detail == nil || exitErr.Detail.Type != "io" { - t.Fatalf("expected io error detail, got %#v", exitErr.Detail) + if internalErr.Subtype != errs.SubtypeFileIO { + t.Fatalf("Subtype = %q, want %q", internalErr.Subtype, errs.SubtypeFileIO) + } + if code := output.ExitCodeOf(err); code != output.ExitInternal { + t.Fatalf("exit code = %d, want %d (ExitInternal)", code, output.ExitInternal) } if !strings.Contains(err.Error(), "walk") { t.Fatalf("expected walk-related error, got: %v", err) diff --git a/shortcuts/drive/drive_sync.go b/shortcuts/drive/drive_sync.go index 3c512cecf..ecb97e52d 100644 --- a/shortcuts/drive/drive_sync.go +++ b/shortcuts/drive/drive_sync.go @@ -13,7 +13,7 @@ import ( "path/filepath" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -72,23 +72,23 @@ var DriveSync = common.Shortcut{ localDir := strings.TrimSpace(runtime.Str("local-dir")) folderToken := strings.TrimSpace(runtime.Str("folder-token")) if localDir == "" { - return common.FlagErrorf("--local-dir is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--local-dir is required").WithParam("--local-dir") } if folderToken == "" { - return common.FlagErrorf("--folder-token is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--folder-token is required").WithParam("--folder-token") } if err := validate.ResourceName(folderToken, "--folder-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--folder-token") } if _, err := validate.SafeLocalFlagPath("--local-dir", localDir); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--local-dir") } info, err := runtime.FileIO().Stat(localDir) if err != nil { - return common.WrapInputStatError(err) + return driveInputStatError(err) } if !info.IsDir() { - return output.ErrValidation("--local-dir is not a directory: %s", localDir) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--local-dir is not a directory: %s", localDir).WithParam("--local-dir") } return nil }, @@ -118,15 +118,15 @@ var DriveSync = common.Shortcut{ safeRoot, err := validate.SafeInputPath(localDir) if err != nil { - return output.ErrValidation("--local-dir: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--local-dir: %s", err).WithParam("--local-dir") } cwdCanonical, err := validate.SafeInputPath(".") if err != nil { - return output.ErrValidation("could not resolve cwd: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "could not resolve cwd: %s", err) } rootRelToCwd, err := filepath.Rel(cwdCanonical, safeRoot) if err != nil { - return output.ErrValidation("--local-dir resolves outside cwd: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--local-dir resolves outside cwd: %s", err).WithParam("--local-dir") } // --- Phase 1: Compute diff (same logic as +status) --- @@ -176,18 +176,18 @@ var DriveSync = common.Shortcut{ } } if len(typeConflicts) > 0 { - return output.ErrValidation("+sync cannot proceed: path type conflict — %s; remove the local entry or the remote entry and retry", strings.Join(typeConflicts, "; ")) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "+sync cannot proceed: path type conflict — %s; remove the local entry or the remote entry and retry", strings.Join(typeConflicts, "; ")) } // Build the exact remote-file views that later execution will use so the // diff phase classifies files against the same duplicate-resolution choice. pullRemoteFiles, _, err := drivePullRemoteViews(entries, duplicateRemote) if err != nil { - return output.Errorf(output.ExitInternal, "internal", "%s", err) + return errs.WrapInternal(err) } remoteEntriesForPush, remoteFolders, _, err := drivePushRemoteViews(entries, duplicateRemote) if err != nil { - return output.Errorf(output.ExitInternal, "internal", "%s", err) + return errs.WrapInternal(err) } remoteFiles := driveSyncStatusRemoteFiles(pullRemoteFiles) @@ -240,43 +240,19 @@ var DriveSync = common.Shortcut{ conflictResolutions := make(map[string]string, len(modified)) if onConflict == driveSyncOnConflictAsk && len(modified) > 0 && runtime.IO().In == nil { - return output.ErrValidation("--on-conflict=ask requires interactive stdin when modified files exist") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--on-conflict=ask requires interactive stdin when modified files exist").WithParam("--on-conflict") } for _, entry := range modified { resolved := onConflict if resolved == driveSyncOnConflictAsk { resolved, err = driveSyncAskConflict(entry.RelPath, runtime) if err != nil { - payload := map[string]interface{}{ - "detection": detection, - "diff": map[string]interface{}{ - "new_local": emptyIfNil(newLocal), - "new_remote": emptyIfNil(newRemote), - "modified": emptyIfNil(modified), - "unchanged": emptyIfNil(unchanged), - }, - "summary": map[string]interface{}{ - "pulled": 0, - "pushed": 0, - "skipped": 0, - "failed": 1, - }, - "items": []driveSyncItem{{ - RelPath: entry.RelPath, - FileToken: entry.FileToken, - Action: "failed", - Direction: "conflict", - Error: err.Error(), - }}, - } - return &output.ExitError{ - Code: output.ExitAPI, - Detail: &output.ErrDetail{ - Type: "partial_failure", - Message: fmt.Sprintf("cannot collect conflict decisions for +sync: %v", err), - Detail: payload, - }, - } + // Phase-1 setup abort: no sync operation ran yet, so this + // is not a batch partial-failure. driveSyncAskConflict + // already returns a typed *errs.ValidationError; propagate + // it unchanged rather than re-wrapping it as a synthetic + // partial_failure payload. + return err } } conflictResolutions[entry.RelPath] = resolved @@ -521,17 +497,12 @@ var DriveSync = common.Shortcut{ } if failed > 0 { - msg := fmt.Sprintf("%d item(s) failed during +sync", failed) - return &output.ExitError{ - Code: output.ExitAPI, - Detail: &output.ErrDetail{ - Type: "partial_failure", - Message: msg, - Detail: payload, - }, - } + payload["note"] = fmt.Sprintf("%d item(s) failed during +sync", failed) } + if failed > 0 { + return runtime.OutPartialFailure(payload, nil) + } runtime.Out(payload, nil) return nil }, @@ -555,7 +526,7 @@ func driveSyncStatusRemoteFiles(pullRemoteFiles map[string]drivePullTarget) map[ func driveSyncAskConflict(relPath string, runtime *common.RuntimeContext) (string, error) { fmt.Fprintf(runtime.IO().ErrOut, "CONFLICT: both sides modified %q. Choose: [R]emote-wins / [L]ocal-wins / [K]eep-both / [S]kip (default: R): ", relPath) if runtime.IO().In == nil { - return "", output.ErrValidation("cannot resolve conflict for %q with --on-conflict=ask: stdin is not available", relPath) + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot resolve conflict for %q with --on-conflict=ask: stdin is not available", relPath).WithParam("--on-conflict") } reader, ok := runtime.IO().In.(*bufio.Reader) if !ok { @@ -564,12 +535,12 @@ func driveSyncAskConflict(relPath string, runtime *common.RuntimeContext) (strin } line, err := reader.ReadString('\n') if err != nil && !errors.Is(err, io.EOF) { - return "", output.ErrValidation("cannot read conflict choice for %q: %s", relPath, err) + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot read conflict choice for %q: %s", relPath, err).WithParam("--on-conflict") } answer := strings.TrimSpace(strings.ToLower(line)) if answer == "" { if errors.Is(err, io.EOF) { - return "", output.ErrValidation("cannot resolve conflict for %q with --on-conflict=ask: stdin reached EOF before any choice was provided", relPath) + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot resolve conflict for %q with --on-conflict=ask: stdin reached EOF before any choice was provided", relPath).WithParam("--on-conflict") } return driveSyncOnConflictRemoteWins, nil } @@ -583,7 +554,7 @@ func driveSyncAskConflict(relPath string, runtime *common.RuntimeContext) (strin case "r", "remote", "remote-wins": return driveSyncOnConflictRemoteWins, nil default: - return "", output.ErrValidation("invalid conflict choice for %q: %q (expected one of remote/local/keep/skip)", relPath, strings.TrimSpace(line)) + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid conflict choice for %q: %q (expected one of remote/local/keep/skip)", relPath, strings.TrimSpace(line)).WithParam("--on-conflict") } } @@ -635,16 +606,16 @@ func driveSyncNeedsCreateScope(uploadPaths []string, localDirs []string, folderC func driveSyncRollbackRenamedLocal(oldAbsPath, newAbsPath string) error { if info, err := os.Stat(oldAbsPath); err == nil { //nolint:forbidigo // shortcuts cannot import internal/vfs (depguard rule shortcuts-no-vfs); safeRoot is validated. if info.IsDir() { - return output.Errorf(output.ExitInternal, "rollback", "original path became a directory during rollback: %s", oldAbsPath) + return errs.NewInternalError(errs.SubtypeFileIO, "original path became a directory during rollback: %s", oldAbsPath) } if err := os.Remove(oldAbsPath); err != nil { //nolint:forbidigo // shortcuts cannot import internal/vfs (depguard rule shortcuts-no-vfs); safeRoot is validated. - return output.Errorf(output.ExitInternal, "rollback", "remove partial restored path %q: %s", oldAbsPath, err) + return errs.NewInternalError(errs.SubtypeFileIO, "remove partial restored path %q: %s", oldAbsPath, err).WithCause(err) } } else if !os.IsNotExist(err) { - return output.Errorf(output.ExitInternal, "rollback", "stat original path %q during rollback: %s", oldAbsPath, err) + return errs.NewInternalError(errs.SubtypeFileIO, "stat original path %q during rollback: %s", oldAbsPath, err).WithCause(err) } if err := os.Rename(newAbsPath, oldAbsPath); err != nil { //nolint:forbidigo // shortcuts cannot import internal/vfs (depguard rule shortcuts-no-vfs); safeRoot is validated. - return output.Errorf(output.ExitInternal, "rollback", "restore renamed local file %q: %s", oldAbsPath, err) + return errs.NewInternalError(errs.SubtypeFileIO, "restore renamed local file %q: %s", oldAbsPath, err).WithCause(err) } return nil } diff --git a/shortcuts/drive/drive_sync_test.go b/shortcuts/drive/drive_sync_test.go index 7364397eb..1e1ea5afc 100644 --- a/shortcuts/drive/drive_sync_test.go +++ b/shortcuts/drive/drive_sync_test.go @@ -18,6 +18,7 @@ import ( "testing" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" @@ -1434,14 +1435,15 @@ func TestDriveSyncAskConflictEOFDuringExecuteReportsFailedItem(t *testing.T) { if err == nil { t.Fatalf("expected EOF failure during ask execution\nstdout: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured ExitError, got: %v", err) + // Collecting conflict decisions runs in the Phase-1 setup pass, before + // any sync operation executes, so the EOF abort propagates the typed + // *errs.ValidationError unchanged rather than a synthetic partial_failure. + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) } - detailMap, _ := exitErr.Detail.Detail.(map[string]interface{}) - items, _ := detailMap["items"].([]driveSyncItem) - if len(items) == 0 || !strings.Contains(items[0].Error, "stdin reached EOF") { - t.Fatalf("expected failed ask item, got detail: %#v", exitErr.Detail.Detail) + if !strings.Contains(validationErr.Error(), "stdin reached EOF") { + t.Fatalf("expected EOF failure, got: %v", validationErr) } data, readErr := os.ReadFile("local/a.txt") if readErr != nil { @@ -1503,12 +1505,15 @@ func TestDriveSyncAskConflictEOFDuringPlanningPreventsAnyWrites(t *testing.T) { if err == nil { t.Fatalf("expected EOF failure during ask planning\nstdout: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured ExitError, got: %v", err) + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) } - if exitErr.Detail.Type != "partial_failure" || !strings.Contains(exitErr.Error(), "stdin reached EOF") { - t.Fatalf("expected planning failure detail mentioning EOF, got: %#v", exitErr.Detail) + if validationErr.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("subtype = %q, want %q", validationErr.Subtype, errs.SubtypeInvalidArgument) + } + if !strings.Contains(validationErr.Error(), "stdin reached EOF") { + t.Fatalf("expected planning failure mentioning EOF, got: %v", validationErr) } if data, readErr := os.ReadFile("local/a.txt"); readErr != nil || string(data) != "local-a" { t.Fatalf("a.txt should remain untouched, readErr=%v content=%q", readErr, string(data)) @@ -1706,14 +1711,10 @@ func TestDriveSyncReportsNewRemoteDownloadFailure(t *testing.T) { if err == nil { t.Fatalf("expected download failure\nstdout: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured ExitError, got: %v", err) - } - detailMap, _ := exitErr.Detail.Detail.(map[string]interface{}) - items, _ := detailMap["items"].([]driveSyncItem) + assertDriveSyncPartialFailure(t, err) + items := driveSyncStdoutItems(t, stdout.Bytes()) if len(items) == 0 || items[0].Direction != "pull" || !strings.Contains(items[0].Error, "save failed") { - t.Fatalf("expected failed pull item, got detail: %#v", exitErr.Detail.Detail) + t.Fatalf("expected failed pull item, got detail: %#v", stdout.String()) } } @@ -1758,14 +1759,10 @@ func TestDriveSyncReportsNewLocalEnsureFailure(t *testing.T) { if err == nil { t.Fatalf("expected ensure failure\nstdout: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured ExitError, got: %v", err) - } - detailMap, _ := exitErr.Detail.Detail.(map[string]interface{}) - items, _ := detailMap["items"].([]driveSyncItem) + assertDriveSyncPartialFailure(t, err) + items := driveSyncStdoutItems(t, stdout.Bytes()) if len(items) == 0 || items[0].Direction != "push" || !strings.Contains(items[0].Error, "create parent failed") { - t.Fatalf("expected failed push item, got detail: %#v", exitErr.Detail.Detail) + t.Fatalf("expected failed push item, got detail: %#v", stdout.String()) } } @@ -1810,14 +1807,10 @@ func TestDriveSyncReportsNewLocalUploadFailure(t *testing.T) { if err == nil { t.Fatalf("expected upload failure\nstdout: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured ExitError, got: %v", err) - } - detailMap, _ := exitErr.Detail.Detail.(map[string]interface{}) - items, _ := detailMap["items"].([]driveSyncItem) + assertDriveSyncPartialFailure(t, err) + items := driveSyncStdoutItems(t, stdout.Bytes()) if len(items) == 0 || items[0].Direction != "push" || !strings.Contains(items[0].Error, "upload failed") { - t.Fatalf("expected failed upload item, got detail: %#v", exitErr.Detail.Detail) + t.Fatalf("expected failed upload item, got detail: %#v", stdout.String()) } } @@ -1875,14 +1868,10 @@ func TestDriveSyncLocalWinsReportsUploadFailure(t *testing.T) { if err == nil { t.Fatalf("expected local-wins upload failure\nstdout: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured ExitError, got: %v", err) - } - detailMap, _ := exitErr.Detail.Detail.(map[string]interface{}) - items, _ := detailMap["items"].([]driveSyncItem) + assertDriveSyncPartialFailure(t, err) + items := driveSyncStdoutItems(t, stdout.Bytes()) if len(items) == 0 || items[0].Direction != "push" || !strings.Contains(items[0].Error, "overwrite failed") { - t.Fatalf("expected failed overwrite item, got detail: %#v", exitErr.Detail.Detail) + t.Fatalf("expected failed overwrite item, got detail: %#v", stdout.String()) } } @@ -1965,30 +1954,13 @@ func TestDriveSyncKeepBothReportsRenameFailure(t *testing.T) { if err == nil { t.Fatalf("expected keep-both suffix exhaustion error\nstdout: %s", stdout.String()) } - // The error may be a plain ExitError (no Detail.Detail) or a - // partial_failure with items. Either way it must mention the - // suffix exhaustion. - errMsg := err.Error() - // The suffix exhaustion message may be in the top-level error or - // inside a partial_failure detail item. Check both. - foundSuffixError := strings.Contains(errMsg, "could not generate a unique rel_path") - if !foundSuffixError { - var exitErr *output.ExitError - if errors.As(err, &exitErr) && exitErr.Detail != nil { - detailMap, _ := exitErr.Detail.Detail.(map[string]interface{}) - items, _ := detailMap["items"].([]driveSyncItem) - for _, item := range items { - if strings.Contains(item.Error, "could not generate a unique rel_path") { - foundSuffixError = true - break - } - } - if !foundSuffixError { - t.Fatalf("expected suffix exhaustion error, got: %s; detail: %#v", errMsg, exitErr.Detail.Detail) - } - } else { - t.Fatalf("expected suffix exhaustion error, got: %s", errMsg) - } + // The suffix-exhaustion failure is an item-level conflict failure, so + // it surfaces as the partial-failure signal: a typed PartialFailureError + // on the error channel and the ok:false items[] payload (carrying the + // suffix message) on stdout via OutPartialFailure. + assertDriveSyncPartialFailure(t, err) + if !strings.Contains(stdout.String(), "could not generate a unique rel_path") { + t.Fatalf("expected suffix exhaustion error in stdout items, got: %s", stdout.String()) } } @@ -2341,14 +2313,10 @@ func TestDriveSyncRemoteWinsReportsModifiedPullFailure(t *testing.T) { if err == nil { t.Fatalf("expected modified pull failure\nstdout: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured ExitError, got: %v", err) - } - detailMap, _ := exitErr.Detail.Detail.(map[string]interface{}) - items, _ := detailMap["items"].([]driveSyncItem) + assertDriveSyncPartialFailure(t, err) + items := driveSyncStdoutItems(t, stdout.Bytes()) if len(items) == 0 || items[0].Direction != "pull" || !strings.Contains(items[0].Error, "save failed") { - t.Fatalf("expected failed modified pull item, got detail: %#v", exitErr.Detail.Detail) + t.Fatalf("expected failed modified pull item, got detail: %#v", stdout.String()) } } @@ -2411,14 +2379,10 @@ func TestDriveSyncKeepBothReportsRollbackFailureAfterPullError(t *testing.T) { if err == nil { t.Fatalf("expected keep-both rollback failure\nstdout: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured ExitError, got: %v", err) - } - detailMap, _ := exitErr.Detail.Detail.(map[string]interface{}) - items, _ := detailMap["items"].([]driveSyncItem) + assertDriveSyncPartialFailure(t, err) + items := driveSyncStdoutItems(t, stdout.Bytes()) if len(items) == 0 || !strings.Contains(items[0].Error, "rollback failed") { - t.Fatalf("expected rollback failure in item error, got detail: %#v", exitErr.Detail.Detail) + t.Fatalf("expected rollback failure in item error, got detail: %#v", stdout.String()) } } @@ -2500,14 +2464,10 @@ func TestDriveSyncLocalWinsNestedFileReportsParentEnsureFailure(t *testing.T) { if err == nil { t.Fatalf("expected parent ensure failure\nstdout: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured ExitError, got: %v", err) - } - detailMap, _ := exitErr.Detail.Detail.(map[string]interface{}) - items, _ := detailMap["items"].([]driveSyncItem) + assertDriveSyncPartialFailure(t, err) + items := driveSyncStdoutItems(t, stdout.Bytes()) if len(items) == 0 || !strings.Contains(items[0].Error, "create parent failed") { - t.Fatalf("expected failed item with create_folder error, got detail: %#v", exitErr.Detail.Detail) + t.Fatalf("expected failed item with create_folder error, got detail: %#v", stdout.String()) } } @@ -2704,7 +2664,7 @@ func TestDriveSyncKeepBothReportsSuffixError(t *testing.T) { // TestDriveSyncKeepBothRollbackSucceedsOnPullFailure verifies the full // keep-both rollback path: when the pull download fails after the local // file has been renamed, the rollback restores the original file and -// the error is reported as a partial_failure. +// the failure is reported via the partial-failure signal. func TestDriveSyncKeepBothRollbackSucceedsOnPullFailure(t *testing.T) { syncTestConfig := &core.CliConfig{ AppID: "drive-sync-keep-both-rollback-pull-fail", AppSecret: "test-secret", Brand: core.BrandFeishu, @@ -2762,14 +2722,10 @@ func TestDriveSyncKeepBothRollbackSucceedsOnPullFailure(t *testing.T) { if err == nil { t.Fatalf("expected keep-both pull failure with rollback\nstdout: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured ExitError, got: %v", err) - } - detailMap, _ := exitErr.Detail.Detail.(map[string]interface{}) - items, _ := detailMap["items"].([]driveSyncItem) + assertDriveSyncPartialFailure(t, err) + items := driveSyncStdoutItems(t, stdout.Bytes()) if len(items) == 0 || !strings.Contains(items[0].Error, "save failed") { - t.Fatalf("expected save failure in item, got detail: %#v", exitErr.Detail.Detail) + t.Fatalf("expected save failure in item, got detail: %#v", stdout.String()) } // Rollback should have restored the original file. @@ -2978,14 +2934,10 @@ func TestDriveSyncLocalWinsUsesReturnedTokenOnUploadFailure(t *testing.T) { if err == nil { t.Fatalf("expected local-wins upload failure\nstdout: %s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured ExitError, got: %v", err) - } - detailMap, _ := exitErr.Detail.Detail.(map[string]interface{}) - items, _ := detailMap["items"].([]driveSyncItem) + assertDriveSyncPartialFailure(t, err) + items := driveSyncStdoutItems(t, stdout.Bytes()) if len(items) == 0 { - t.Fatalf("expected failed item, got detail: %#v", exitErr.Detail.Detail) + t.Fatalf("expected failed item, got detail: %#v", stdout.String()) } // The reported token should be the new one from the partial-success // response, not the stale existingToken ("tok_a"). @@ -3095,3 +3047,39 @@ func TestDriveSyncRejectsLocalDirVsRemoteFileTypeConflict(t *testing.T) { t.Fatalf("error should mention local directory, got: %v", err) } } + +// assertDriveSyncPartialFailure asserts that err is the typed partial-failure +// exit signal +sync returns on any item-level failure. The structured +// {detection, diff, summary, items, note} payload rides on stdout as an +// ok:false envelope via runtime.OutPartialFailure (in alignment with +// +push/+pull), so this helper only checks the exit-code signal; callers read +// the payload from stdout. +func assertDriveSyncPartialFailure(t *testing.T, err error) { + t.Helper() + if err == nil { + t.Fatal("expected partial-failure exit signal, got nil") + } + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("expected *output.PartialFailureError, got %T: %v", err, err) + } + if pfErr.Code != output.ExitAPI { + t.Errorf("exit code = %d, want %d (ExitAPI)", pfErr.Code, output.ExitAPI) + } +} + +// driveSyncStdoutItems extracts the items[] payload from the stdout envelope +// written by runtime.Out. The per-item failure context that used to live in +// the partial_failure ExitError detail now rides on stdout. +func driveSyncStdoutItems(t *testing.T, stdout []byte) []driveSyncItem { + t.Helper() + var envelope struct { + Data struct { + Items []driveSyncItem `json:"items"` + } `json:"data"` + } + if err := json.Unmarshal(stdout, &envelope); err != nil { + t.Fatalf("unmarshal stdout: %v\nraw=%s", err, string(stdout)) + } + return envelope.Data.Items +} diff --git a/shortcuts/drive/drive_task_result.go b/shortcuts/drive/drive_task_result.go index d506e1b17..1bdb74cdf 100644 --- a/shortcuts/drive/drive_task_result.go +++ b/shortcuts/drive/drive_task_result.go @@ -9,8 +9,8 @@ import ( "fmt" "strings" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/credential" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -43,34 +43,34 @@ var DriveTaskResult = common.Shortcut{ "wiki_delete_node": true, } if !validScenarios[scenario] { - return output.ErrValidation("unsupported scenario: %s. Supported scenarios: import, export, task_check, wiki_move, wiki_delete_space, wiki_delete_node", scenario) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported scenario: %s. Supported scenarios: import, export, task_check, wiki_move, wiki_delete_space, wiki_delete_node", scenario).WithParam("--scenario") } // Validate required params based on scenario switch scenario { case "import", "export": if runtime.Str("ticket") == "" { - return output.ErrValidation("--ticket is required for %s scenario", scenario) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--ticket is required for %s scenario", scenario).WithParam("--ticket") } if err := validate.ResourceName(runtime.Str("ticket"), "--ticket"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--ticket") } case "task_check", "wiki_move", "wiki_delete_space", "wiki_delete_node": if runtime.Str("task-id") == "" { - return output.ErrValidation("--task-id is required for %s scenario", scenario) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--task-id is required for %s scenario", scenario).WithParam("--task-id") } if err := validate.ResourceName(runtime.Str("task-id"), "--task-id"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--task-id") } } // For export scenario, file-token is required if scenario == "export" && runtime.Str("file-token") == "" { - return output.ErrValidation("--file-token is required for export scenario") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--file-token is required for export scenario").WithParam("--file-token") } if scenario == "export" { if err := validate.ResourceName(runtime.Str("file-token"), "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--file-token") } } @@ -261,9 +261,10 @@ func requireDriveScopes(storedScopes string, required []string) error { return nil } - return output.ErrWithHint(output.ExitAuth, "missing_scope", - fmt.Sprintf("missing required scope(s): %s", strings.Join(missing, ", ")), - fmt.Sprintf("run `lark-cli auth login --scope \"%s\"` in the background. It blocks and outputs a verification URL — retrieve the URL and open it in a browser to complete login.", strings.Join(missing, " "))) + return errs.NewPermissionError(errs.SubtypeMissingScope, + "missing required scope(s): %s", strings.Join(missing, ", ")). + WithMissingScopes(missing...). + WithHint("run `lark-cli auth login --scope \"%s\"` in the background. It blocks and outputs a verification URL — retrieve the URL and open it in a browser to complete login.", strings.Join(missing, " ")) } func missingDriveScopes(storedScopes string, required []string) []string { @@ -408,10 +409,10 @@ func queryWikiMoveTask(runtime *common.RuntimeContext, taskID string) (map[strin func getWikiMoveTaskStatus(runtime *common.RuntimeContext, taskID string) (wikiMoveTaskQueryStatus, error) { if err := validate.ResourceName(taskID, "--task-id"); err != nil { - return wikiMoveTaskQueryStatus{}, output.ErrValidation("%s", err) + return wikiMoveTaskQueryStatus{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--task-id") } - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "GET", fmt.Sprintf("/open-apis/wiki/v2/tasks/%s", validate.EncodePathSegment(taskID)), map[string]interface{}{"task_type": "move"}, @@ -426,7 +427,7 @@ func getWikiMoveTaskStatus(runtime *common.RuntimeContext, taskID string) (wikiM func parseWikiMoveTaskQueryStatus(taskID string, task map[string]interface{}) (wikiMoveTaskQueryStatus, error) { if task == nil { - return wikiMoveTaskQueryStatus{}, output.Errorf(output.ExitAPI, "api_error", "wiki task response missing task") + return wikiMoveTaskQueryStatus{}, errs.NewInternalError(errs.SubtypeInvalidResponse, "wiki task response missing task") } status := wikiMoveTaskQueryStatus{ @@ -490,10 +491,10 @@ func appendWikiMoveNodeFields(out, node map[string]interface{}) { // rather than the per-node array used by wiki move. func queryWikiDeleteSpaceTask(runtime *common.RuntimeContext, taskID string) (map[string]interface{}, error) { if err := validate.ResourceName(taskID, "--task-id"); err != nil { - return nil, output.ErrValidation("%s", err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--task-id") } - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "GET", fmt.Sprintf("/open-apis/wiki/v2/tasks/%s", validate.EncodePathSegment(taskID)), map[string]interface{}{"task_type": "delete_space"}, @@ -505,7 +506,7 @@ func queryWikiDeleteSpaceTask(runtime *common.RuntimeContext, taskID string) (ma task := common.GetMap(data, "task") if task == nil { - return nil, output.Errorf(output.ExitAPI, "api_error", "wiki task response missing task") + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "wiki task response missing task") } resolvedTaskID := common.GetString(task, "task_id") @@ -558,10 +559,10 @@ func queryWikiDeleteSpaceTask(runtime *common.RuntimeContext, taskID string) (ma // keep drive from depending on shortcuts/wiki. func queryWikiDeleteNodeTask(runtime *common.RuntimeContext, taskID string) (map[string]interface{}, error) { if err := validate.ResourceName(taskID, "--task-id"); err != nil { - return nil, output.ErrValidation("%s", err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--task-id") } - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( "GET", fmt.Sprintf("/open-apis/wiki/v2/tasks/%s", validate.EncodePathSegment(taskID)), map[string]interface{}{"task_type": "delete_node"}, @@ -573,7 +574,7 @@ func queryWikiDeleteNodeTask(runtime *common.RuntimeContext, taskID string) (map task := common.GetMap(data, "task") if task == nil { - return nil, output.Errorf(output.ExitAPI, "api_error", "wiki task response missing task") + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "wiki task response missing task") } resolvedTaskID := common.GetString(task, "task_id") diff --git a/shortcuts/drive/drive_task_result_test.go b/shortcuts/drive/drive_task_result_test.go index 79e43d76d..b97fc0ef8 100644 --- a/shortcuts/drive/drive_task_result_test.go +++ b/shortcuts/drive/drive_task_result_test.go @@ -13,10 +13,12 @@ import ( "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/credential" "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -86,6 +88,16 @@ func TestDriveTaskResultValidateErrorsByScenario(t *testing.T) { if err == nil || !strings.Contains(err.Error(), tt.wantErr) { t.Fatalf("expected error containing %q, got %v", tt.wantErr, err) } + var vErr *errs.ValidationError + if !errors.As(err, &vErr) { + t.Fatalf("expected *errs.ValidationError, got %T", err) + } + if vErr.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("Subtype = %q, want %q", vErr.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Fatalf("exit code = %d, want ExitValidation (%d)", got, output.ExitValidation) + } }) } } @@ -428,6 +440,16 @@ func TestValidateDriveTaskResultScopesWikiScenariosRequireWikiScope(t *testing.T if err == nil || !strings.Contains(err.Error(), "missing required scope(s): wiki:space:read") { t.Fatalf("expected missing wiki scope error, got %v", err) } + var permErr *errs.PermissionError + if !errors.As(err, &permErr) { + t.Fatalf("expected *errs.PermissionError, got %T", err) + } + if permErr.Subtype != errs.SubtypeMissingScope { + t.Fatalf("Subtype = %q, want %q", permErr.Subtype, errs.SubtypeMissingScope) + } + if len(permErr.MissingScopes) != 1 || permErr.MissingScopes[0] != "wiki:space:read" { + t.Fatalf("MissingScopes = %v, want [wiki:space:read]", permErr.MissingScopes) + } }) t.Run(scenario+"/accepts wiki scope", func(t *testing.T) { t.Parallel() @@ -663,6 +685,19 @@ func TestParseWikiMoveTaskQueryStatusRejectsMissingTask(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "missing task") { t.Fatalf("expected missing task error, got %v", err) } + // A successful API call (code==0) that omits the `task` field is a + // malformed RESPONSE, not a user error: classify as internal / + // invalid_response (exit 5), not an API business error (exit 1). + var iErr *errs.InternalError + if !errors.As(err, &iErr) { + t.Fatalf("expected *errs.InternalError, got %T", err) + } + if iErr.Subtype != errs.SubtypeInvalidResponse { + t.Fatalf("Subtype = %q, want %q", iErr.Subtype, errs.SubtypeInvalidResponse) + } + if got := output.ExitCodeOf(err); got != output.ExitInternal { + t.Fatalf("exit code = %d, want ExitInternal (%d)", got, output.ExitInternal) + } } func TestWikiMoveTaskQueryStatusPrimarySurfacesFailureOverEarlierSuccess(t *testing.T) { diff --git a/shortcuts/drive/drive_upload.go b/shortcuts/drive/drive_upload.go index c2688208f..f74220bc5 100644 --- a/shortcuts/drive/drive_upload.go +++ b/shortcuts/drive/drive_upload.go @@ -5,7 +5,6 @@ package drive import ( "context" - "encoding/json" "errors" "fmt" "io" @@ -15,6 +14,7 @@ import ( larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" @@ -151,7 +151,7 @@ var DriveUpload = common.Shortcut{ info, err := runtime.FileIO().Stat(spec.FilePath) if err != nil { - return common.WrapInputStatError(err) + return driveInputStatError(err) } fileSize := info.Size() @@ -194,13 +194,13 @@ var DriveUpload = common.Shortcut{ func validateDriveUploadSpec(runtime *common.RuntimeContext, spec driveUploadSpec) error { if driveUploadFlagExplicitlyEmpty(runtime, "file-token") { - return common.FlagErrorf("--file-token cannot be empty; omit --file-token for a new upload or pass an existing file token to overwrite") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--file-token cannot be empty; omit --file-token for a new upload or pass an existing file token to overwrite").WithParam("--file-token") } if driveUploadFlagExplicitlyEmpty(runtime, "folder-token") { - return common.FlagErrorf("--folder-token cannot be empty; omit --folder-token to upload into Drive root folder or pass a folder token") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--folder-token cannot be empty; omit --folder-token to upload into Drive root folder or pass a folder token").WithParam("--folder-token") } if driveUploadFlagExplicitlyEmpty(runtime, "wiki-token") { - return common.FlagErrorf("--wiki-token cannot be empty; omit --wiki-token to upload into Drive root folder or pass a wiki node token") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--wiki-token cannot be empty; omit --wiki-token to upload into Drive root folder or pass a wiki node token").WithParam("--wiki-token") } targets := 0 @@ -211,21 +211,21 @@ func validateDriveUploadSpec(runtime *common.RuntimeContext, spec driveUploadSpe targets++ } if targets > 1 { - return common.FlagErrorf("--folder-token and --wiki-token are mutually exclusive") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--folder-token and --wiki-token are mutually exclusive") } if spec.FolderToken != "" { if err := validate.ResourceName(spec.FolderToken, "--folder-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--folder-token") } } if spec.WikiToken != "" { if err := validate.ResourceName(spec.WikiToken, "--wiki-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--wiki-token") } } if spec.FileToken != "" { if err := validate.ResourceName(spec.FileToken, "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--file-token") } } return nil @@ -240,7 +240,7 @@ func driveUploadFlagExplicitlyEmpty(runtime *common.RuntimeContext, flagName str func uploadFileToDrive(ctx context.Context, runtime *common.RuntimeContext, filePath, fileName string, target driveUploadTarget, fileSize int64, existingFileToken string) (driveUploadResult, error) { f, err := runtime.FileIO().Open(filePath) if err != nil { - return driveUploadResult{}, common.WrapInputStatError(err) + return driveUploadResult{}, driveInputStatError(err) } defer f.Close() @@ -265,23 +265,16 @@ func uploadFileToDrive(ctx context.Context, runtime *common.RuntimeContext, file if errors.As(err, &exitErr) { return driveUploadResult{}, err } - return driveUploadResult{}, output.ErrNetwork("upload failed: %v", err) + return driveUploadResult{}, wrapDriveNetworkErr(err, "upload failed: %v", err) } - var result map[string]interface{} - if err := json.Unmarshal(apiResp.RawBody, &result); err != nil { - return driveUploadResult{}, output.Errorf(output.ExitAPI, "api_error", "upload failed: invalid response JSON: %v", err) - } - - if larkCode := int(common.GetFloat(result, "code")); larkCode != 0 { - msg, _ := result["msg"].(string) - return driveUploadResult{}, output.ErrAPI(larkCode, fmt.Sprintf("upload failed: [%d] %s", larkCode, msg), result["error"]) + data, err := runtime.ClassifyAPIResponse(apiResp) + if err != nil { + return driveUploadResult{}, err } - - data, _ := result["data"].(map[string]interface{}) fileToken := common.GetString(data, "file_token") if fileToken == "" { - return driveUploadResult{}, output.Errorf(output.ExitAPI, "api_error", "upload failed: no file_token returned") + return driveUploadResult{}, errs.NewInternalError(errs.SubtypeInvalidResponse, "upload failed: no file_token returned") } return driveUploadResult{ FileToken: fileToken, @@ -304,7 +297,7 @@ func uploadFileMultipart(_ context.Context, runtime *common.RuntimeContext, file if existingFileToken != "" { prepareBody["file_token"] = existingFileToken } - prepareResult, err := runtime.CallAPI("POST", "/open-apis/drive/v1/files/upload_prepare", nil, prepareBody) + prepareResult, err := runtime.CallAPITyped("POST", "/open-apis/drive/v1/files/upload_prepare", nil, prepareBody) if err != nil { return driveUploadResult{}, err } @@ -316,7 +309,7 @@ func uploadFileMultipart(_ context.Context, runtime *common.RuntimeContext, file blockNum := int(blockNumF) if uploadID == "" || blockSize <= 0 || blockNum <= 0 { - return driveUploadResult{}, output.Errorf(output.ExitAPI, "api_error", + return driveUploadResult{}, errs.NewInternalError(errs.SubtypeInvalidResponse, "upload_prepare returned invalid data: upload_id=%q, block_size=%d, block_num=%d", uploadID, blockSize, blockNum) } @@ -334,7 +327,7 @@ func uploadFileMultipart(_ context.Context, runtime *common.RuntimeContext, file partFile, err := runtime.FileIO().Open(filePath) if err != nil { - return driveUploadResult{}, common.WrapInputStatError(err) + return driveUploadResult{}, driveInputStatError(err) } fd := larkcore.NewFormdata() @@ -354,16 +347,11 @@ func uploadFileMultipart(_ context.Context, runtime *common.RuntimeContext, file if errors.As(err, &exitErr) { return driveUploadResult{}, err } - return driveUploadResult{}, output.ErrNetwork("upload part %d/%d failed: %v", seq+1, blockNum, err) + return driveUploadResult{}, wrapDriveNetworkErr(err, "upload part %d/%d failed: %v", seq+1, blockNum, err) } - var partResult map[string]interface{} - if err := json.Unmarshal(apiResp.RawBody, &partResult); err != nil { - return driveUploadResult{}, output.Errorf(output.ExitAPI, "api_error", "upload part %d/%d: invalid response JSON: %v", seq+1, blockNum, err) - } - if larkCode := int(common.GetFloat(partResult, "code")); larkCode != 0 { - msg, _ := partResult["msg"].(string) - return driveUploadResult{}, output.ErrAPI(larkCode, fmt.Sprintf("upload part %d/%d failed: [%d] %s", seq+1, blockNum, larkCode, msg), partResult["error"]) + if _, err := runtime.ClassifyAPIResponse(apiResp); err != nil { + return driveUploadResult{}, err } fmt.Fprintf(runtime.IO().ErrOut, " Block %d/%d uploaded (%s)\n", seq+1, blockNum, common.FormatSize(partSize)) @@ -374,14 +362,14 @@ func uploadFileMultipart(_ context.Context, runtime *common.RuntimeContext, file "upload_id": uploadID, "block_num": blockNum, } - finishResult, err := runtime.CallAPI("POST", "/open-apis/drive/v1/files/upload_finish", nil, finishBody) + finishResult, err := runtime.CallAPITyped("POST", "/open-apis/drive/v1/files/upload_finish", nil, finishBody) if err != nil { return driveUploadResult{}, err } fileToken := common.GetString(finishResult, "file_token") if fileToken == "" { - return driveUploadResult{}, output.Errorf(output.ExitAPI, "api_error", "upload_finish succeeded but no file_token returned") + return driveUploadResult{}, errs.NewInternalError(errs.SubtypeInvalidResponse, "upload_finish succeeded but no file_token returned") } return driveUploadResult{ diff --git a/shortcuts/drive/drive_version.go b/shortcuts/drive/drive_version.go index bcf3fcf0c..e560dde49 100644 --- a/shortcuts/drive/drive_version.go +++ b/shortcuts/drive/drive_version.go @@ -16,8 +16,8 @@ import ( larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/util" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" @@ -34,10 +34,10 @@ type driveVersionHistorySpec struct { func validateDriveNumericValue(value, flagName, valueLabel string) error { value = strings.TrimSpace(value) if value == "" { - return output.ErrValidation("%s cannot be empty", flagName) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s cannot be empty", flagName).WithParam(flagName) } if !driveVersionNumberRe.MatchString(value) { - return output.ErrValidation("%s must be a numeric %s", flagName, valueLabel) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s must be a numeric %s", flagName, valueLabel).WithParam(flagName) } return nil } @@ -52,10 +52,10 @@ func validateDriveCursorValue(value, flagName string) error { func validateDriveVersionHistorySpec(spec driveVersionHistorySpec) error { if err := validate.ResourceName(spec.FileToken, "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--file-token") } if spec.Limit < 1 || spec.Limit > 200 { - return output.ErrValidation("invalid --limit %d: must be between 1 and 200", spec.Limit) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --limit %d: must be between 1 and 200", spec.Limit).WithParam("--limit") } if spec.Cursor != "" { if err := validateDriveCursorValue(spec.Cursor, "--cursor"); err != nil { @@ -180,7 +180,7 @@ var DriveVersionHistory = common.Shortcut{ Cursor: strings.TrimSpace(runtime.Str("cursor")), } - data, err := runtime.CallAPI( + data, err := runtime.CallAPITyped( http.MethodGet, fmt.Sprintf("/open-apis/drive/v1/files/%s/history", validate.EncodePathSegment(spec.FileToken)), driveVersionHistoryParams(spec), @@ -214,7 +214,7 @@ type driveVersionGetSpec struct { func validateDriveVersionGetSpec(runtime *common.RuntimeContext, spec driveVersionGetSpec) error { if err := validate.ResourceName(spec.FileToken, "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--file-token") } if err := validateDriveVersionValue(spec.Version, "--version"); err != nil { return err @@ -223,7 +223,7 @@ func validateDriveVersionGetSpec(runtime *common.RuntimeContext, spec driveVersi return nil } if _, err := validate.SafeOutputPath(spec.Output); err != nil { - return output.ErrValidation("unsafe output path: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", err).WithParam("--output") } return nil } @@ -299,7 +299,7 @@ var DriveVersionGet = common.Shortcut{ }, }) if err != nil { - return output.ErrNetwork("download failed: %s", err) + return wrapDriveNetworkErr(err, "download failed: %s", err) } defer resp.Body.Close() @@ -315,10 +315,10 @@ var DriveVersionGet = common.Shortcut{ outputPath, _ = common.AutoAppendDownloadExtension(outputPath, resp.Header, "") } if _, resolveErr := runtime.ResolveSavePath(outputPath); resolveErr != nil { - return output.ErrValidation("unsafe output path: %s", resolveErr) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", resolveErr).WithParam("--output") } if _, statErr := runtime.FileIO().Stat(outputPath); statErr == nil && !spec.Overwrite { - return output.ErrValidation("output file already exists: %s (use --overwrite to replace)", outputPath) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "output file already exists: %s (use --overwrite to replace)", outputPath).WithParam("--output") } result, err := runtime.FileIO().Save(outputPath, fileio.SaveOptions{ @@ -326,7 +326,7 @@ var DriveVersionGet = common.Shortcut{ ContentLength: resp.ContentLength, }, resp.Body) if err != nil { - return common.WrapSaveErrorByCategory(err, "io") + return driveSaveError(err) } savedPath, _ := runtime.ResolveSavePath(outputPath) @@ -354,7 +354,7 @@ type driveVersionMutationSpec struct { func validateDriveVersionMutationSpec(spec driveVersionMutationSpec) error { if err := validate.ResourceName(spec.FileToken, "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--file-token") } return validateDriveVersionValue(spec.Version, "--version") } @@ -392,7 +392,7 @@ var DriveVersionRevert = common.Shortcut{ FileToken: strings.TrimSpace(runtime.Str("file-token")), Version: strings.TrimSpace(runtime.Str("version")), } - if _, err := runtime.CallAPI( + if _, err := runtime.CallAPITyped( http.MethodPost, fmt.Sprintf("/open-apis/drive/v1/files/%s/revert", validate.EncodePathSegment(spec.FileToken)), nil, @@ -439,7 +439,7 @@ var DriveVersionDelete = common.Shortcut{ FileToken: strings.TrimSpace(runtime.Str("file-token")), Version: strings.TrimSpace(runtime.Str("version")), } - if _, err := runtime.CallAPI( + if _, err := runtime.CallAPITyped( http.MethodPost, fmt.Sprintf("/open-apis/drive/v1/files/%s/version_del", validate.EncodePathSegment(spec.FileToken)), nil, diff --git a/shortcuts/drive/drive_version_test.go b/shortcuts/drive/drive_version_test.go index 309243ad4..34d595766 100644 --- a/shortcuts/drive/drive_version_test.go +++ b/shortcuts/drive/drive_version_test.go @@ -5,14 +5,17 @@ package drive import ( "encoding/json" + "errors" "net/http" "os" "path/filepath" "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/httpmock" + "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -53,6 +56,16 @@ func TestValidateDriveVersionHistorySpec(t *testing.T) { if err == nil || !strings.Contains(err.Error(), tt.wantErr) { t.Fatalf("expected error containing %q, got %v", tt.wantErr, err) } + var vErr *errs.ValidationError + if !errors.As(err, &vErr) { + t.Fatalf("expected *errs.ValidationError, got %T", err) + } + if vErr.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("Subtype = %q, want %q", vErr.Subtype, errs.SubtypeInvalidArgument) + } + if got := output.ExitCodeOf(err); got != output.ExitValidation { + t.Fatalf("exit code = %d, want ExitValidation (%d)", got, output.ExitValidation) + } }) } } @@ -255,6 +268,13 @@ func TestDriveVersionGetRejectsExistingFileWithoutOverwrite(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "output file already exists") { t.Fatalf("expected output exists error, got %v", err) } + var vErr *errs.ValidationError + if !errors.As(err, &vErr) { + t.Fatalf("expected *errs.ValidationError, got %T", err) + } + if vErr.Subtype != errs.SubtypeInvalidArgument || vErr.Param != "--output" { + t.Fatalf("typed shape = subtype %q param %q, want invalid_argument/--output", vErr.Subtype, vErr.Param) + } } func TestDriveVersionGetOverwritesExistingFileWhenRequested(t *testing.T) { diff --git a/shortcuts/drive/list_remote.go b/shortcuts/drive/list_remote.go index 5f773b02c..2eee7c62c 100644 --- a/shortcuts/drive/list_remote.go +++ b/shortcuts/drive/list_remote.go @@ -11,9 +11,10 @@ import ( "path" "sort" "strconv" + "strings" "time" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -85,7 +86,7 @@ func listRemoteFolderEntries(ctx context.Context, runtime *common.RuntimeContext if pageToken != "" { params["page_token"] = pageToken } - result, err := runtime.CallAPI("GET", "/open-apis/drive/v1/files", params, nil) + result, err := runtime.CallAPITyped("GET", "/open-apis/drive/v1/files", params, nil) if err != nil { return nil, err } @@ -176,24 +177,27 @@ func duplicateRemoteFilePaths(entries []driveRemoteEntry) []driveDuplicateRemote return duplicates } -// Deprecated: duplicateRemotePathError produces a legacy *output.ExitError -// that predates the typed error contract introduced by errs/. New code MUST -// NOT use it — duplicate-path signals should move to a typed -// *errs.ValidationError (with duplicates metadata as a typed extension -// field) when the drive shortcut migrates to typed errors. This helper is -// retained only while existing call sites are migrated; it will be removed -// once they have moved to the typed surface. -func duplicateRemotePathError(duplicates []driveDuplicateRemotePath) *output.ExitError { - return &output.ExitError{ - Code: output.ExitAPI, - Detail: &output.ErrDetail{ - Type: "duplicate_remote_path", - Message: "multiple Drive entries map to the same rel_path", - Detail: map[string]interface{}{ - "duplicates_remote": duplicates, - }, - }, +// duplicateRemotePathError reports that multiple Drive entries resolve to the +// same rel_path. Each colliding rel_path becomes one InvalidParam whose Name is +// the rel_path and whose Reason enumerates the colliding entries (type + +// file_token), so an AI agent reading the typed envelope can identify exactly +// which Drive objects collide without re-listing the folder. +func duplicateRemotePathError(duplicates []driveDuplicateRemotePath) error { + params := make([]errs.InvalidParam, 0, len(duplicates)) + for _, d := range duplicates { + descriptions := make([]string, 0, len(d.Entries)) + for _, entry := range d.Entries { + descriptions = append(descriptions, fmt.Sprintf("%s %s", entry.Type, entry.FileToken)) + } + params = append(params, errs.InvalidParam{ + Name: d.RelPath, + Reason: fmt.Sprintf("%d Drive entries collide here: %s", len(d.Entries), strings.Join(descriptions, ", ")), + }) } + return errs.NewValidationError(errs.SubtypeFailedPrecondition, + "%d rel_path(s) map to multiple Drive entries", len(duplicates)). + WithHint("resolve the duplicate remote files first: re-run +pull with --on-duplicate-remote=rename (downloads each with a hashed suffix), or use --on-duplicate-remote=newest|oldest (supported by +pull/+sync/+push) to pick one, or delete the extra remote files; a plain retry will not help"). + WithParams(params...) } const ( @@ -300,7 +304,7 @@ func compareDriveRemoteModifiedToLocal(remoteModified string, local time.Time) ( func chooseRemoteFile(files []driveRemoteEntry, strategy string) (driveRemoteEntry, error) { if len(files) == 0 { - return driveRemoteEntry{}, fmt.Errorf("no Drive entries available for strategy %q", strategy) + return driveRemoteEntry{}, errs.NewInternalError(errs.SubtypeUnknown, "no Drive entries available for strategy %q", strategy) } candidates := append([]driveRemoteEntry(nil), files...) sortRemoteFiles(candidates, strategy) @@ -385,7 +389,7 @@ func relPathWithUniqueFileTokenSuffix(relPath, fileToken string, occupied map[st return candidate, nil } } - return "", fmt.Errorf("could not generate a unique rel_path for %q after %d attempts", relPath, driveUniqueSuffixMaxSeq) + return "", errs.NewInternalError(errs.SubtypeUnknown, "could not generate a unique rel_path for %q after %d attempts", relPath, driveUniqueSuffixMaxSeq) } // joinRelDrive joins a rel_path base with an entry name using "/". diff --git a/tests/cli_e2e/drive/drive_duplicate_sync_workflow_test.go b/tests/cli_e2e/drive/drive_duplicate_sync_workflow_test.go index 9946a99a1..531044941 100644 --- a/tests/cli_e2e/drive/drive_duplicate_sync_workflow_test.go +++ b/tests/cli_e2e/drive/drive_duplicate_sync_workflow_test.go @@ -74,8 +74,9 @@ func TestDrive_DuplicateRemoteWorkflow(t *testing.T) { if statusResult.ExitCode == 0 { t.Fatalf("+status should fail on duplicate remote rel_path\nstdout:\n%s\nstderr:\n%s", statusResult.Stdout, statusResult.Stderr) } - if !strings.Contains(statusResult.Stderr, `"type": "duplicate_remote_path"`) { - t.Fatalf("+status stderr should contain duplicate_remote_path\nstdout:\n%s\nstderr:\n%s", statusResult.Stdout, statusResult.Stderr) + if !strings.Contains(statusResult.Stderr, `"type": "validation"`) || + !strings.Contains(statusResult.Stderr, "map to multiple Drive entries") { + t.Fatalf("+status stderr should be a typed validation error for duplicate rel_path\nstdout:\n%s\nstderr:\n%s", statusResult.Stdout, statusResult.Stderr) } pullFailResult, err := clie2e.RunCmd(ctx, clie2e.Request{ @@ -91,8 +92,9 @@ func TestDrive_DuplicateRemoteWorkflow(t *testing.T) { if pullFailResult.ExitCode == 0 { t.Fatalf("+pull should fail on duplicate remote rel_path by default\nstdout:\n%s\nstderr:\n%s", pullFailResult.Stdout, pullFailResult.Stderr) } - if !strings.Contains(pullFailResult.Stderr, `"type": "duplicate_remote_path"`) { - t.Fatalf("+pull stderr should contain duplicate_remote_path\nstdout:\n%s\nstderr:\n%s", pullFailResult.Stdout, pullFailResult.Stderr) + if !strings.Contains(pullFailResult.Stderr, `"type": "validation"`) || + !strings.Contains(pullFailResult.Stderr, "map to multiple Drive entries") { + t.Fatalf("+pull stderr should be a typed validation error for duplicate rel_path\nstdout:\n%s\nstderr:\n%s", pullFailResult.Stdout, pullFailResult.Stderr) } if _, statErr := os.Stat(filepath.Join(workDir, "local", "dup.txt")); !os.IsNotExist(statErr) { t.Fatalf("default duplicate failure must not write dup.txt; stat err=%v", statErr)