From c09814eb8f1019ba313da53276194a2807d782ca Mon Sep 17 00:00:00 2001 From: "zhaoyukun.yk" Date: Tue, 9 Jun 2026 14:58:37 +0800 Subject: [PATCH] feat(markdown): emit typed error envelopes across the markdown domain Emit structured validation, API, network, file, and internal error envelopes for Markdown shortcuts so users and agents can recover from failed markdown workflows using stable type, subtype, param, and code fields. Add Markdown 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 | 19 ++++ shortcuts/common/runner.go | 26 +---- shortcuts/markdown/helpers.go | 54 ++++----- shortcuts/markdown/markdown_diff.go | 36 +++--- shortcuts/markdown/markdown_diff_test.go | 103 +++++++++++++++--- shortcuts/markdown/markdown_errors.go | 53 +++++++++ shortcuts/markdown/markdown_fetch.go | 13 +-- shortcuts/markdown/markdown_overwrite.go | 3 +- shortcuts/markdown/markdown_patch.go | 15 ++- shortcuts/markdown/markdown_test.go | 67 ++++++++++++ 13 files changed, 291 insertions(+), 106 deletions(-) create mode 100644 shortcuts/markdown/markdown_errors.go 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) {