From 201e3e016f89b71886f307cbc5a0745751d5950d Mon Sep 17 00:00:00 2001 From: evandance <120630830+evandance@users.noreply.github.com> Date: Tue, 9 Jun 2026 20:43:20 +0800 Subject: [PATCH] feat(doc): emit typed error envelopes across the doc domain (#1346) Emit structured validation, API, network, file, and internal error envelopes for Doc shortcuts so users and agents can recover from failed document workflows using stable type, subtype, param, and code fields. Add Doc domain errscontract and golangci guards to prevent legacy envelope and common helper regressions. --- .golangci.yml | 6 +- .../rule_no_legacy_common_helper_call.go | 1 + .../rule_no_legacy_envelope_literal.go | 1 + lint/errscontract/rules_test.go | 18 + shortcuts/doc/clipboard.go | 30 +- shortcuts/doc/doc_errors.go | 34 ++ shortcuts/doc/doc_errors_test.go | 420 ++++++++++++++++++ shortcuts/doc/doc_media_download.go | 14 +- shortcuts/doc/doc_media_insert.go | 103 +++-- shortcuts/doc/doc_media_preview.go | 14 +- shortcuts/doc/doc_media_upload.go | 6 +- shortcuts/doc/docs_create_v2.go | 8 +- shortcuts/doc/docs_fetch_v2.go | 22 +- shortcuts/doc/docs_search.go | 13 +- shortcuts/doc/docs_update_v2.go | 33 +- shortcuts/doc/helpers.go | 16 +- shortcuts/doc/v2_only.go | 14 +- 17 files changed, 630 insertions(+), 123 deletions(-) create mode 100644 shortcuts/doc/doc_errors.go create mode 100644 shortcuts/doc/doc_errors_test.go diff --git a/.golangci.yml b/.golangci.yml index bddedb88..2e52c666 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -73,20 +73,20 @@ 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/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|internal/event/consume/|cmd/event/|events/|shortcuts/event/) + - 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/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|internal/event/consume/|cmd/event/|events/|shortcuts/event/) 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/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/common/mcp_client\.go|cmd/event/|events/|shortcuts/event/) + - path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/common/mcp_client\.go|cmd/event/|events/|shortcuts/event/) text: errs-no-bare-wrap linters: - forbidigo # errs-no-legacy-helper enforced on domains whose shared validation/save # helpers have migrated to typed final errors. - - path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|cmd/event/|events/|shortcuts/event/) + - path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|cmd/event/|events/|shortcuts/event/) text: errs-no-legacy-helper linters: - forbidigo diff --git a/lint/errscontract/rule_no_legacy_common_helper_call.go b/lint/errscontract/rule_no_legacy_common_helper_call.go index 7a655820..a1e404ff 100644 --- a/lint/errscontract/rule_no_legacy_common_helper_call.go +++ b/lint/errscontract/rule_no_legacy_common_helper_call.go @@ -21,6 +21,7 @@ var migratedCommonHelperPaths = []string{ "shortcuts/base/", "shortcuts/calendar/", "shortcuts/contact/", + "shortcuts/doc/", "shortcuts/drive/", "shortcuts/event/", "shortcuts/mail/", diff --git a/lint/errscontract/rule_no_legacy_envelope_literal.go b/lint/errscontract/rule_no_legacy_envelope_literal.go index bfaba9ff..e75bb3fc 100644 --- a/lint/errscontract/rule_no_legacy_envelope_literal.go +++ b/lint/errscontract/rule_no_legacy_envelope_literal.go @@ -22,6 +22,7 @@ var migratedEnvelopePaths = []string{ "shortcuts/base/", "shortcuts/calendar/", "shortcuts/contact/", + "shortcuts/doc/", "shortcuts/drive/", "shortcuts/event/", "shortcuts/mail/", diff --git a/lint/errscontract/rules_test.go b/lint/errscontract/rules_test.go index 84535a7d..410bb6f8 100644 --- a/lint/errscontract/rules_test.go +++ b/lint/errscontract/rules_test.go @@ -950,6 +950,7 @@ func TestCheckNoLegacyCommonHelperCall_RejectsLegacyHelpersOnMigratedPath(t *tes "HandleApiResult", } paths := []string{ + "shortcuts/doc/docs_fetch_v2.go", "shortcuts/drive/drive_search.go", "shortcuts/mail/mail_send.go", "shortcuts/okr/okr_progress_create.go", @@ -1003,6 +1004,23 @@ func boom() { } } +func TestCheckNoLegacyCommonHelperCall_CoversDocPathWithAliasAndFunctionValue(t *testing.T) { + src := `package migrated + +import c "github.com/larksuite/cli/shortcuts/common" + +func boom() { + f := c.FlagErrorf + _ = f + c.WrapInputStatError(nil) +} +` + v := CheckNoLegacyCommonHelperCall("shortcuts/doc/docs_fetch_v2.go", src) + if len(v) != 2 { + t.Fatalf("expected 2 violations for aliased/function-value legacy helpers on doc path, got %d: %+v", len(v), v) + } +} + func TestCheckNoLegacyCommonHelperCall_AllowsNonMigratedPath(t *testing.T) { src := `package contact diff --git a/shortcuts/doc/clipboard.go b/shortcuts/doc/clipboard.go index cb9f2c22..c7b8ed87 100644 --- a/shortcuts/doc/clipboard.go +++ b/shortcuts/doc/clipboard.go @@ -11,6 +11,8 @@ import ( "regexp" "runtime" "strings" + + "github.com/larksuite/cli/errs" ) // readClipboardImageBytes reads the current clipboard image and returns the @@ -35,13 +37,13 @@ func readClipboardImageBytes() ([]byte, error) { case "linux": data, err = readClipboardLinux() default: - return nil, fmt.Errorf("clipboard image upload is not supported on %s", runtime.GOOS) + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "clipboard image upload is not supported on %s", runtime.GOOS) } if err != nil { return nil, err } if len(data) == 0 { - return nil, fmt.Errorf("clipboard contains no image data") + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "clipboard contains no image data") } return data, nil } @@ -91,9 +93,9 @@ func readClipboardDarwin() ([]byte, error) { } if stderrText != "" { - return nil, fmt.Errorf("clipboard contains no image data (osascript: %s)", stderrText) + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "clipboard contains no image data (osascript: %s)", stderrText) } - return nil, fmt.Errorf("clipboard contains no image data") + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "clipboard contains no image data") } // runOsascript invokes osascript with a single AppleScript expression and @@ -188,14 +190,14 @@ func decodeOsascriptData(s string) ([]byte, error) { // decodeHex decodes an uppercase hex string (as produced by osascript) to bytes. func decodeHex(h string) ([]byte, error) { if len(h)%2 != 0 { - return nil, fmt.Errorf("odd hex length") + return nil, fmt.Errorf("odd hex length") //nolint:forbidigo // intermediate decode helper; result discarded by caller on error } b := make([]byte, len(h)/2) for i := 0; i < len(h); i += 2 { hi := hexVal(h[i]) lo := hexVal(h[i+1]) if hi < 0 || lo < 0 { - return nil, fmt.Errorf("invalid hex char at %d", i) + return nil, fmt.Errorf("invalid hex char at %d", i) //nolint:forbidigo // intermediate decode helper; result discarded by caller on error } b[i/2] = byte(hi<<4 | lo) } @@ -237,12 +239,12 @@ $img.Save($ms, [System.Drawing.Imaging.ImageFormat]::Png) if msg == "" { msg = err.Error() } - return nil, fmt.Errorf("clipboard read failed (%s)", msg) + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "clipboard read failed (%s)", msg).WithCause(err) } b64 := strings.TrimSpace(string(out)) data, decErr := base64.StdEncoding.DecodeString(b64) if decErr != nil { - return nil, fmt.Errorf("clipboard image decode failed: %w", decErr) + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "clipboard image decode failed: %s", decErr).WithCause(decErr) } return data, nil } @@ -325,15 +327,15 @@ func readClipboardLinux() ([]byte, error) { foundTool = true out, err := exec.Command(t.name, t.args...).Output() if err != nil { - lastErr = fmt.Errorf("clipboard image read failed via %s: %w", t.name, err) + lastErr = errs.NewValidationError(errs.SubtypeFailedPrecondition, "clipboard image read failed via %s: %s", t.name, err).WithCause(err) continue } if len(out) == 0 { - lastErr = fmt.Errorf("clipboard contains no image data (%s returned empty output)", t.name) + lastErr = errs.NewValidationError(errs.SubtypeFailedPrecondition, "clipboard contains no image data (%s returned empty output)", t.name) continue } if t.validatePNG && !hasPNGMagic(out) { - lastErr = fmt.Errorf("clipboard contains no PNG image data (%s output is not a PNG)", t.name) + lastErr = errs.NewValidationError(errs.SubtypeFailedPrecondition, "clipboard contains no PNG image data (%s output is not a PNG)", t.name) continue } return out, nil @@ -342,8 +344,8 @@ func readClipboardLinux() ([]byte, error) { if foundTool && lastErr != nil { return nil, lastErr } - return nil, fmt.Errorf( - "clipboard image read failed: no supported tool found. " + - "Install one of xclip, wl-clipboard, or xsel via your distro's package manager " + + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, + "clipboard image read failed: no supported tool found. "+ + "Install one of xclip, wl-clipboard, or xsel via your distro's package manager "+ "(apt, dnf, pacman, apk, brew, etc.).") } diff --git a/shortcuts/doc/doc_errors.go b/shortcuts/doc/doc_errors.go new file mode 100644 index 00000000..770351a8 --- /dev/null +++ b/shortcuts/doc/doc_errors.go @@ -0,0 +1,34 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package doc + +import ( + "errors" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/shortcuts/common" +) + +// wrapDocNetworkErr 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 wrapDocNetworkErr(err error, format string, args ...any) error { + if _, ok := errs.ProblemOf(err); ok { + return err + } + return errs.NewNetworkError(errs.SubtypeNetworkTransport, format, args...).WithCause(err) +} + +// wrapDocInputFileErr wraps a --file Stat/read failure via the shared typed +// helper (which sets the cause) and tags it with the --file param so agents +// learn which flag to fix. The common helper is flag-agnostic, so the param is +// attached here at the Doc call site rather than mutating shared behavior. +func wrapDocInputFileErr(err error, readMsg string) error { + wrapped := common.WrapInputStatErrorTyped(err, readMsg) + var ve *errs.ValidationError + if errors.As(wrapped, &ve) { + ve.Param = "--file" + } + return wrapped +} diff --git a/shortcuts/doc/doc_errors_test.go b/shortcuts/doc/doc_errors_test.go new file mode 100644 index 00000000..56871a87 --- /dev/null +++ b/shortcuts/doc/doc_errors_test.go @@ -0,0 +1,420 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package doc + +import ( + "context" + "errors" + "slices" + "strconv" + "testing" + + "github.com/spf13/cobra" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/shortcuts/common" +) + +// testDocxToken is a bare docx token that parseDocumentRef accepts, letting the +// validation tests reach the flag checks that run after --doc is resolved. +const testDocxToken = "doxcnDocErrorsTestToken" + +// docValidateRuntime builds a RuntimeContext carrying only the flags a Doc +// Validate function reads. String values are applied (and marked Changed) only +// when non-empty; int values are always applied so Changed() reports true, +// mirroring how cobra records an explicitly supplied numeric flag. +func docValidateRuntime(t *testing.T, str map[string]string, bools map[string]bool, ints map[string]int) *common.RuntimeContext { + t.Helper() + cmd := &cobra.Command{Use: "docs"} + fs := cmd.Flags() + for name, val := range str { + fs.String(name, "", "") + if val != "" { + if err := fs.Set(name, val); err != nil { + t.Fatalf("set --%s=%q: %v", name, val, err) + } + } + } + for name, val := range bools { + fs.Bool(name, false, "") + if val { + if err := fs.Set(name, "true"); err != nil { + t.Fatalf("set --%s: %v", name, err) + } + } + } + for name, val := range ints { + fs.Int(name, 0, "") + if err := fs.Set(name, strconv.Itoa(val)); err != nil { + t.Fatalf("set --%s=%d: %v", name, val, err) + } + } + return common.TestNewRuntimeContext(cmd, nil) +} + +// assertValidationContract pins the typed envelope every migrated Doc +// validation fault must emit: a *errs.ValidationError in CategoryValidation +// with the expected Subtype, the single offending flag in Param, and every +// involved flag in Params. Single-flag faults set Param and leave Params empty; +// multi-flag faults (mutual exclusion, "one of A or B") leave Param empty and +// enumerate each flag in Params so agents resolve them without parsing the text. +func assertValidationContract(t *testing.T, err error, wantSubtype errs.Subtype, wantParam string, wantParams ...string) { + t.Helper() + if err == nil { + t.Fatal("expected validation error, got nil") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error type = %T, want *errs.ValidationError (%v)", err, err) + } + if ve.Category != errs.CategoryValidation { + t.Errorf("category = %q, want %q", ve.Category, errs.CategoryValidation) + } + if ve.Subtype != wantSubtype { + t.Errorf("subtype = %q, want %q", ve.Subtype, wantSubtype) + } + if ve.Param != wantParam { + t.Errorf("param = %q, want %q", ve.Param, wantParam) + } + gotParams := make([]string, len(ve.Params)) + for i, p := range ve.Params { + gotParams[i] = p.Name + } + if !slices.Equal(gotParams, wantParams) { + t.Errorf("params = %v, want %v", gotParams, wantParams) + } +} + +func TestDocMediaInsertValidateContract(t *testing.T) { + cases := []struct { + name string + str map[string]string + bools map[string]bool + ints map[string]int + wantParam string + wantParams []string + }{ + { + name: "neither file nor clipboard", + str: map[string]string{"doc": testDocxToken}, + wantParam: "", // one-of-two flags: enumerated in Params + wantParams: []string{"--file", "--from-clipboard"}, + }, + { + name: "file and clipboard together", + str: map[string]string{"doc": testDocxToken, "file": "dummy.png"}, + bools: map[string]bool{"from-clipboard": true}, + wantParam: "", // mutual exclusion: enumerated in Params + wantParams: []string{"--file", "--from-clipboard"}, + }, + { + name: "non-docx document", + str: map[string]string{"doc": "https://example.larksuite.com/doc/xxxxxx", "file": "dummy.png"}, + wantParam: "--doc", + }, + { + name: "blank selection", + str: map[string]string{"doc": testDocxToken, "file": "dummy.png", "selection-with-ellipsis": " "}, + wantParam: "--selection-with-ellipsis", + }, + { + name: "before without selection", + str: map[string]string{"doc": testDocxToken, "file": "dummy.png"}, + bools: map[string]bool{"before": true}, + wantParam: "--before", + }, + { + name: "invalid file-view", + str: map[string]string{"doc": testDocxToken, "file": "dummy.png", "file-view": "bogus"}, + wantParam: "--file-view", + }, + { + name: "file-view without type file", + str: map[string]string{"doc": testDocxToken, "file": "dummy.png", "file-view": "card", "type": "image"}, + wantParam: "--file-view", + }, + { + name: "dimensions with non-image type", + str: map[string]string{"doc": testDocxToken, "file": "dummy.png", "type": "file"}, + ints: map[string]int{"width": 100}, + wantParam: "", // only --width was set here, so only it is enumerated + wantParams: []string{"--width"}, + }, + { + name: "non-positive width", + str: map[string]string{"doc": testDocxToken, "file": "dummy.png", "type": "image"}, + ints: map[string]int{"width": 0}, + wantParam: "--width", + }, + { + name: "non-positive height", + str: map[string]string{"doc": testDocxToken, "file": "dummy.png", "type": "image"}, + ints: map[string]int{"height": 0}, + wantParam: "--height", + }, + { + name: "width over maximum", + str: map[string]string{"doc": testDocxToken, "file": "dummy.png", "type": "image"}, + ints: map[string]int{"width": 10001}, + wantParam: "--width", + }, + { + name: "height over maximum", + str: map[string]string{"doc": testDocxToken, "file": "dummy.png", "type": "image"}, + ints: map[string]int{"height": 10001}, + wantParam: "--height", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + rt := docValidateRuntime(t, tc.str, tc.bools, tc.ints) + err := DocMediaInsert.Validate(context.Background(), rt) + assertValidationContract(t, err, errs.SubtypeInvalidArgument, tc.wantParam, tc.wantParams...) + }) + } +} + +func TestValidateCreateV2Contract(t *testing.T) { + cases := []struct { + name string + str map[string]string + wantParam string + wantParams []string + }{ + { + name: "content required", + str: map[string]string{}, + wantParam: "--content", + }, + { + name: "parent token and position mutually exclusive", + str: map[string]string{"content": "", "parent-token": "fldcnX", "parent-position": "my_library"}, + wantParam: "", // mutual exclusion: enumerated in Params + wantParams: []string{"--parent-token", "--parent-position"}, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + rt := docValidateRuntime(t, tc.str, nil, nil) + err := validateCreateV2(context.Background(), rt) + assertValidationContract(t, err, errs.SubtypeInvalidArgument, tc.wantParam, tc.wantParams...) + }) + } +} + +func TestValidateFetchV2Contract(t *testing.T) { + cases := []struct { + name string + str map[string]string + ints map[string]int + wantParam string + wantParams []string + }{ + { + name: "range mode without block ids", + str: map[string]string{"doc": testDocxToken, "detail": "simple", "scope": "range"}, + wantParam: "", // either --start-block-id or --end-block-id: enumerated in Params + wantParams: []string{"--start-block-id", "--end-block-id"}, + }, + { + name: "keyword mode without keyword", + str: map[string]string{"doc": testDocxToken, "detail": "simple", "scope": "keyword"}, + wantParam: "--keyword", + }, + { + name: "section mode without start block id", + str: map[string]string{"doc": testDocxToken, "detail": "simple", "scope": "section"}, + wantParam: "--start-block-id", + }, + { + name: "negative context-before", + str: map[string]string{"doc": testDocxToken, "detail": "simple", "scope": "outline"}, + ints: map[string]int{"context-before": -1}, + wantParam: "--context-before", + }, + { + name: "unknown scope", + str: map[string]string{"doc": testDocxToken, "detail": "simple", "scope": "bogus"}, + wantParam: "--scope", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + rt := docValidateRuntime(t, tc.str, nil, tc.ints) + err := validateFetchV2(context.Background(), rt) + assertValidationContract(t, err, errs.SubtypeInvalidArgument, tc.wantParam, tc.wantParams...) + }) + } +} + +// TestBuildDocsSearchRequestPreservesParseCause pins the --filter parse faults: +// the typed envelope carries Param --filter and chains the original parse error +// so errors.Is/Unwrap traversal keeps the underlying JSON/time-parse detail. +func TestBuildDocsSearchRequestPreservesParseCause(t *testing.T) { + cases := []struct { + name string + filter string + }{ + {"invalid filter json", "{not json"}, + {"invalid open_time start", `{"open_time":{"start":"not-a-time"}}`}, + {"invalid open_time end", `{"open_time":{"end":"not-a-time"}}`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := buildDocsSearchRequest("q", tc.filter, "", "15") + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error type = %T, want *errs.ValidationError (%v)", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != "--filter" { + t.Errorf("param = %q, want %q", ve.Param, "--filter") + } + if errors.Unwrap(ve) == nil { + t.Error("parse error not chained: errors.Unwrap == nil") + } + }) + } +} + +// TestWrapDocNetworkErr pins wrapDocNetworkErr's contract: a typed error passes +// through untouched, while a raw error becomes a transport-level NetworkError +// that still chains the original cause for errors.Is/Unwrap. +func TestWrapDocNetworkErr(t *testing.T) { + t.Run("typed error passes through unchanged", func(t *testing.T) { + typed := errs.NewValidationError(errs.SubtypeInvalidArgument, "bad input") + got := wrapDocNetworkErr(typed, "fetch failed") + if got != error(typed) { + t.Fatalf("typed error must pass through unchanged, got %T", got) + } + }) + t.Run("raw error becomes transport network error", func(t *testing.T) { + raw := errors.New("dial tcp: i/o timeout") + got := wrapDocNetworkErr(raw, "fetch failed: %s", "docx") + var ne *errs.NetworkError + if !errors.As(got, &ne) { + t.Fatalf("raw error must become *errs.NetworkError, got %T", got) + } + if ne.Subtype != errs.SubtypeNetworkTransport { + t.Errorf("subtype = %q, want %q", ne.Subtype, errs.SubtypeNetworkTransport) + } + if !errors.Is(got, raw) { + t.Error("cause not chained: errors.Is(got, raw) == false") + } + }) +} + +// TestWrapDocInputFileErr pins that a --file stat/read failure becomes a typed +// validation error tagged with the --file param and the cause preserved, so an +// agent knows which flag to fix even though the shared helper is flag-agnostic. +func TestWrapDocInputFileErr(t *testing.T) { + raw := errors.New("no such file or directory") + got := wrapDocInputFileErr(raw, "file not found") + var ve *errs.ValidationError + if !errors.As(got, &ve) { + t.Fatalf("error type = %T, want *errs.ValidationError (%v)", got, got) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != "--file" { + t.Errorf("param = %q, want %q", ve.Param, "--file") + } + if !errors.Is(got, raw) { + t.Error("cause not chained: errors.Is(got, raw) == false") + } +} + +func TestValidateUpdateV2Contract(t *testing.T) { + cases := []struct { + name string + str map[string]string + wantParam string + }{ + { + name: "command required", + str: map[string]string{"doc": testDocxToken}, + wantParam: "--command", + }, + { + name: "invalid command", + str: map[string]string{"doc": testDocxToken, "command": "bogus"}, + wantParam: "--command", + }, + { + name: "str_replace without pattern", + str: map[string]string{"doc": testDocxToken, "command": "str_replace"}, + wantParam: "--pattern", + }, + { + name: "block_delete without block id", + str: map[string]string{"doc": testDocxToken, "command": "block_delete"}, + wantParam: "--block-id", + }, + { + name: "block_insert_after without block id", + str: map[string]string{"doc": testDocxToken, "command": "block_insert_after"}, + wantParam: "--block-id", + }, + { + name: "block_insert_after without content", + str: map[string]string{"doc": testDocxToken, "command": "block_insert_after", "block-id": "blkX"}, + wantParam: "--content", + }, + { + name: "block_copy_insert_after without block id", + str: map[string]string{"doc": testDocxToken, "command": "block_copy_insert_after"}, + wantParam: "--block-id", + }, + { + name: "block_copy_insert_after without src block ids", + str: map[string]string{"doc": testDocxToken, "command": "block_copy_insert_after", "block-id": "blkX"}, + wantParam: "--src-block-ids", + }, + { + name: "block_move_after without block id", + str: map[string]string{"doc": testDocxToken, "command": "block_move_after"}, + wantParam: "--block-id", + }, + { + name: "block_move_after without src block ids", + str: map[string]string{"doc": testDocxToken, "command": "block_move_after", "block-id": "blkX"}, + wantParam: "--src-block-ids", + }, + { + name: "block_move_after rejects content", + str: map[string]string{"doc": testDocxToken, "command": "block_move_after", "block-id": "blkX", "src-block-ids": "blkY", "content": "x"}, + wantParam: "--content", + }, + { + name: "block_replace without block id", + str: map[string]string{"doc": testDocxToken, "command": "block_replace"}, + wantParam: "--block-id", + }, + { + name: "block_replace without content", + str: map[string]string{"doc": testDocxToken, "command": "block_replace", "block-id": "blkX"}, + wantParam: "--content", + }, + { + name: "overwrite without content", + str: map[string]string{"doc": testDocxToken, "command": "overwrite"}, + wantParam: "--content", + }, + { + name: "append without content", + str: map[string]string{"doc": testDocxToken, "command": "append"}, + wantParam: "--content", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + rt := docValidateRuntime(t, tc.str, nil, nil) + err := validateUpdateV2(context.Background(), rt) + assertValidationContract(t, err, errs.SubtypeInvalidArgument, tc.wantParam) + }) + } +} diff --git a/shortcuts/doc/doc_media_download.go b/shortcuts/doc/doc_media_download.go index 79a16245..1d28481f 100644 --- a/shortcuts/doc/doc_media_download.go +++ b/shortcuts/doc/doc_media_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" ) @@ -51,10 +51,10 @@ var DocMediaDownload = common.Shortcut{ overwrite := runtime.Bool("overwrite") if err := validate.ResourceName(token, "--token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--token") } if _, err := runtime.ResolveSavePath(outputPath); err != nil { - return output.ErrValidation("unsafe output path: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", err).WithParam("--output").WithCause(err) } fmt.Fprintf(runtime.IO().ErrOut, "Downloading: %s %s\n", mediaType, common.MaskToken(token)) @@ -73,7 +73,7 @@ var DocMediaDownload = common.Shortcut{ ApiPath: apiPath, }) if err != nil { - return output.ErrNetwork("download failed: %v", err) + return wrapDocNetworkErr(err, "download failed: %v", err) } defer resp.Body.Close() @@ -86,14 +86,14 @@ var DocMediaDownload = common.Shortcut{ // Validate final path after extension append if finalPath != outputPath { if _, err := runtime.ResolveSavePath(finalPath); err != nil { - return output.ErrValidation("unsafe output path: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", err).WithParam("--output").WithCause(err) } } // Overwrite check on final path (after extension detection) if !overwrite { if _, statErr := runtime.FileIO().Stat(finalPath); statErr == nil { - return output.ErrValidation("output file already exists: %s (use --overwrite to replace)", finalPath) + return errs.NewValidationError(errs.SubtypeFailedPrecondition, "output file already exists: %s (use --overwrite to replace)", finalPath).WithParam("--output") } } @@ -102,7 +102,7 @@ var DocMediaDownload = common.Shortcut{ ContentLength: resp.ContentLength, }, resp.Body) if err != nil { - return common.WrapSaveErrorByCategory(err, "io") + return common.WrapSaveErrorTyped(err) } savedPath, _ := runtime.ResolveSavePath(finalPath) diff --git a/shortcuts/doc/doc_media_insert.go b/shortcuts/doc/doc_media_insert.go index 5c31495a..495f5067 100644 --- a/shortcuts/doc/doc_media_insert.go +++ b/shortcuts/doc/doc_media_insert.go @@ -15,8 +15,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/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -67,10 +67,16 @@ var DocMediaInsert = common.Shortcut{ filePath := runtime.Str("file") fromClipboard := runtime.Bool("from-clipboard") if filePath == "" && !fromClipboard { - return common.FlagErrorf("one of --file or --from-clipboard is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "one of --file or --from-clipboard is required").WithParams( + errs.InvalidParam{Name: "--file", Reason: "provide either --file or --from-clipboard"}, + errs.InvalidParam{Name: "--from-clipboard", Reason: "provide either --file or --from-clipboard"}, + ) } if filePath != "" && fromClipboard { - return common.FlagErrorf("--file and --from-clipboard are mutually exclusive") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--file and --from-clipboard are mutually exclusive").WithParams( + errs.InvalidParam{Name: "--file", Reason: "mutually exclusive with --from-clipboard"}, + errs.InvalidParam{Name: "--from-clipboard", Reason: "mutually exclusive with --file"}, + ) } docRef, err := parseDocumentRef(runtime.Str("doc")) @@ -78,7 +84,7 @@ var DocMediaInsert = common.Shortcut{ return err } if docRef.Kind == "doc" { - return output.ErrValidation("docs +media-insert only supports docx documents; use a docx token/URL or a wiki URL that resolves to docx") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "docs +media-insert only supports docx documents; use a docx token/URL or a wiki URL that resolves to docx").WithParam("--doc") } rawSelection := runtime.Str("selection-with-ellipsis") trimmedSelection := strings.TrimSpace(rawSelection) @@ -87,36 +93,43 @@ var DocMediaInsert = common.Shortcut{ // trim-to-empty would make +media-insert fall back to append-mode and // write at the wrong location. if rawSelection != "" && trimmedSelection == "" { - return output.ErrValidation("--selection-with-ellipsis must not be blank or whitespace-only") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--selection-with-ellipsis must not be blank or whitespace-only").WithParam("--selection-with-ellipsis") } if runtime.Bool("before") && trimmedSelection == "" { - return output.ErrValidation("--before requires --selection-with-ellipsis") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--before requires --selection-with-ellipsis").WithParam("--before") } if view := runtime.Str("file-view"); view != "" { if _, ok := fileViewMap[view]; !ok { - return output.ErrValidation("invalid --file-view value %q, expected one of: card | preview | inline", view) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --file-view value %q, expected one of: card | preview | inline", view).WithParam("--file-view") } if runtime.Str("type") != "file" { - return output.ErrValidation("--file-view only applies when --type=file") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--file-view only applies when --type=file").WithParam("--file-view") } } widthChanged := runtime.Changed("width") heightChanged := runtime.Changed("height") if (widthChanged || heightChanged) && runtime.Str("type") != "image" { - return output.ErrValidation("--width/--height only apply when --type=image") + var params []errs.InvalidParam + if widthChanged { + params = append(params, errs.InvalidParam{Name: "--width", Reason: "only applies when --type=image"}) + } + if heightChanged { + params = append(params, errs.InvalidParam{Name: "--height", Reason: "only applies when --type=image"}) + } + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--width/--height only apply when --type=image").WithParams(params...) } if widthChanged && runtime.Int("width") <= 0 { - return output.ErrValidation("--width must be a positive integer") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--width must be a positive integer").WithParam("--width") } if heightChanged && runtime.Int("height") <= 0 { - return output.ErrValidation("--height must be a positive integer") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--height must be a positive integer").WithParam("--height") } const maxDimension = 10000 if widthChanged && runtime.Int("width") > maxDimension { - return output.ErrValidation("--width must not exceed %d pixels", maxDimension) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--width must not exceed %d pixels", maxDimension).WithParam("--width") } if heightChanged && runtime.Int("height") > maxDimension { - return output.ErrValidation("--height must not exceed %d pixels", maxDimension) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--height must not exceed %d pixels", maxDimension).WithParam("--height") } return nil }, @@ -269,10 +282,10 @@ var DocMediaInsert = common.Shortcut{ } else { stat, err := runtime.FileIO().Stat(filePath) if err != nil { - return common.WrapInputStatError(err, "file not found") + return wrapDocInputFileErr(err, "file not found") } if !stat.Mode().IsRegular() { - return output.ErrValidation("file must be a regular file: %s", filePath) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "file must be a regular file: %s", filePath).WithParam("--file") } fileSize = stat.Size() fileName = filepath.Base(filePath) @@ -284,7 +297,7 @@ var DocMediaInsert = common.Shortcut{ } // Step 1: Get document root block to find where to insert - rootData, err := runtime.CallAPI("GET", + rootData, err := runtime.CallAPITyped("GET", fmt.Sprintf("/open-apis/docx/v1/documents/%s/blocks/%s", validate.EncodePathSegment(documentID), validate.EncodePathSegment(documentID)), nil, nil) if err != nil { @@ -318,7 +331,7 @@ var DocMediaInsert = common.Shortcut{ // Step 2: Create an empty block at the target position fmt.Fprintf(runtime.IO().ErrOut, "Creating block at index %d\n", insertIndex) - createData, err := runtime.CallAPI("POST", + createData, err := runtime.CallAPITyped("POST", fmt.Sprintf("/open-apis/docx/v1/documents/%s/blocks/%s/children", validate.EncodePathSegment(documentID), validate.EncodePathSegment(parentBlockID)), nil, buildCreateBlockData(mediaType, insertIndex, fileViewType)) if err != nil { @@ -328,7 +341,7 @@ var DocMediaInsert = common.Shortcut{ blockId, uploadParentNode, replaceBlockID := extractCreatedBlockTargets(createData, mediaType) if blockId == "" { - return output.Errorf(output.ExitAPI, "api_error", "failed to create block: no block_id returned") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "failed to create block: no block_id returned") } fmt.Fprintf(runtime.IO().ErrOut, "Block created: %s\n", blockId) @@ -340,7 +353,7 @@ var DocMediaInsert = common.Shortcut{ // later steps should try to remove it instead of leaving an empty artifact. rollback := func() error { fmt.Fprintf(runtime.IO().ErrOut, "Rolling back: deleting block %s\n", blockId) - _, err := runtime.CallAPI("DELETE", + _, err := runtime.CallAPITyped("DELETE", fmt.Sprintf("/open-apis/docx/v1/documents/%s/blocks/%s/children/batch_delete", validate.EncodePathSegment(documentID), validate.EncodePathSegment(parentBlockID)), nil, buildDeleteBlockData(insertIndex)) return err @@ -379,15 +392,21 @@ var DocMediaInsert = common.Shortcut{ } else { f, openErr := runtime.FileIO().Open(filePath) if openErr != nil { - return withRollbackWarning(output.ErrValidation( - "unable to detect image dimensions from %s for aspect-ratio calculation; provide both --width and --height", fileName)) + return withRollbackWarning(errs.NewValidationError(errs.SubtypeInvalidArgument, + "unable to detect image dimensions from %s for aspect-ratio calculation; provide both --width and --height", fileName).WithCause(openErr).WithParams( + errs.InvalidParam{Name: "--width", Reason: "provide explicitly; source image dimensions could not be detected"}, + errs.InvalidParam{Name: "--height", Reason: "provide explicitly; source image dimensions could not be detected"}, + )) } nativeW, nativeH, dimErr = detectImageDimensions(f) f.Close() } if dimErr != nil { - return withRollbackWarning(output.ErrValidation( - "unable to detect image dimensions from %s for aspect-ratio calculation; provide both --width and --height", fileName)) + return withRollbackWarning(errs.NewValidationError(errs.SubtypeInvalidArgument, + "unable to detect image dimensions from %s for aspect-ratio calculation; provide both --width and --height", fileName).WithCause(dimErr).WithParams( + errs.InvalidParam{Name: "--width", Reason: "provide explicitly; source image dimensions could not be detected"}, + errs.InvalidParam{Name: "--height", Reason: "provide explicitly; source image dimensions could not be detected"}, + )) } dims := computeMissingDimension(userWidth, userHeight, nativeW, nativeH) finalWidth = dims.width @@ -417,7 +436,7 @@ var DocMediaInsert = common.Shortcut{ // Step 4: Bind file token to block via batch_update fmt.Fprintf(runtime.IO().ErrOut, "Binding uploaded media to block %s\n", replaceBlockID) - if _, err := runtime.CallAPI("PATCH", + if _, err := runtime.CallAPITyped("PATCH", fmt.Sprintf("/open-apis/docx/v1/documents/%s/blocks/batch_update", validate.EncodePathSegment(documentID)), nil, buildBatchUpdateData(replaceBlockID, mediaType, fileToken, alignStr, caption, finalWidth, finalHeight)); err != nil { return withRollbackWarning(err) @@ -512,10 +531,10 @@ func resolveDocxDocumentID(runtime *common.RuntimeContext, input string) (string case "docx": return docRef.Token, nil case "doc": - return "", output.ErrValidation("docs +media-insert only supports docx documents; use a docx token/URL or a wiki URL that resolves to docx") + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "docs +media-insert only supports docx documents; use a docx token/URL or a wiki URL that resolves to docx").WithParam("--doc") case "wiki": 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}, @@ -529,16 +548,16 @@ func resolveDocxDocumentID(runtime *common.RuntimeContext, input string) (string objType := common.GetString(node, "obj_type") objToken := common.GetString(node, "obj_token") if objType == "" || objToken == "" { - return "", output.Errorf(output.ExitAPI, "api_error", "wiki get_node returned incomplete node data") + return "", errs.NewInternalError(errs.SubtypeInvalidResponse, "wiki get_node returned incomplete node data") } if objType != "docx" { - return "", output.ErrValidation("wiki resolved to %q, but docs +media-insert only supports docx documents", objType) + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "wiki resolved to %q, but docs +media-insert only supports docx documents", objType).WithParam("--doc") } fmt.Fprintf(runtime.IO().ErrOut, "Resolved wiki to docx: %s\n", common.MaskToken(objToken)) return objToken, nil default: - return "", output.ErrValidation("docs +media-insert only supports docx documents") + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "docs +media-insert only supports docx documents").WithParam("--doc") } } @@ -622,7 +641,7 @@ func buildBatchUpdateData(blockID, mediaType, fileToken, alignStr, caption strin func extractAppendTarget(rootData map[string]interface{}, fallbackBlockID string) (parentBlockID string, insertIndex int, children []interface{}, err error) { block, _ := rootData["block"].(map[string]interface{}) if len(block) == 0 { - return "", 0, nil, output.Errorf(output.ExitAPI, "api_error", "failed to query document root block") + return "", 0, nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "failed to query document root block") } parentBlockID = fallbackBlockID @@ -653,12 +672,10 @@ func locateInsertIndex(runtime *common.RuntimeContext, documentID string, select matches := common.GetSlice(result, "matches") if len(matches) == 0 { - return 0, output.ErrWithHint( - output.ExitValidation, - "no_match", - fmt.Sprintf("locate-doc did not find any block matching selection (%s)", redactSelection(selection)), - "check spelling or use 'start...end' syntax to narrow the selection", - ) + return 0, errs.NewValidationError(errs.SubtypeInvalidArgument, + "locate-doc did not find any block matching selection (%s)", redactSelection(selection)). + WithParam("--selection-with-ellipsis"). + WithHint("check spelling or use 'start...end' syntax to narrow the selection") } if len(matches) > 1 { // Silently picking the first match surprises users whose selection appears @@ -682,7 +699,7 @@ func locateInsertIndex(runtime *common.RuntimeContext, documentID string, select } } if anchorBlockID == "" { - return 0, output.Errorf(output.ExitAPI, "api_error", "locate-doc response missing anchor_block_id") + return 0, errs.NewInternalError(errs.SubtypeInvalidResponse, "locate-doc response missing anchor_block_id") } parentBlockID := common.GetString(matchMap, "parent_block_id") @@ -740,7 +757,7 @@ func locateInsertIndex(runtime *common.RuntimeContext, documentID string, select nextParent = "" // clear hint after first use if parent == "" || parent == cur { // Need to fetch this block to find its parent. - data, err := runtime.CallAPI("GET", + data, err := runtime.CallAPITyped("GET", fmt.Sprintf("/open-apis/docx/v1/documents/%s/blocks/%s", validate.EncodePathSegment(documentID), validate.EncodePathSegment(cur)), nil, nil) @@ -757,12 +774,10 @@ func locateInsertIndex(runtime *common.RuntimeContext, documentID string, select walkDepth++ } - return 0, output.ErrWithHint( - output.ExitValidation, - "block_not_reachable", - fmt.Sprintf("block matching selection (%s) is not reachable from document root", redactSelection(selection)), - "try a top-level heading or paragraph as the selection", - ) + return 0, errs.NewValidationError(errs.SubtypeInvalidArgument, + "block matching selection (%s) is not reachable from document root", redactSelection(selection)). + WithParam("--selection-with-ellipsis"). + WithHint("try a top-level heading or paragraph as the selection") } func extractCreatedBlockTargets(createData map[string]interface{}, mediaType string) (blockID, uploadParentNode, replaceBlockID string) { diff --git a/shortcuts/doc/doc_media_preview.go b/shortcuts/doc/doc_media_preview.go index 5b0a7be2..6bc88d8e 100644 --- a/shortcuts/doc/doc_media_preview.go +++ b/shortcuts/doc/doc_media_preview.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" ) @@ -45,11 +45,11 @@ var DocMediaPreview = common.Shortcut{ overwrite := runtime.Bool("overwrite") if err := validate.ResourceName(token, "--token"); err != nil { - return output.ErrValidation("%s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err).WithParam("--token") } // Early path validation before API call (final validation after auto-extension below) if _, err := runtime.ResolveSavePath(outputPath); err != nil { - return output.ErrValidation("unsafe output path: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", err).WithParam("--output").WithCause(err) } fmt.Fprintf(runtime.IO().ErrOut, "Previewing: media %s\n", common.MaskToken(token)) @@ -65,7 +65,7 @@ var DocMediaPreview = common.Shortcut{ }, }) if err != nil { - return output.ErrNetwork("preview failed: %v", err) + return wrapDocNetworkErr(err, "preview failed: %v", err) } defer resp.Body.Close() @@ -74,14 +74,14 @@ var DocMediaPreview = common.Shortcut{ // Validate final path after extension append if finalPath != outputPath { if _, err := runtime.ResolveSavePath(finalPath); err != nil { - return output.ErrValidation("unsafe output path: %s", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", err).WithParam("--output").WithCause(err) } } // Overwrite check on final path (after extension detection) if !overwrite { if _, statErr := runtime.FileIO().Stat(finalPath); statErr == nil { - return output.ErrValidation("output file already exists: %s (use --overwrite to replace)", finalPath) + return errs.NewValidationError(errs.SubtypeFailedPrecondition, "output file already exists: %s (use --overwrite to replace)", finalPath).WithParam("--output") } } @@ -90,7 +90,7 @@ var DocMediaPreview = common.Shortcut{ ContentLength: resp.ContentLength, }, resp.Body) if err != nil { - return common.WrapSaveErrorByCategory(err, "io") + return common.WrapSaveErrorTyped(err) } savedPath, _ := runtime.ResolveSavePath(finalPath) diff --git a/shortcuts/doc/doc_media_upload.go b/shortcuts/doc/doc_media_upload.go index e7168c62..449dbb40 100644 --- a/shortcuts/doc/doc_media_upload.go +++ b/shortcuts/doc/doc_media_upload.go @@ -9,8 +9,8 @@ import ( "io" "path/filepath" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -84,10 +84,10 @@ var DocMediaUpload = common.Shortcut{ // Validate file stat, err := runtime.FileIO().Stat(filePath) if err != nil { - return common.WrapInputStatError(err, "file not found") + return wrapDocInputFileErr(err, "file not found") } if !stat.Mode().IsRegular() { - return output.ErrValidation("file must be a regular file: %s", filePath) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "file must be a regular file: %s", filePath).WithParam("--file") } fileName := filepath.Base(filePath) diff --git a/shortcuts/doc/docs_create_v2.go b/shortcuts/doc/docs_create_v2.go index 70d07ddc..6f0e1471 100644 --- a/shortcuts/doc/docs_create_v2.go +++ b/shortcuts/doc/docs_create_v2.go @@ -7,6 +7,7 @@ import ( "context" "strings" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -25,10 +26,13 @@ func validateCreateV2(_ context.Context, runtime *common.RuntimeContext) error { return err } if runtime.Str("content") == "" { - return common.FlagErrorf("--content is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--content is required").WithParam("--content") } if runtime.Str("parent-token") != "" && runtime.Str("parent-position") != "" { - return common.FlagErrorf("--parent-token and --parent-position are mutually exclusive") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--parent-token and --parent-position are mutually exclusive").WithParams( + errs.InvalidParam{Name: "--parent-token", Reason: "mutually exclusive with --parent-position"}, + errs.InvalidParam{Name: "--parent-position", Reason: "mutually exclusive with --parent-token"}, + ) } return nil } diff --git a/shortcuts/doc/docs_fetch_v2.go b/shortcuts/doc/docs_fetch_v2.go index 305847a2..0ab446c3 100644 --- a/shortcuts/doc/docs_fetch_v2.go +++ b/shortcuts/doc/docs_fetch_v2.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -37,7 +38,7 @@ func validateFetchV2(_ context.Context, runtime *common.RuntimeContext) error { return err } if _, err := parseDocumentRef(runtime.Str("doc")); err != nil { - return common.FlagErrorf("invalid --doc: %v", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --doc: %v", err).WithParam("--doc") } if err := validateFetchDetail(runtime); err != nil { return err @@ -153,7 +154,7 @@ func validateFetchDetail(runtime *common.RuntimeContext) error { return nil } if detail == "with-ids" || detail == "full" { - return common.FlagErrorf("--detail %s is only supported with --doc-format xml; %s output has no block ids, use --detail simple or switch to --doc-format xml", detail, format) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--detail %s is only supported with --doc-format xml; %s output has no block ids, use --detail simple or switch to --doc-format xml", detail, format).WithParam("--detail") } return nil } @@ -166,13 +167,13 @@ func validateReadModeFlags(runtime *common.RuntimeContext) error { } if v := runtime.Int("context-before"); v < 0 { - return common.FlagErrorf("--context-before must be >= 0, got %d", v) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--context-before must be >= 0, got %d", v).WithParam("--context-before") } if v := runtime.Int("context-after"); v < 0 { - return common.FlagErrorf("--context-after must be >= 0, got %d", v) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--context-after must be >= 0, got %d", v).WithParam("--context-after") } if v := runtime.Int("max-depth"); v < -1 { - return common.FlagErrorf("--max-depth must be >= -1, got %d", v) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--max-depth must be >= -1, got %d", v).WithParam("--max-depth") } switch mode { @@ -181,20 +182,23 @@ func validateReadModeFlags(runtime *common.RuntimeContext) error { case "range": if strings.TrimSpace(runtime.Str("start-block-id")) == "" && strings.TrimSpace(runtime.Str("end-block-id")) == "" { - return common.FlagErrorf("range mode requires --start-block-id or --end-block-id") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "range mode requires --start-block-id or --end-block-id").WithParams( + errs.InvalidParam{Name: "--start-block-id", Reason: "provide --start-block-id or --end-block-id for range mode"}, + errs.InvalidParam{Name: "--end-block-id", Reason: "provide --start-block-id or --end-block-id for range mode"}, + ) } return nil case "keyword": if strings.TrimSpace(runtime.Str("keyword")) == "" { - return common.FlagErrorf("keyword mode requires --keyword") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "keyword mode requires --keyword").WithParam("--keyword") } return nil case "section": if strings.TrimSpace(runtime.Str("start-block-id")) == "" { - return common.FlagErrorf("section mode requires --start-block-id") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "section mode requires --start-block-id").WithParam("--start-block-id") } return nil default: - return common.FlagErrorf("invalid --scope %q", mode) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --scope %q", mode).WithParam("--scope") } } diff --git a/shortcuts/doc/docs_search.go b/shortcuts/doc/docs_search.go index 73dfca4b..80ebf968 100644 --- a/shortcuts/doc/docs_search.go +++ b/shortcuts/doc/docs_search.go @@ -14,6 +14,7 @@ import ( "strings" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -58,7 +59,7 @@ var DocsSearch = common.Shortcut{ return err } - data, err := runtime.CallAPI("POST", "/open-apis/search/v2/doc_wiki/search", nil, requestData) + data, err := runtime.CallAPITyped("POST", "/open-apis/search/v2/doc_wiki/search", nil, requestData) if err != nil { return err } @@ -159,7 +160,7 @@ func buildDocsSearchRequest(query, filterStr, pageToken, pageSizeStr string) (ma var filter map[string]interface{} if err := json.Unmarshal([]byte(filterStr), &filter); err != nil { - return nil, output.ErrValidation("--filter is not valid JSON") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--filter is not valid JSON").WithParam("--filter").WithCause(err) } if err := convertTimeRangeInFilter(filter, "open_time"); err != nil { return nil, err @@ -172,7 +173,7 @@ func buildDocsSearchRequest(query, filterStr, pageToken, pageSizeStr string) (ma hasSpaceIDs := hasNonEmptyFilterArray(filter, "space_ids") if hasFolderTokens && hasSpaceIDs { - return nil, output.ErrValidation("--filter cannot contain both folder_tokens and space_ids; doc and wiki scoped search cannot be combined") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--filter cannot contain both folder_tokens and space_ids; doc and wiki scoped search cannot be combined").WithParam("--filter") } docFilter := cloneFilterMap(filter) @@ -225,14 +226,14 @@ func convertTimeRangeInFilter(filter map[string]interface{}, key string) error { if start, ok := rangeMap["start"].(string); ok && start != "" { startTime, err := toUnixSeconds(start) if err != nil { - return output.ErrValidation("invalid %s.start %q: %s", key, start, err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid %s.start %q: %s", key, start, err).WithParam("--filter").WithCause(err) } result["start"] = startTime } if end, ok := rangeMap["end"].(string); ok && end != "" { endTime, err := toUnixSeconds(end) if err != nil { - return output.ErrValidation("invalid %s.end %q: %s", key, end, err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid %s.end %q: %s", key, end, err).WithParam("--filter").WithCause(err) } result["end"] = endTime } @@ -256,7 +257,7 @@ func toUnixSeconds(input string) (int64, error) { if n, err := strconv.ParseInt(input, 10, 64); err == nil { return n, nil } - return 0, fmt.Errorf("expected RFC3339, YYYY-MM-DD[ HH:MM:SS], or unix seconds") + return 0, fmt.Errorf("expected RFC3339, YYYY-MM-DD[ HH:MM:SS], or unix seconds") //nolint:forbidigo // intermediate parse helper; caller wraps into typed ValidationError } func unixTimestampToISO8601(v interface{}) string { diff --git a/shortcuts/doc/docs_update_v2.go b/shortcuts/doc/docs_update_v2.go index fcc7beb1..cc75d44e 100644 --- a/shortcuts/doc/docs_update_v2.go +++ b/shortcuts/doc/docs_update_v2.go @@ -7,6 +7,7 @@ import ( "context" "fmt" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -43,14 +44,14 @@ func validateUpdateV2(_ context.Context, runtime *common.RuntimeContext) error { return err } if _, err := parseDocumentRef(runtime.Str("doc")); err != nil { - return common.FlagErrorf("invalid --doc: %v", err) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --doc: %v", err).WithParam("--doc") } cmd := runtime.Str("command") if cmd == "" { - return common.FlagErrorf("--command is required") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--command is required").WithParam("--command") } if !validCommandsV2[cmd] { - return common.FlagErrorf("invalid --command %q, valid: str_replace | block_delete | block_insert_after | block_copy_insert_after | block_replace | block_move_after | overwrite | append", cmd) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --command %q, valid: str_replace | block_delete | block_insert_after | block_copy_insert_after | block_replace | block_move_after | overwrite | append", cmd).WithParam("--command") } content := runtime.Str("content") pattern := runtime.Str("pattern") @@ -60,50 +61,50 @@ func validateUpdateV2(_ context.Context, runtime *common.RuntimeContext) error { switch cmd { case "str_replace": if pattern == "" { - return common.FlagErrorf("--command str_replace requires --pattern") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--command str_replace requires --pattern").WithParam("--pattern") } case "block_delete": if blockID == "" { - return common.FlagErrorf("--command block_delete requires --block-id") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--command block_delete requires --block-id").WithParam("--block-id") } case "block_insert_after": if blockID == "" { - return common.FlagErrorf("--command block_insert_after requires --block-id") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--command block_insert_after requires --block-id").WithParam("--block-id") } if content == "" { - return common.FlagErrorf("--command block_insert_after requires --content") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--command block_insert_after requires --content").WithParam("--content") } case "block_copy_insert_after": if blockID == "" { - return common.FlagErrorf("--command block_copy_insert_after requires --block-id") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--command block_copy_insert_after requires --block-id").WithParam("--block-id") } if srcBlockIDs == "" { - return common.FlagErrorf("--command block_copy_insert_after requires --src-block-ids") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--command block_copy_insert_after requires --src-block-ids").WithParam("--src-block-ids") } case "block_move_after": if blockID == "" { - return common.FlagErrorf("--command block_move_after requires --block-id") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--command block_move_after requires --block-id").WithParam("--block-id") } if srcBlockIDs == "" { - return common.FlagErrorf("--command block_move_after requires --src-block-ids") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--command block_move_after requires --src-block-ids").WithParam("--src-block-ids") } if content != "" { - return common.FlagErrorf("--command block_move_after does not accept --content; use --src-block-ids") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--command block_move_after does not accept --content; use --src-block-ids").WithParam("--content") } case "block_replace": if blockID == "" { - return common.FlagErrorf("--command block_replace requires --block-id") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--command block_replace requires --block-id").WithParam("--block-id") } if content == "" { - return common.FlagErrorf("--command block_replace requires --content") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--command block_replace requires --content").WithParam("--content") } case "overwrite": if content == "" { - return common.FlagErrorf("--command overwrite requires --content") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--command overwrite requires --content").WithParam("--content") } case "append": if content == "" { - return common.FlagErrorf("--command append requires --content") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "--command append requires --content").WithParam("--content") } } return nil diff --git a/shortcuts/doc/helpers.go b/shortcuts/doc/helpers.go index c3446d3b..4305e9ba 100644 --- a/shortcuts/doc/helpers.go +++ b/shortcuts/doc/helpers.go @@ -8,7 +8,7 @@ import ( "encoding/json" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -24,7 +24,7 @@ type documentRef struct { func parseDocumentRef(input string) (documentRef, error) { raw := strings.TrimSpace(input) if raw == "" { - return documentRef{}, output.ErrValidation("--doc cannot be empty") + return documentRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "--doc cannot be empty").WithParam("--doc") } if token, ok := extractDocumentToken(raw, "/wiki/"); ok { @@ -37,10 +37,10 @@ func parseDocumentRef(input string) (documentRef, error) { return documentRef{Kind: "doc", Token: token}, nil } if strings.Contains(raw, "://") { - return documentRef{}, output.ErrValidation("unsupported --doc input %q: use a docx URL/token or a wiki URL that resolves to docx", raw) + return documentRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported --doc input %q: use a docx URL/token or a wiki URL that resolves to docx", raw).WithParam("--doc") } if strings.ContainsAny(raw, "/?#") { - return documentRef{}, output.ErrValidation("unsupported --doc input %q: use a docx token or a wiki URL", raw) + return documentRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported --doc input %q: use a docx token or a wiki URL", raw).WithParam("--doc") } return documentRef{Kind: "docx", Token: raw}, nil @@ -64,10 +64,10 @@ func extractDocumentToken(raw, marker string) (string, bool) { // doDocAPI executes an OpenAPI request against the docs_ai endpoints and returns // the parsed "data" field from the standard Lark response envelope {code, msg, data}. -// Uses the log-id-aware variant so the x-tt-logid header is surfaced in both the -// success payload and error details — doc v2 callers rely on it for support escalations. +// CallAPITyped lifts the x-tt-logid response header onto the typed error so log_id +// surfaces for support escalations even when the body omits it. func doDocAPI(runtime *common.RuntimeContext, method, apiPath string, body interface{}) (map[string]interface{}, error) { - return runtime.DoAPIJSONWithLogID(method, apiPath, nil, body) + return runtime.CallAPITyped(method, apiPath, nil, body) } func docsSceneFromContext(ctx context.Context) string { @@ -87,7 +87,7 @@ func injectDocsScene(runtime *common.RuntimeContext, body map[string]interface{} func buildDriveRouteExtra(docID string) (string, error) { extra, err := json.Marshal(map[string]string{"drive_route_token": docID}) if err != nil { - return "", output.Errorf(output.ExitInternal, "internal_error", "failed to marshal upload extra data: %v", err) + return "", errs.NewInternalError(errs.SubtypeUnknown, "failed to marshal upload extra data: %v", err).WithCause(err) } return string(extra), nil } diff --git a/shortcuts/doc/v2_only.go b/shortcuts/doc/v2_only.go index e7485780..1d051516 100644 --- a/shortcuts/doc/v2_only.go +++ b/shortcuts/doc/v2_only.go @@ -6,6 +6,7 @@ package doc import ( "strings" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -65,7 +66,7 @@ func validateDocsV2Only(runtime *common.RuntimeContext, shortcut string, legacyF switch apiVersion := strings.TrimSpace(runtime.Str("api-version")); apiVersion { case "", "v1", "v2": default: - return docsV2OnlyError(shortcut, "--api-version is deprecated and only accepts v1 or v2; both values execute the v2 API") + return docsV2OnlyError(shortcut, "--api-version is deprecated and only accepts v1 or v2; both values execute the v2 API", "--api-version") } var used []string @@ -87,11 +88,12 @@ func validateDocsV2Only(runtime *common.RuntimeContext, shortcut string, legacyF if len(replacements) > 0 { detail += "; " + strings.Join(replacements, "; ") } - return docsV2OnlyError(shortcut, detail) + return docsV2OnlyError(shortcut, detail, used[0]) } -func docsV2OnlyError(shortcut, detail string) error { - return common.FlagErrorf( +func docsV2OnlyError(shortcut, detail, param string) error { + err := errs.NewValidationError( + errs.SubtypeInvalidArgument, "docs %s is v2-only; %s. Run `%s` for the current schema and examples. AI agents MUST read `%s` (XML) or `%s` (Markdown) and follow the latest format rules there. MUST NOT grep/open local SKILL.md files to discover this guidance; use `lark-cli skills read ...` so content stays version-matched with this CLI. Run `%s` for the latest command flags", shortcut, detail, @@ -100,4 +102,8 @@ func docsV2OnlyError(shortcut, detail string) error { docsMDSkillReadCommand, docsHelpCommandForShortcut(shortcut), ) + if param != "" { + err = err.WithParam(param) + } + return err }