diff --git a/.golangci.yml b/.golangci.yml index 4e3b6c42f..7caac0c68 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/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|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/markdown/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|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/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|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/markdown/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|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/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|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/markdown/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|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 5f096bc56..1d5336f58 100644 --- a/lint/errscontract/rule_no_legacy_common_helper_call.go +++ b/lint/errscontract/rule_no_legacy_common_helper_call.go @@ -25,6 +25,7 @@ var migratedCommonHelperPaths = []string{ "shortcuts/drive/", "shortcuts/event/", "shortcuts/mail/", + "shortcuts/markdown/", "shortcuts/minutes/", "shortcuts/okr/", "shortcuts/sheets/", diff --git a/lint/errscontract/rule_no_legacy_envelope_literal.go b/lint/errscontract/rule_no_legacy_envelope_literal.go index ef2a3e167..03be7c3af 100644 --- a/lint/errscontract/rule_no_legacy_envelope_literal.go +++ b/lint/errscontract/rule_no_legacy_envelope_literal.go @@ -26,6 +26,7 @@ var migratedEnvelopePaths = []string{ "shortcuts/drive/", "shortcuts/event/", "shortcuts/mail/", + "shortcuts/markdown/", "shortcuts/minutes/", "shortcuts/okr/", "shortcuts/sheets/", diff --git a/lint/errscontract/rules_test.go b/lint/errscontract/rules_test.go index 94cf58113..d736a6a36 100644 --- a/lint/errscontract/rules_test.go +++ b/lint/errscontract/rules_test.go @@ -620,6 +620,7 @@ func boom() error { func TestCheckNoLegacyEnvelopeLiteral_RejectsExitErrorLiteralOnMigratedShortcutPaths(t *testing.T) { for _, path := range []string{ + "shortcuts/markdown/markdown_fetch.go", "shortcuts/okr/okr_image_upload.go", "shortcuts/task/task_update.go", "shortcuts/whiteboard/whiteboard_update.go", @@ -953,6 +954,7 @@ func TestCheckNoLegacyCommonHelperCall_RejectsLegacyHelpersOnMigratedPath(t *tes "shortcuts/doc/docs_fetch_v2.go", "shortcuts/drive/drive_search.go", "shortcuts/mail/mail_send.go", + "shortcuts/markdown/markdown_fetch.go", "shortcuts/okr/okr_progress_create.go", "shortcuts/sheets/helpers.go", "shortcuts/slides/slides_create.go", @@ -1057,6 +1059,23 @@ func boom() { } } +func TestCheckNoLegacyCommonHelperCall_CoversMarkdownPathWithAliasAndFunctionValue(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/markdown/markdown_fetch.go", src) + if len(v) != 2 { + t.Fatalf("expected 2 violations for aliased/function-value legacy helpers on markdown path, got %d: %+v", len(v), v) + } +} + func TestCheckNoLegacyCommonHelperCall_AllowsNonMigratedPath(t *testing.T) { src := `package contact diff --git a/shortcuts/common/runner.go b/shortcuts/common/runner.go index 59be2040f..e3365d2f5 100644 --- a/shortcuts/common/runner.go +++ b/shortcuts/common/runner.go @@ -676,30 +676,10 @@ func WrapInputStatErrorTyped(err error, readMsg ...string) error { WithCause(err) } -// WrapSaveErrorByCategory maps a FileIO.Save error to structured output errors, -// using standardized messages and the given error category (e.g. "api_error", "io"). -// Path validation errors always use ErrValidation (exit code 2). -// -// Deprecated: use WrapSaveErrorTyped for typed error envelopes. -func WrapSaveErrorByCategory(err error, category string) error { - if err == nil { - return nil - } - var me *fileio.MkdirError - switch { - case errors.Is(err, fileio.ErrPathValidation): - return output.ErrValidation("unsafe output path: %s", err) - case errors.As(err, &me): - return output.Errorf(output.ExitInternal, category, "cannot create parent directory: %s", err) - default: - return output.Errorf(output.ExitInternal, category, "cannot create file: %s", err) - } -} - // WrapSaveErrorTyped maps a FileIO.Save error to typed validation/internal errors. -// Unlike WrapSaveErrorByCategory, non-path failures always emit the canonical -// "internal" wire type: call sites migrating from a custom category -// (e.g. "io", "api_error") change their envelope's type field. +// Non-path failures always emit the canonical "internal" wire type; call sites +// migrating from a custom category (e.g. "io", "api_error") change their +// envelope's type field. func WrapSaveErrorTyped(err error) error { if err == nil { return nil diff --git a/shortcuts/markdown/helpers.go b/shortcuts/markdown/helpers.go index da60e5053..aa792fb4c 100644 --- a/shortcuts/markdown/helpers.go +++ b/shortcuts/markdown/helpers.go @@ -6,7 +6,6 @@ package markdown import ( "bytes" "context" - "errors" "fmt" "io" "net/http" @@ -19,7 +18,6 @@ import ( "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/client" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -85,16 +83,18 @@ func (spec markdownUploadSpec) Target() markdownUploadTarget { func validateMarkdownSpec(runtime *common.RuntimeContext, spec markdownUploadSpec, requireName bool) error { switch { case spec.ContentSet && spec.FileSet: - return common.FlagErrorf("--content and --file are mutually exclusive") + return markdownValidationError("--content and --file are mutually exclusive"). + WithParams(markdownInvalidParam("--content", "mutually exclusive"), markdownInvalidParam("--file", "mutually exclusive")) case !spec.ContentSet && !spec.FileSet: - return common.FlagErrorf("specify exactly one of --content or --file") + return markdownValidationError("specify exactly one of --content or --file"). + WithParams(markdownInvalidParam("--content", "required; specify exactly one"), markdownInvalidParam("--file", "required; specify exactly one")) } if markdownFlagExplicitlyEmpty(runtime, "folder-token") { - return common.FlagErrorf("--folder-token cannot be empty; omit it to upload into Drive root folder") + return markdownValidationParamError("--folder-token", "--folder-token cannot be empty; omit it to upload into Drive root folder") } if markdownFlagExplicitlyEmpty(runtime, "wiki-token") { - return common.FlagErrorf("--wiki-token cannot be empty; provide a valid wiki node token or omit the flag entirely") + return markdownValidationParamError("--wiki-token", "--wiki-token cannot be empty; provide a valid wiki node token or omit the flag entirely") } targets := 0 if spec.FolderToken != "" { @@ -104,22 +104,23 @@ func validateMarkdownSpec(runtime *common.RuntimeContext, spec markdownUploadSpe targets++ } if targets > 1 { - return common.FlagErrorf("--folder-token and --wiki-token are mutually exclusive") + return markdownValidationError("--folder-token and --wiki-token are mutually exclusive"). + WithParams(markdownInvalidParam("--folder-token", "mutually exclusive"), markdownInvalidParam("--wiki-token", "mutually exclusive")) } if spec.FolderToken != "" { if err := validate.ResourceName(spec.FolderToken, "--folder-token"); err != nil { - return output.ErrValidation("%s", err) + return markdownValidationParamError("--folder-token", "%s", err).WithCause(err) } } if spec.WikiToken != "" { if err := validate.ResourceName(spec.WikiToken, "--wiki-token"); err != nil { - return output.ErrValidation("%s", err) + return markdownValidationParamError("--wiki-token", "%s", err).WithCause(err) } } if requireName && spec.ContentSet { if strings.TrimSpace(spec.FileName) == "" { - return common.FlagErrorf("--name is required when using --content") + return markdownValidationParamError("--name", "--name is required when using --content") } if err := validateMarkdownFileName(spec.FileName, "--name"); err != nil { return err @@ -128,10 +129,10 @@ func validateMarkdownSpec(runtime *common.RuntimeContext, spec markdownUploadSpe if spec.FileSet { if strings.TrimSpace(spec.FilePath) == "" { - return common.FlagErrorf("--file cannot be empty") + return markdownValidationParamError("--file", "--file cannot be empty") } if _, err := validate.SafeInputPath(spec.FilePath); err != nil { - return output.ErrValidation("unsafe file path: %s", err) + return markdownValidationParamError("--file", "unsafe file path: %s", err).WithCause(err) } if err := validateMarkdownFileName(filepath.Base(spec.FilePath), "--file"); err != nil { return err @@ -154,10 +155,10 @@ func markdownFlagExplicitlyEmpty(runtime *common.RuntimeContext, flagName string func validateMarkdownFileName(name, flagName string) error { trimmed := strings.TrimSpace(name) if trimmed == "" { - return common.FlagErrorf("%s cannot be empty", flagName) + return markdownValidationParamError(flagName, "%s cannot be empty", flagName) } if !strings.HasSuffix(strings.ToLower(trimmed), ".md") { - return common.FlagErrorf("%s must end with .md", flagName) + return markdownValidationParamError(flagName, "%s must end with .md", flagName) } return nil } @@ -201,22 +202,9 @@ func openMarkdownDownload(ctx context.Context, runtime *common.RuntimeContext, f return resp, nil } -func wrapMarkdownDownloadError(err error) error { - // Preserve any already-classified error: legacy *output.ExitError or any - // typed errs.* error. Only un-classified errors get wrapped as network. - var exitErr *output.ExitError - if errors.As(err, &exitErr) { - return err - } - if _, ok := errs.ProblemOf(err); ok { - return err - } - return output.ErrNetwork("download failed: %s", err) -} - func validateNonEmptyMarkdownSize(size int64) error { if size == 0 { - return output.ErrValidation("%s", markdownEmptyContentError) + return markdownValidationError("%s", markdownEmptyContentError) } return nil } @@ -227,12 +215,12 @@ func markdownSourceSize(runtime *common.RuntimeContext, spec markdownUploadSpec) size = int64(len(spec.Content)) } else { if strings.TrimSpace(spec.FilePath) == "" { - return 0, common.FlagErrorf("--file cannot be empty") + return 0, markdownValidationParamError("--file", "--file cannot be empty") } info, err := runtime.FileIO().Stat(spec.FilePath) if err != nil { - return 0, common.WrapInputStatError(err) + return 0, withMarkdownFileParam(common.WrapInputStatErrorTyped(err), "--file") } size = info.Size() } @@ -424,7 +412,7 @@ func uploadMarkdownFileAll(runtime *common.RuntimeContext, spec markdownUploadSp return withMarkdownUploadRetryResult(runtime, markdownUploadAllAction, func() (markdownUploadResult, error) { fileReader, err := openReader() if err != nil { - return markdownUploadResult{}, common.WrapInputStatErrorTyped(err) + return markdownUploadResult{}, withMarkdownFileParam(common.WrapInputStatErrorTyped(err), "--file") } defer fileReader.Close() @@ -491,7 +479,7 @@ func uploadMarkdownFileMultipart(runtime *common.RuntimeContext, spec markdownUp fileReader, err := openReader() if err != nil { - return markdownUploadResult{}, common.WrapInputStatErrorTyped(err) + return markdownUploadResult{}, withMarkdownFileParam(common.WrapInputStatErrorTyped(err), "--file") } defer fileReader.Close() @@ -563,7 +551,7 @@ func uploadMarkdownMultipartParts(runtime *common.RuntimeContext, fileReader io. n, readErr := io.ReadFull(fileReader, buffer[:int(chunkSize)]) if readErr != nil { - return output.ErrValidation("cannot read file: %s", readErr) + return markdownValidationParamError("--file", "cannot read file: %s", readErr).WithCause(readErr) } fd := larkcore.NewFormdata() diff --git a/shortcuts/markdown/markdown_diff.go b/shortcuts/markdown/markdown_diff.go index 21d5ca337..218c8ff95 100644 --- a/shortcuts/markdown/markdown_diff.go +++ b/shortcuts/markdown/markdown_diff.go @@ -5,7 +5,6 @@ package markdown import ( "context" - "errors" "fmt" "io" "regexp" @@ -14,6 +13,7 @@ import ( "github.com/sergi/go-diff/diffmatchpatch" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" @@ -65,7 +65,7 @@ type markdownDiffHunkRange struct { func validateMarkdownDiffSpec(runtime *common.RuntimeContext, spec markdownDiffSpec) error { if err := validate.ResourceName(spec.FileToken, "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return markdownValidationParamError("--file-token", "%s", err).WithCause(err) } if spec.FromVersion != "" { if err := validateMarkdownDiffVersionValue(spec.FromVersion, "--from-version"); err != nil { @@ -79,29 +79,34 @@ func validateMarkdownDiffSpec(runtime *common.RuntimeContext, spec markdownDiffS } if spec.FilePath != "" { if _, err := validate.SafeInputPath(spec.FilePath); err != nil { - return output.ErrValidation("unsafe file path: %s", err) + return markdownValidationParamError("--file", "unsafe file path: %s", err).WithCause(err) } if err := validateMarkdownFileName(spec.FilePath, "--file"); err != nil { return err } } if spec.ContextLines < 0 { - return output.ErrValidation("--context-lines must be >= 0") + return markdownValidationParamError("--context-lines", "--context-lines must be >= 0") } if spec.Format != "" && spec.Format != "json" && spec.Format != "pretty" { - return output.ErrValidation("markdown +diff only supports --format json or pretty") + return markdownValidationParamError("--format", "markdown +diff only supports --format json or pretty") } if spec.FilePath == "" { if spec.FromVersion == "" && spec.ToVersion == "" { - return common.FlagErrorf("specify --from-version, or both --from-version and --to-version, or use --file for remote vs local diff") + return markdownValidationError("specify --from-version, or both --from-version and --to-version, or use --file for remote vs local diff"). + WithParams( + markdownInvalidParam("--from-version", "required; specify one"), + markdownInvalidParam("--to-version", "required; specify one"), + markdownInvalidParam("--file", "required; specify one"), + ) } if spec.FromVersion == "" && spec.ToVersion != "" { - return common.FlagErrorf("--to-version requires --from-version") + return markdownValidationParamError("--to-version", "--to-version requires --from-version") } return nil } if spec.ToVersion != "" { - return common.FlagErrorf("--to-version is not supported together with --file") + return markdownValidationParamError("--to-version", "--to-version is not supported together with --file") } return nil } @@ -109,10 +114,10 @@ func validateMarkdownDiffSpec(runtime *common.RuntimeContext, spec markdownDiffS func validateMarkdownDiffVersionValue(value, flagName string) error { value = strings.TrimSpace(value) if value == "" { - return output.ErrValidation("%s cannot be empty", flagName) + return markdownValidationParamError(flagName, "%s cannot be empty", flagName) } if !markdownDiffVersionRe.MatchString(value) { - return output.ErrValidation("%s must be a numeric version string", flagName) + return markdownValidationParamError(flagName, "%s must be a numeric version string", flagName) } return nil } @@ -178,17 +183,16 @@ func downloadMarkdownContent(ctx context.Context, runtime *common.RuntimeContext func readMarkdownLocalFile(runtime *common.RuntimeContext, filePath string) (string, error) { f, err := runtime.FileIO().Open(filePath) if err != nil { - return "", common.WrapInputStatError(err) + return "", withMarkdownFileParam(common.WrapInputStatErrorTyped(err), "--file") } defer f.Close() payload, err := readMarkdownDiffPayload(f, "local Markdown file") if err != nil { - var exitErr *output.ExitError - if errors.As(err, &exitErr) { - return "", err + if _, ok := errs.ProblemOf(err); ok { + return "", withMarkdownFileParam(err, "--file") } - return "", output.ErrValidation("cannot read file: %s", err) + return "", markdownValidationParamError("--file", "cannot read file: %s", err).WithCause(err) } return string(payload), nil } @@ -199,7 +203,7 @@ func readMarkdownDiffPayload(r io.Reader, source string) ([]byte, error) { return nil, err } if len(payload) > markdownDiffMaxContentBytes { - return nil, output.ErrValidation("%s exceeds %s markdown +diff content limit", source, common.FormatSize(markdownDiffMaxContentBytes)) + return nil, markdownValidationError("%s exceeds %s markdown +diff content limit", source, common.FormatSize(markdownDiffMaxContentBytes)) } return payload, nil } diff --git a/shortcuts/markdown/markdown_diff_test.go b/shortcuts/markdown/markdown_diff_test.go index b870778e4..21a708615 100644 --- a/shortcuts/markdown/markdown_diff_test.go +++ b/shortcuts/markdown/markdown_diff_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" @@ -47,6 +48,32 @@ func TestMarkdownDiffRejectsToVersionWithoutFromVersion(t *testing.T) { } } +func TestMarkdownDiffMissingVersionAndFileNamesCandidateParams(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + f, stdout, _, _ := cmdutil.TestFactory(t, markdownTestConfig()) + + err := mountAndRunMarkdown(t, MarkdownDiff, []string{ + "+diff", + "--file-token", "box_md_diff", + }, f, stdout) + if err == nil { + t.Fatal("expected validation error when no version or file source is given") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T (%v)", err, err) + } + got := map[string]bool{} + for _, p := range ve.Params { + got[p.Name] = true + } + for _, want := range []string{"--from-version", "--to-version", "--file"} { + if !got[want] { + t.Fatalf("params %+v missing candidate %q", ve.Params, want) + } + } +} + func TestMarkdownDiffRemoteVsRemoteJSON(t *testing.T) { t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) f, stdout, _, reg := cmdutil.TestFactory(t, markdownTestConfig()) @@ -211,24 +238,72 @@ func TestMarkdownDiffRejectsOversizedLocalContent(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "local Markdown file exceeds 10.0 MB markdown +diff content limit") { t.Fatalf("expected local content size error, got %v", err) } + requireMarkdownValidationParam(t, err, "--file") } -func TestMarkdownDownloadErrorPreservesStructuredErrors(t *testing.T) { - apiErr := output.ErrAPI(99991663, "permission denied", map[string]interface{}{"permission": "drive:file:download"}) - if got := wrapMarkdownDownloadError(apiErr); got != apiErr { - t.Fatalf("wrapMarkdownDownloadError() = %v, want original API error", got) - } +func TestWrapMarkdownDownloadError(t *testing.T) { + cause := errors.New("dial tcp timeout") - got := wrapMarkdownDownloadError(errors.New("dial tcp timeout")) - var exitErr *output.ExitError - if !errors.As(got, &exitErr) { - t.Fatalf("wrapMarkdownDownloadError() = %T, want *output.ExitError", got) - } - if exitErr.Code != output.ExitNetwork { - t.Fatalf("exit code = %d, want %d", exitErr.Code, output.ExitNetwork) + tests := []struct { + name string + in error + wantSame bool // result must be the same error value (carrier preserved) + wantMessage string + wantCat errs.Category + wantSubtype errs.Subtype + wantCode int + wantCause error // when set, errors.Is(result, wantCause) must hold + }{ + { + name: "non-validation typed error keeps carrier and gains prefix", + in: errs.NewAPIError(errs.SubtypePermissionDenied, "permission denied").WithCode(99991663), + wantSame: true, + wantMessage: "download failed: permission denied", + wantCat: errs.CategoryAPI, + wantSubtype: errs.SubtypePermissionDenied, + wantCode: 99991663, + }, + { + name: "validation error passes through verbatim", + in: markdownValidationError("invalid markdown content"), + wantSame: true, + wantMessage: "invalid markdown content", + wantCat: errs.CategoryValidation, + wantSubtype: errs.SubtypeInvalidArgument, + }, + { + name: "untyped error becomes a network transport error", + in: cause, + wantMessage: "download failed: dial tcp timeout", + wantCat: errs.CategoryNetwork, + wantSubtype: errs.SubtypeNetworkTransport, + wantCause: cause, + }, } - if !strings.Contains(got.Error(), "download failed: dial tcp timeout") { - t.Fatalf("wrapped error = %q", got.Error()) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := wrapMarkdownDownloadError(tt.in) + if tt.wantSame && got != tt.in { + t.Fatalf("wrapMarkdownDownloadError() returned a new error; want the original carrier preserved") + } + if got.Error() != tt.wantMessage { + t.Fatalf("message = %q, want %q", got.Error(), tt.wantMessage) + } + problem, ok := errs.ProblemOf(got) + if !ok { + t.Fatalf("wrapMarkdownDownloadError() = %T, want a typed problem", got) + } + if problem.Category != tt.wantCat || problem.Subtype != tt.wantSubtype { + t.Fatalf("classification = %s/%s, want %s/%s", problem.Category, problem.Subtype, tt.wantCat, tt.wantSubtype) + } + if tt.wantCode != 0 && problem.Code != tt.wantCode { + t.Fatalf("code = %d, want %d", problem.Code, tt.wantCode) + } + if tt.wantCause != nil && !errors.Is(got, tt.wantCause) { + t.Fatalf("wrapped error does not unwrap to its cause") + } + }) } } diff --git a/shortcuts/markdown/markdown_errors.go b/shortcuts/markdown/markdown_errors.go new file mode 100644 index 000000000..7db4cf8b6 --- /dev/null +++ b/shortcuts/markdown/markdown_errors.go @@ -0,0 +1,53 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package markdown + +import ( + "errors" + + "github.com/larksuite/cli/errs" +) + +func markdownValidationError(format string, args ...any) *errs.ValidationError { + return errs.NewValidationError(errs.SubtypeInvalidArgument, format, args...) +} + +func markdownValidationParamError(param, format string, args ...any) *errs.ValidationError { + return markdownValidationError(format, args...).WithParam(param) +} + +func markdownInvalidParam(name, reason string) errs.InvalidParam { + return errs.InvalidParam{Name: name, Reason: reason} +} + +// withMarkdownFileParam tags a validation failure with the originating flag +// when it does not already name one. Shared input-file helpers such as +// common.WrapInputStatErrorTyped cannot know which flag supplied the path, so +// the caller attaches it here to keep the recoverable param on the wire. +func withMarkdownFileParam(err error, param string) error { + if err == nil || param == "" { + return err + } + var ve *errs.ValidationError + if errors.As(err, &ve) && ve.Param == "" { + ve.WithParam(param) + } + return err +} + +// wrapMarkdownDownloadError classifies a download failure. An already-typed +// error keeps its carrier — type, subtype, code and extensions — so callers see +// the upstream classification: a validation problem passes through verbatim, +// any other problem gains a "download failed" prefix for operation context. +// An untyped error becomes a network transport error carrying the original as +// its cause. +func wrapMarkdownDownloadError(err error) error { + if p, ok := errs.ProblemOf(err); ok { + if p.Category != errs.CategoryValidation { + p.Message = "download failed: " + p.Message + } + return err + } + return errs.NewNetworkError(errs.SubtypeNetworkTransport, "download failed: %s", err).WithCause(err) +} diff --git a/shortcuts/markdown/markdown_fetch.go b/shortcuts/markdown/markdown_fetch.go index cfc9d8a85..e93423b81 100644 --- a/shortcuts/markdown/markdown_fetch.go +++ b/shortcuts/markdown/markdown_fetch.go @@ -14,7 +14,6 @@ import ( larkcore "github.com/larksuite/oapi-sdk-go/v3/core" "github.com/larksuite/cli/extension/fileio" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -35,14 +34,14 @@ var MarkdownFetch = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { fileToken := strings.TrimSpace(runtime.Str("file-token")) if err := validate.ResourceName(fileToken, "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return markdownValidationParamError("--file-token", "%s", err).WithCause(err) } outputPath := strings.TrimSpace(runtime.Str("output")) if outputPath == "" { return nil } if _, err := validate.SafeOutputPath(outputPath); err != nil { - return output.ErrValidation("unsafe output path: %s", err) + return markdownValidationParamError("--output", "unsafe output path: %s", err).WithCause(err) } return nil }, @@ -67,7 +66,7 @@ var MarkdownFetch = 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 wrapMarkdownDownloadError(err) } defer resp.Body.Close() @@ -75,7 +74,7 @@ var MarkdownFetch = common.Shortcut{ if outputPath == "" { payload, err := io.ReadAll(resp.Body) if err != nil { - return output.ErrNetwork("download failed: %s", err) + return wrapMarkdownDownloadError(err) } out := map[string]interface{}{ "file_token": fileToken, @@ -93,7 +92,7 @@ var MarkdownFetch = common.Shortcut{ outputPath = filepath.Join(outputPath, fileName) } if _, statErr := runtime.FileIO().Stat(outputPath); statErr == nil && !runtime.Bool("overwrite") { - return output.ErrValidation("output file already exists: %s (use --overwrite to replace)", outputPath) + return markdownValidationParamError("--output", "output file already exists: %s (use --overwrite to replace)", outputPath) } result, err := runtime.FileIO().Save(outputPath, fileio.SaveOptions{ @@ -101,7 +100,7 @@ var MarkdownFetch = common.Shortcut{ ContentLength: resp.ContentLength, }, resp.Body) if err != nil { - return common.WrapSaveErrorByCategory(err, "io") + return common.WrapSaveErrorTyped(err) } savedPath, _ := runtime.ResolveSavePath(outputPath) diff --git a/shortcuts/markdown/markdown_overwrite.go b/shortcuts/markdown/markdown_overwrite.go index 648124e40..ff6584c02 100644 --- a/shortcuts/markdown/markdown_overwrite.go +++ b/shortcuts/markdown/markdown_overwrite.go @@ -8,7 +8,6 @@ import ( "io" "strings" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -30,7 +29,7 @@ var MarkdownOverwrite = common.Shortcut{ Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { fileToken := strings.TrimSpace(runtime.Str("file-token")) if err := validate.ResourceName(fileToken, "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return markdownValidationParamError("--file-token", "%s", err).WithCause(err) } return validateMarkdownSpec(runtime, markdownUploadSpec{ FileToken: fileToken, diff --git a/shortcuts/markdown/markdown_patch.go b/shortcuts/markdown/markdown_patch.go index 95cbe7aa7..5e1ceff17 100644 --- a/shortcuts/markdown/markdown_patch.go +++ b/shortcuts/markdown/markdown_patch.go @@ -10,7 +10,6 @@ import ( "regexp" "strings" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -49,7 +48,7 @@ var MarkdownPatch = common.Shortcut{ } if spec.Regex { if _, err := regexp.Compile(spec.Pattern); err != nil { - return output.ErrValidation("invalid --pattern regex: %s", err) + return markdownValidationParamError("--pattern", "invalid --pattern regex: %s", err).WithCause(err) } } return nil @@ -122,7 +121,7 @@ var MarkdownPatch = common.Shortcut{ payload, err := io.ReadAll(resp.Body) if err != nil { - return output.ErrNetwork("download failed: %s", err) + return wrapMarkdownDownloadError(err) } original := string(payload) patched, matchCount, err := applyMarkdownPatch(original, spec) @@ -192,16 +191,16 @@ func newMarkdownPatchSpec(runtime *common.RuntimeContext) markdownPatchSpec { func validateMarkdownPatchSpec(runtime *common.RuntimeContext, spec markdownPatchSpec) error { if err := validate.ResourceName(spec.FileToken, "--file-token"); err != nil { - return output.ErrValidation("%s", err) + return markdownValidationParamError("--file-token", "%s", err).WithCause(err) } if !runtime.Changed("pattern") { - return common.FlagErrorf("--pattern is required") + return markdownValidationParamError("--pattern", "--pattern is required") } if spec.Pattern == "" { - return output.ErrValidation("--pattern cannot be empty") + return markdownValidationParamError("--pattern", "--pattern cannot be empty") } if !spec.ContentSet { - return common.FlagErrorf("--content is required") + return markdownValidationParamError("--content", "--content is required") } return nil } @@ -212,7 +211,7 @@ func applyMarkdownPatch(original string, spec markdownPatchSpec) (string, int, e } re, err := regexp.Compile(spec.Pattern) if err != nil { - return "", 0, output.ErrValidation("invalid --pattern regex: %s", err) + return "", 0, markdownValidationParamError("--pattern", "invalid --pattern regex: %s", err).WithCause(err) } matches := re.FindAllStringIndex(original, -1) return re.ReplaceAllString(original, spec.Content), len(matches), nil diff --git a/shortcuts/markdown/markdown_test.go b/shortcuts/markdown/markdown_test.go index f4358ba54..25a7687f2 100644 --- a/shortcuts/markdown/markdown_test.go +++ b/shortcuts/markdown/markdown_test.go @@ -858,6 +858,41 @@ func TestMarkdownCreateBotAutoGrantFailed(t *testing.T) { } } +// requireMarkdownValidationParam asserts err is a typed validation envelope +// (category + subtype) whose recoverable Param names the expected flag. It does +// not assert a cause: Param-tagged validation failures such as the +diff content +// limit carry no underlying error. +func requireMarkdownValidationParam(t *testing.T, err error, want string) { + t.Helper() + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed problem, got %T (%v)", err, err) + } + if p.Category != errs.CategoryValidation || p.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("classification = %s/%s, want %s/%s", p.Category, p.Subtype, errs.CategoryValidation, errs.SubtypeInvalidArgument) + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T (%v)", err, err) + } + if ve.Param != want { + t.Fatalf("validation param = %q, want %q", ve.Param, want) + } +} + +// requireMarkdownValidationParamWithCause is requireMarkdownValidationParam for +// file open/read failures, which wrap the underlying os error via WithCause. It +// additionally enforces that the cause is preserved. Validation failures that +// carry no underlying error (e.g. the +diff content limit) use the plain helper. +func requireMarkdownValidationParamWithCause(t *testing.T, err error, want string) { + t.Helper() + requireMarkdownValidationParam(t, err, want) + var ve *errs.ValidationError + if !errors.As(err, &ve) || ve.Cause == nil { + t.Fatalf("expected validation cause to be preserved, got %T (%v)", err, err) + } +} + func TestMarkdownCreateMissingFileReturnsReadError(t *testing.T) { f, stdout, _, _ := cmdutil.TestFactory(t, markdownTestConfig()) @@ -868,6 +903,7 @@ func TestMarkdownCreateMissingFileReturnsReadError(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "cannot read file") { t.Fatalf("expected cannot read file error, got %v", err) } + requireMarkdownValidationParamWithCause(t, err, "--file") } func TestMarkdownCreateMultipartUploadSuccess(t *testing.T) { @@ -1286,6 +1322,36 @@ func TestUploadMarkdownFileAllMissingFileTokenGetsActionPrefix(t *testing.T) { } } +func TestUploadMarkdownFileMultipartOpenFailureNamesFileParam(t *testing.T) { + f, _, _, reg := cmdutil.TestFactory(t, markdownTestConfig()) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/drive/v1/files/upload_prepare", + Body: map[string]interface{}{ + "code": 0, + "data": map[string]interface{}{ + "upload_id": "upload_123", + "block_size": 4194304, + "block_num": 1, + }, + }, + }) + + _, err := uploadMarkdownFileMultipart( + common.TestNewRuntimeContextForAPI(context.Background(), &cobra.Command{Use: "+create"}, markdownTestConfig(), f, core.AsUser), + markdownUploadSpec{FileSet: true, FilePath: "missing.md"}, + "missing.md", + int64(1), + func() (io.ReadCloser, error) { + return nil, errors.New("open missing.md: no such file") + }, + ) + if err == nil { + t.Fatal("expected open failure after prepare, got nil") + } + requireMarkdownValidationParamWithCause(t, err, "--file") +} + func TestUploadMarkdownFileMultipartPrepareAndFinishParseErrorsGetActionPrefix(t *testing.T) { t.Run("prepare", func(t *testing.T) { f, _, _, reg := cmdutil.TestFactory(t, markdownTestConfig()) @@ -1710,6 +1776,7 @@ func TestMarkdownOverwriteMissingFileReturnsReadError(t *testing.T) { if err == nil || !strings.Contains(err.Error(), "cannot read file") { t.Fatalf("expected cannot read file error, got %v", err) } + requireMarkdownValidationParamWithCause(t, err, "--file") } func TestMarkdownOverwritePrettyOutputUsesDataVersionFallback(t *testing.T) {