Skip to content

Commit 04472c9

Browse files
committed
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.
1 parent b07be60 commit 04472c9

13 files changed

Lines changed: 291 additions & 106 deletions

.golangci.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,20 @@ linters:
7373
- forbidigo
7474
# errs-typed-only enforced on paths already migrated to errs.NewXxxError.
7575
# Add a path when its migration is complete.
76-
- 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/task/|shortcuts/vc/|shortcuts/whiteboard/|internal/event/consume/|cmd/event/|events/|shortcuts/event/)
76+
- 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/task/|shortcuts/vc/|shortcuts/whiteboard/|internal/event/consume/|cmd/event/|events/|shortcuts/event/)
7777
text: errs-typed-only
7878
linters:
7979
- forbidigo
8080
# errs-no-bare-wrap enforced on paths fully migrated to typed final
8181
# errors. Scoped separately from errs-typed-only because cmd/auth/,
8282
# cmd/config/ still have residual fmt.Errorf and must not be caught.
83-
- path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/common/mcp_client\.go|cmd/event/|events/|shortcuts/event/)
83+
- 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/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/common/mcp_client\.go|cmd/event/|events/|shortcuts/event/)
8484
text: errs-no-bare-wrap
8585
linters:
8686
- forbidigo
8787
# errs-no-legacy-helper enforced on domains whose shared validation/save
8888
# helpers have migrated to typed final errors.
89-
- path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|cmd/event/|events/|shortcuts/event/)
89+
- 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/task/|shortcuts/vc/|shortcuts/whiteboard/|cmd/event/|events/|shortcuts/event/)
9090
text: errs-no-legacy-helper
9191
linters:
9292
- forbidigo

lint/errscontract/rule_no_legacy_common_helper_call.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ var migratedCommonHelperPaths = []string{
2525
"shortcuts/drive/",
2626
"shortcuts/event/",
2727
"shortcuts/mail/",
28+
"shortcuts/markdown/",
2829
"shortcuts/minutes/",
2930
"shortcuts/okr/",
3031
"shortcuts/sheets/",

lint/errscontract/rule_no_legacy_envelope_literal.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ var migratedEnvelopePaths = []string{
2626
"shortcuts/drive/",
2727
"shortcuts/event/",
2828
"shortcuts/mail/",
29+
"shortcuts/markdown/",
2930
"shortcuts/minutes/",
3031
"shortcuts/okr/",
3132
"shortcuts/sheets/",

lint/errscontract/rules_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,7 @@ func boom() error {
620620

621621
func TestCheckNoLegacyEnvelopeLiteral_RejectsExitErrorLiteralOnMigratedShortcutPaths(t *testing.T) {
622622
for _, path := range []string{
623+
"shortcuts/markdown/markdown_fetch.go",
623624
"shortcuts/okr/okr_image_upload.go",
624625
"shortcuts/task/task_update.go",
625626
"shortcuts/whiteboard/whiteboard_update.go",
@@ -953,6 +954,7 @@ func TestCheckNoLegacyCommonHelperCall_RejectsLegacyHelpersOnMigratedPath(t *tes
953954
"shortcuts/doc/docs_fetch_v2.go",
954955
"shortcuts/drive/drive_search.go",
955956
"shortcuts/mail/mail_send.go",
957+
"shortcuts/markdown/markdown_fetch.go",
956958
"shortcuts/okr/okr_progress_create.go",
957959
"shortcuts/sheets/helpers.go",
958960
"shortcuts/task/task_update.go",
@@ -1039,6 +1041,23 @@ func boom() {
10391041
}
10401042
}
10411043

1044+
func TestCheckNoLegacyCommonHelperCall_CoversMarkdownPathWithAliasAndFunctionValue(t *testing.T) {
1045+
src := `package migrated
1046+
1047+
import c "github.com/larksuite/cli/shortcuts/common"
1048+
1049+
func boom() {
1050+
f := c.FlagErrorf
1051+
_ = f
1052+
c.WrapInputStatError(nil)
1053+
}
1054+
`
1055+
v := CheckNoLegacyCommonHelperCall("shortcuts/markdown/markdown_fetch.go", src)
1056+
if len(v) != 2 {
1057+
t.Fatalf("expected 2 violations for aliased/function-value legacy helpers on markdown path, got %d: %+v", len(v), v)
1058+
}
1059+
}
1060+
10421061
func TestCheckNoLegacyCommonHelperCall_AllowsNonMigratedPath(t *testing.T) {
10431062
src := `package contact
10441063

shortcuts/common/runner.go

Lines changed: 3 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -676,30 +676,10 @@ func WrapInputStatErrorTyped(err error, readMsg ...string) error {
676676
WithCause(err)
677677
}
678678

679-
// WrapSaveErrorByCategory maps a FileIO.Save error to structured output errors,
680-
// using standardized messages and the given error category (e.g. "api_error", "io").
681-
// Path validation errors always use ErrValidation (exit code 2).
682-
//
683-
// Deprecated: use WrapSaveErrorTyped for typed error envelopes.
684-
func WrapSaveErrorByCategory(err error, category string) error {
685-
if err == nil {
686-
return nil
687-
}
688-
var me *fileio.MkdirError
689-
switch {
690-
case errors.Is(err, fileio.ErrPathValidation):
691-
return output.ErrValidation("unsafe output path: %s", err)
692-
case errors.As(err, &me):
693-
return output.Errorf(output.ExitInternal, category, "cannot create parent directory: %s", err)
694-
default:
695-
return output.Errorf(output.ExitInternal, category, "cannot create file: %s", err)
696-
}
697-
}
698-
699679
// WrapSaveErrorTyped maps a FileIO.Save error to typed validation/internal errors.
700-
// Unlike WrapSaveErrorByCategory, non-path failures always emit the canonical
701-
// "internal" wire type: call sites migrating from a custom category
702-
// (e.g. "io", "api_error") change their envelope's type field.
680+
// Non-path failures always emit the canonical "internal" wire type; call sites
681+
// migrating from a custom category (e.g. "io", "api_error") change their
682+
// envelope's type field.
703683
func WrapSaveErrorTyped(err error) error {
704684
if err == nil {
705685
return nil

shortcuts/markdown/helpers.go

Lines changed: 21 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ package markdown
66
import (
77
"bytes"
88
"context"
9-
"errors"
109
"fmt"
1110
"io"
1211
"net/http"
@@ -19,7 +18,6 @@ import (
1918

2019
"github.com/larksuite/cli/errs"
2120
"github.com/larksuite/cli/internal/client"
22-
"github.com/larksuite/cli/internal/output"
2321
"github.com/larksuite/cli/internal/validate"
2422
"github.com/larksuite/cli/shortcuts/common"
2523
)
@@ -85,16 +83,18 @@ func (spec markdownUploadSpec) Target() markdownUploadTarget {
8583
func validateMarkdownSpec(runtime *common.RuntimeContext, spec markdownUploadSpec, requireName bool) error {
8684
switch {
8785
case spec.ContentSet && spec.FileSet:
88-
return common.FlagErrorf("--content and --file are mutually exclusive")
86+
return markdownValidationError("--content and --file are mutually exclusive").
87+
WithParams(markdownInvalidParam("--content", "mutually exclusive"), markdownInvalidParam("--file", "mutually exclusive"))
8988
case !spec.ContentSet && !spec.FileSet:
90-
return common.FlagErrorf("specify exactly one of --content or --file")
89+
return markdownValidationError("specify exactly one of --content or --file").
90+
WithParams(markdownInvalidParam("--content", "required; specify exactly one"), markdownInvalidParam("--file", "required; specify exactly one"))
9191
}
9292

9393
if markdownFlagExplicitlyEmpty(runtime, "folder-token") {
94-
return common.FlagErrorf("--folder-token cannot be empty; omit it to upload into Drive root folder")
94+
return markdownValidationParamError("--folder-token", "--folder-token cannot be empty; omit it to upload into Drive root folder")
9595
}
9696
if markdownFlagExplicitlyEmpty(runtime, "wiki-token") {
97-
return common.FlagErrorf("--wiki-token cannot be empty; provide a valid wiki node token or omit the flag entirely")
97+
return markdownValidationParamError("--wiki-token", "--wiki-token cannot be empty; provide a valid wiki node token or omit the flag entirely")
9898
}
9999
targets := 0
100100
if spec.FolderToken != "" {
@@ -104,22 +104,23 @@ func validateMarkdownSpec(runtime *common.RuntimeContext, spec markdownUploadSpe
104104
targets++
105105
}
106106
if targets > 1 {
107-
return common.FlagErrorf("--folder-token and --wiki-token are mutually exclusive")
107+
return markdownValidationError("--folder-token and --wiki-token are mutually exclusive").
108+
WithParams(markdownInvalidParam("--folder-token", "mutually exclusive"), markdownInvalidParam("--wiki-token", "mutually exclusive"))
108109
}
109110
if spec.FolderToken != "" {
110111
if err := validate.ResourceName(spec.FolderToken, "--folder-token"); err != nil {
111-
return output.ErrValidation("%s", err)
112+
return markdownValidationParamError("--folder-token", "%s", err).WithCause(err)
112113
}
113114
}
114115
if spec.WikiToken != "" {
115116
if err := validate.ResourceName(spec.WikiToken, "--wiki-token"); err != nil {
116-
return output.ErrValidation("%s", err)
117+
return markdownValidationParamError("--wiki-token", "%s", err).WithCause(err)
117118
}
118119
}
119120

120121
if requireName && spec.ContentSet {
121122
if strings.TrimSpace(spec.FileName) == "" {
122-
return common.FlagErrorf("--name is required when using --content")
123+
return markdownValidationParamError("--name", "--name is required when using --content")
123124
}
124125
if err := validateMarkdownFileName(spec.FileName, "--name"); err != nil {
125126
return err
@@ -128,10 +129,10 @@ func validateMarkdownSpec(runtime *common.RuntimeContext, spec markdownUploadSpe
128129

129130
if spec.FileSet {
130131
if strings.TrimSpace(spec.FilePath) == "" {
131-
return common.FlagErrorf("--file cannot be empty")
132+
return markdownValidationParamError("--file", "--file cannot be empty")
132133
}
133134
if _, err := validate.SafeInputPath(spec.FilePath); err != nil {
134-
return output.ErrValidation("unsafe file path: %s", err)
135+
return markdownValidationParamError("--file", "unsafe file path: %s", err).WithCause(err)
135136
}
136137
if err := validateMarkdownFileName(filepath.Base(spec.FilePath), "--file"); err != nil {
137138
return err
@@ -154,10 +155,10 @@ func markdownFlagExplicitlyEmpty(runtime *common.RuntimeContext, flagName string
154155
func validateMarkdownFileName(name, flagName string) error {
155156
trimmed := strings.TrimSpace(name)
156157
if trimmed == "" {
157-
return common.FlagErrorf("%s cannot be empty", flagName)
158+
return markdownValidationParamError(flagName, "%s cannot be empty", flagName)
158159
}
159160
if !strings.HasSuffix(strings.ToLower(trimmed), ".md") {
160-
return common.FlagErrorf("%s must end with .md", flagName)
161+
return markdownValidationParamError(flagName, "%s must end with .md", flagName)
161162
}
162163
return nil
163164
}
@@ -201,22 +202,9 @@ func openMarkdownDownload(ctx context.Context, runtime *common.RuntimeContext, f
201202
return resp, nil
202203
}
203204

204-
func wrapMarkdownDownloadError(err error) error {
205-
// Preserve any already-classified error: legacy *output.ExitError or any
206-
// typed errs.* error. Only un-classified errors get wrapped as network.
207-
var exitErr *output.ExitError
208-
if errors.As(err, &exitErr) {
209-
return err
210-
}
211-
if _, ok := errs.ProblemOf(err); ok {
212-
return err
213-
}
214-
return output.ErrNetwork("download failed: %s", err)
215-
}
216-
217205
func validateNonEmptyMarkdownSize(size int64) error {
218206
if size == 0 {
219-
return output.ErrValidation("%s", markdownEmptyContentError)
207+
return markdownValidationError("%s", markdownEmptyContentError)
220208
}
221209
return nil
222210
}
@@ -227,12 +215,12 @@ func markdownSourceSize(runtime *common.RuntimeContext, spec markdownUploadSpec)
227215
size = int64(len(spec.Content))
228216
} else {
229217
if strings.TrimSpace(spec.FilePath) == "" {
230-
return 0, common.FlagErrorf("--file cannot be empty")
218+
return 0, markdownValidationParamError("--file", "--file cannot be empty")
231219
}
232220

233221
info, err := runtime.FileIO().Stat(spec.FilePath)
234222
if err != nil {
235-
return 0, common.WrapInputStatError(err)
223+
return 0, withMarkdownFileParam(common.WrapInputStatErrorTyped(err), "--file")
236224
}
237225
size = info.Size()
238226
}
@@ -424,7 +412,7 @@ func uploadMarkdownFileAll(runtime *common.RuntimeContext, spec markdownUploadSp
424412
return withMarkdownUploadRetryResult(runtime, markdownUploadAllAction, func() (markdownUploadResult, error) {
425413
fileReader, err := openReader()
426414
if err != nil {
427-
return markdownUploadResult{}, common.WrapInputStatErrorTyped(err)
415+
return markdownUploadResult{}, withMarkdownFileParam(common.WrapInputStatErrorTyped(err), "--file")
428416
}
429417
defer fileReader.Close()
430418

@@ -491,7 +479,7 @@ func uploadMarkdownFileMultipart(runtime *common.RuntimeContext, spec markdownUp
491479

492480
fileReader, err := openReader()
493481
if err != nil {
494-
return markdownUploadResult{}, common.WrapInputStatErrorTyped(err)
482+
return markdownUploadResult{}, withMarkdownFileParam(common.WrapInputStatErrorTyped(err), "--file")
495483
}
496484
defer fileReader.Close()
497485

@@ -563,7 +551,7 @@ func uploadMarkdownMultipartParts(runtime *common.RuntimeContext, fileReader io.
563551

564552
n, readErr := io.ReadFull(fileReader, buffer[:int(chunkSize)])
565553
if readErr != nil {
566-
return output.ErrValidation("cannot read file: %s", readErr)
554+
return markdownValidationParamError("--file", "cannot read file: %s", readErr).WithCause(readErr)
567555
}
568556

569557
fd := larkcore.NewFormdata()

shortcuts/markdown/markdown_diff.go

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package markdown
55

66
import (
77
"context"
8-
"errors"
98
"fmt"
109
"io"
1110
"regexp"
@@ -14,6 +13,7 @@ import (
1413

1514
"github.com/sergi/go-diff/diffmatchpatch"
1615

16+
"github.com/larksuite/cli/errs"
1717
"github.com/larksuite/cli/internal/output"
1818
"github.com/larksuite/cli/internal/validate"
1919
"github.com/larksuite/cli/shortcuts/common"
@@ -65,7 +65,7 @@ type markdownDiffHunkRange struct {
6565

6666
func validateMarkdownDiffSpec(runtime *common.RuntimeContext, spec markdownDiffSpec) error {
6767
if err := validate.ResourceName(spec.FileToken, "--file-token"); err != nil {
68-
return output.ErrValidation("%s", err)
68+
return markdownValidationParamError("--file-token", "%s", err).WithCause(err)
6969
}
7070
if spec.FromVersion != "" {
7171
if err := validateMarkdownDiffVersionValue(spec.FromVersion, "--from-version"); err != nil {
@@ -79,40 +79,45 @@ func validateMarkdownDiffSpec(runtime *common.RuntimeContext, spec markdownDiffS
7979
}
8080
if spec.FilePath != "" {
8181
if _, err := validate.SafeInputPath(spec.FilePath); err != nil {
82-
return output.ErrValidation("unsafe file path: %s", err)
82+
return markdownValidationParamError("--file", "unsafe file path: %s", err).WithCause(err)
8383
}
8484
if err := validateMarkdownFileName(spec.FilePath, "--file"); err != nil {
8585
return err
8686
}
8787
}
8888
if spec.ContextLines < 0 {
89-
return output.ErrValidation("--context-lines must be >= 0")
89+
return markdownValidationParamError("--context-lines", "--context-lines must be >= 0")
9090
}
9191
if spec.Format != "" && spec.Format != "json" && spec.Format != "pretty" {
92-
return output.ErrValidation("markdown +diff only supports --format json or pretty")
92+
return markdownValidationParamError("--format", "markdown +diff only supports --format json or pretty")
9393
}
9494
if spec.FilePath == "" {
9595
if spec.FromVersion == "" && spec.ToVersion == "" {
96-
return common.FlagErrorf("specify --from-version, or both --from-version and --to-version, or use --file for remote vs local diff")
96+
return markdownValidationError("specify --from-version, or both --from-version and --to-version, or use --file for remote vs local diff").
97+
WithParams(
98+
markdownInvalidParam("--from-version", "required; specify one"),
99+
markdownInvalidParam("--to-version", "required; specify one"),
100+
markdownInvalidParam("--file", "required; specify one"),
101+
)
97102
}
98103
if spec.FromVersion == "" && spec.ToVersion != "" {
99-
return common.FlagErrorf("--to-version requires --from-version")
104+
return markdownValidationParamError("--to-version", "--to-version requires --from-version")
100105
}
101106
return nil
102107
}
103108
if spec.ToVersion != "" {
104-
return common.FlagErrorf("--to-version is not supported together with --file")
109+
return markdownValidationParamError("--to-version", "--to-version is not supported together with --file")
105110
}
106111
return nil
107112
}
108113

109114
func validateMarkdownDiffVersionValue(value, flagName string) error {
110115
value = strings.TrimSpace(value)
111116
if value == "" {
112-
return output.ErrValidation("%s cannot be empty", flagName)
117+
return markdownValidationParamError(flagName, "%s cannot be empty", flagName)
113118
}
114119
if !markdownDiffVersionRe.MatchString(value) {
115-
return output.ErrValidation("%s must be a numeric version string", flagName)
120+
return markdownValidationParamError(flagName, "%s must be a numeric version string", flagName)
116121
}
117122
return nil
118123
}
@@ -178,17 +183,16 @@ func downloadMarkdownContent(ctx context.Context, runtime *common.RuntimeContext
178183
func readMarkdownLocalFile(runtime *common.RuntimeContext, filePath string) (string, error) {
179184
f, err := runtime.FileIO().Open(filePath)
180185
if err != nil {
181-
return "", common.WrapInputStatError(err)
186+
return "", withMarkdownFileParam(common.WrapInputStatErrorTyped(err), "--file")
182187
}
183188
defer f.Close()
184189

185190
payload, err := readMarkdownDiffPayload(f, "local Markdown file")
186191
if err != nil {
187-
var exitErr *output.ExitError
188-
if errors.As(err, &exitErr) {
189-
return "", err
192+
if _, ok := errs.ProblemOf(err); ok {
193+
return "", withMarkdownFileParam(err, "--file")
190194
}
191-
return "", output.ErrValidation("cannot read file: %s", err)
195+
return "", markdownValidationParamError("--file", "cannot read file: %s", err).WithCause(err)
192196
}
193197
return string(payload), nil
194198
}
@@ -199,7 +203,7 @@ func readMarkdownDiffPayload(r io.Reader, source string) ([]byte, error) {
199203
return nil, err
200204
}
201205
if len(payload) > markdownDiffMaxContentBytes {
202-
return nil, output.ErrValidation("%s exceeds %s markdown +diff content limit", source, common.FormatSize(markdownDiffMaxContentBytes))
206+
return nil, markdownValidationError("%s exceeds %s markdown +diff content limit", source, common.FormatSize(markdownDiffMaxContentBytes))
203207
}
204208
return payload, nil
205209
}

0 commit comments

Comments
 (0)