Skip to content

Commit e3aae12

Browse files
committed
feat(slides): emit typed error envelopes across the slides domain
Emit structured validation, API, network, file, and internal error envelopes for Slides shortcuts so users and agents can recover from failed presentation workflows using stable type, subtype, param, and code fields. Add Slides domain errscontract and golangci guards to prevent legacy envelope and common helper regressions.
1 parent 201e3e0 commit e3aae12

13 files changed

Lines changed: 393 additions & 86 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/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/minutes/|shortcuts/okr/|shortcuts/slides/|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/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/minutes/|shortcuts/okr/|shortcuts/slides/|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/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/minutes/|shortcuts/okr/|shortcuts/slides/|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
@@ -27,6 +27,7 @@ var migratedCommonHelperPaths = []string{
2727
"shortcuts/mail/",
2828
"shortcuts/minutes/",
2929
"shortcuts/okr/",
30+
"shortcuts/slides/",
3031
"shortcuts/task/",
3132
"shortcuts/vc/",
3233
"shortcuts/whiteboard/",

lint/errscontract/rule_no_legacy_envelope_literal.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ var migratedEnvelopePaths = []string{
2828
"shortcuts/mail/",
2929
"shortcuts/minutes/",
3030
"shortcuts/okr/",
31+
"shortcuts/slides/",
3132
"shortcuts/task/",
3233
"shortcuts/vc/",
3334
"shortcuts/whiteboard/",

lint/errscontract/rules_test.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -954,6 +954,7 @@ func TestCheckNoLegacyCommonHelperCall_RejectsLegacyHelpersOnMigratedPath(t *tes
954954
"shortcuts/drive/drive_search.go",
955955
"shortcuts/mail/mail_send.go",
956956
"shortcuts/okr/okr_progress_create.go",
957+
"shortcuts/slides/slides_create.go",
957958
"shortcuts/task/task_update.go",
958959
"shortcuts/whiteboard/whiteboard_query.go",
959960
}
@@ -1021,6 +1022,23 @@ func boom() {
10211022
}
10221023
}
10231024

1025+
func TestCheckNoLegacyCommonHelperCall_CoversSlidesPathWithAliasAndFunctionValue(t *testing.T) {
1026+
src := `package migrated
1027+
1028+
import c "github.com/larksuite/cli/shortcuts/common"
1029+
1030+
func boom() {
1031+
f := c.FlagErrorf
1032+
_ = f
1033+
c.WrapInputStatError(nil)
1034+
}
1035+
`
1036+
v := CheckNoLegacyCommonHelperCall("shortcuts/slides/slides_create.go", src)
1037+
if len(v) != 2 {
1038+
t.Fatalf("expected 2 violations for aliased/function-value legacy helpers on slides path, got %d: %+v", len(v), v)
1039+
}
1040+
}
1041+
10241042
func TestCheckNoLegacyCommonHelperCall_AllowsNonMigratedPath(t *testing.T) {
10251043
src := `package contact
10261044

shortcuts/slides/helpers.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
"regexp"
1010
"strings"
1111

12-
"github.com/larksuite/cli/internal/output"
12+
"github.com/larksuite/cli/errs"
1313
"github.com/larksuite/cli/shortcuts/common"
1414
)
1515

@@ -30,29 +30,29 @@ type presentationRef struct {
3030
func parsePresentationRef(input string) (presentationRef, error) {
3131
raw := strings.TrimSpace(input)
3232
if raw == "" {
33-
return presentationRef{}, output.ErrValidation("--presentation cannot be empty")
33+
return presentationRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "--presentation cannot be empty").WithParam("--presentation")
3434
}
3535
// URL inputs: parse properly and only honor /slides/ or /wiki/ when they
3636
// appear as a prefix of the URL path. Substring matching previously let
3737
// e.g. `https://x/docx/foo?next=/slides/abc` resolve to token "abc".
3838
if strings.Contains(raw, "://") {
3939
u, err := url.Parse(raw)
4040
if err != nil || u.Path == "" {
41-
return presentationRef{}, output.ErrValidation("unsupported --presentation input %q: use an xml_presentation_id, a /slides/ URL, or a /wiki/ URL", raw)
41+
return presentationRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported --presentation input %q: use an xml_presentation_id, a /slides/ URL, or a /wiki/ URL", raw).WithParam("--presentation")
4242
}
4343
if token, ok := tokenAfterPathPrefix(u.Path, "/slides/"); ok {
4444
return presentationRef{Kind: "slides", Token: token}, nil
4545
}
4646
if token, ok := tokenAfterPathPrefix(u.Path, "/wiki/"); ok {
4747
return presentationRef{Kind: "wiki", Token: token}, nil
4848
}
49-
return presentationRef{}, output.ErrValidation("unsupported --presentation input %q: use an xml_presentation_id, a /slides/ URL, or a /wiki/ URL", raw)
49+
return presentationRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported --presentation input %q: use an xml_presentation_id, a /slides/ URL, or a /wiki/ URL", raw).WithParam("--presentation")
5050
}
5151
// Non-URL input must be a bare token — anything with path/query/fragment
5252
// chars is rejected so partial-path inputs like `tmp/wiki/wikcn123` don't
5353
// get silently accepted.
5454
if strings.ContainsAny(raw, "/?#") {
55-
return presentationRef{}, output.ErrValidation("unsupported --presentation input %q: use an xml_presentation_id, a /slides/ URL, or a /wiki/ URL", raw)
55+
return presentationRef{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported --presentation input %q: use an xml_presentation_id, a /slides/ URL, or a /wiki/ URL", raw).WithParam("--presentation")
5656
}
5757
return presentationRef{Kind: "slides", Token: raw}, nil
5858
}
@@ -82,7 +82,7 @@ func resolvePresentationID(runtime *common.RuntimeContext, ref presentationRef)
8282
case "slides":
8383
return ref.Token, nil
8484
case "wiki":
85-
data, err := runtime.CallAPI(
85+
data, err := runtime.CallAPITyped(
8686
"GET",
8787
"/open-apis/wiki/v2/spaces/get_node",
8888
map[string]interface{}{"token": ref.Token},
@@ -95,14 +95,14 @@ func resolvePresentationID(runtime *common.RuntimeContext, ref presentationRef)
9595
objType := common.GetString(node, "obj_type")
9696
objToken := common.GetString(node, "obj_token")
9797
if objType == "" || objToken == "" {
98-
return "", output.Errorf(output.ExitAPI, "api_error", "wiki get_node returned incomplete node data")
98+
return "", errs.NewInternalError(errs.SubtypeInvalidResponse, "wiki get_node returned incomplete node data")
9999
}
100100
if objType != "slides" {
101-
return "", output.ErrValidation("wiki resolved to %q, but slides shortcuts require a slides presentation", objType)
101+
return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "wiki resolved to %q, but slides shortcuts require a slides presentation", objType).WithParam("--presentation")
102102
}
103103
return objToken, nil
104104
default:
105-
return "", output.ErrValidation("unsupported presentation ref kind %q", ref.Kind)
105+
return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "unsupported presentation ref kind %q", ref.Kind)
106106
}
107107
}
108108

@@ -191,7 +191,7 @@ var xmlIdAttrRegex = regexp.MustCompile(`(?s)(?:^|\s)id\s*=\s*(["'])(.*?)(["'])`
191191
func ensureXMLRootID(xmlFragment, want string) (string, error) {
192192
m := xmlRootOpenTagRegex.FindStringSubmatchIndex(xmlFragment)
193193
if m == nil {
194-
return "", fmt.Errorf("no root element found in XML fragment")
194+
return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "no root element found in XML fragment")
195195
}
196196
prefix := xmlFragment[m[2]:m[3]]
197197
tagName := xmlFragment[m[4]:m[5]]

shortcuts/slides/slides_create.go

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import (
1010
"path/filepath"
1111
"strings"
1212

13-
"github.com/larksuite/cli/internal/output"
13+
"github.com/larksuite/cli/errs"
1414
"github.com/larksuite/cli/internal/validate"
1515
"github.com/larksuite/cli/shortcuts/common"
1616
)
@@ -45,24 +45,24 @@ var SlidesCreate = common.Shortcut{
4545
if slidesStr := runtime.Str("slides"); slidesStr != "" {
4646
var slides []string
4747
if err := json.Unmarshal([]byte(slidesStr), &slides); err != nil {
48-
return common.FlagErrorf("--slides invalid JSON, must be an array of XML strings")
48+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides invalid JSON, must be an array of XML strings").WithParam("--slides")
4949
}
5050
if len(slides) > maxSlidesPerCreate {
51-
return common.FlagErrorf("--slides array exceeds maximum of %d slides; create the presentation first, then add slides via xml_presentation.slide.create", maxSlidesPerCreate)
51+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides array exceeds maximum of %d slides; create the presentation first, then add slides via xml_presentation.slide.create", maxSlidesPerCreate).WithParam("--slides")
5252
}
5353
// Validate placeholder paths up front so we don't create a presentation
5454
// only to fail mid-way on a missing local file.
5555
for _, path := range extractImagePlaceholderPaths(slides) {
5656
stat, err := runtime.FileIO().Stat(path)
5757
if err != nil {
58-
return common.WrapInputStatError(err, fmt.Sprintf("--slides @%s: file not found", path))
58+
return slidesInputStatError(err, "--slides", fmt.Sprintf("--slides @%s: file not found", path))
5959
}
6060
if !stat.Mode().IsRegular() {
61-
return common.FlagErrorf("--slides @%s: must be a regular file", path)
61+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides @%s: must be a regular file", path).WithParam("--slides")
6262
}
6363
if stat.Size() > common.MaxDriveMediaUploadSinglePartSize {
64-
return common.FlagErrorf("--slides @%s: file size %s exceeds 20 MB limit for slides image upload",
65-
path, common.FormatSize(stat.Size()))
64+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--slides @%s: file size %s exceeds 20 MB limit for slides image upload",
65+
path, common.FormatSize(stat.Size())).WithParam("--slides")
6666
}
6767
}
6868
}
@@ -128,7 +128,7 @@ var SlidesCreate = common.Shortcut{
128128
slidesStr := runtime.Str("slides")
129129

130130
// Step 1: Create presentation
131-
data, err := runtime.CallAPI(
131+
data, err := runtime.CallAPITyped(
132132
"POST",
133133
"/open-apis/slides_ai/v1/xml_presentations",
134134
nil,
@@ -144,7 +144,7 @@ var SlidesCreate = common.Shortcut{
144144

145145
presentationID := common.GetString(data, "xml_presentation_id")
146146
if presentationID == "" {
147-
return output.Errorf(output.ExitAPI, "api_error", "slides create returned no xml_presentation_id")
147+
return errs.NewInternalError(errs.SubtypeInvalidResponse, "slides create returned no xml_presentation_id")
148148
}
149149

150150
result := map[string]interface{}{
@@ -168,9 +168,7 @@ var SlidesCreate = common.Shortcut{
168168
if len(placeholders) > 0 {
169169
tokens, uploaded, err := uploadSlidesPlaceholders(runtime, presentationID, placeholders)
170170
if err != nil {
171-
return output.Errorf(output.ExitAPI, "api_error",
172-
"image upload failed: %v (presentation %s was created; %d image(s) uploaded before failure)",
173-
err, presentationID, uploaded)
171+
return appendSlidesProgressHint(err, fmt.Sprintf("presentation %s was created; %d image(s) uploaded before failure", presentationID, uploaded))
174172
}
175173
for i := range slides {
176174
slides[i] = replaceImagePlaceholders(slides[i], tokens)
@@ -185,7 +183,7 @@ var SlidesCreate = common.Shortcut{
185183

186184
var slideIDs []string
187185
for i, slideXML := range slides {
188-
slideData, err := runtime.CallAPI(
186+
slideData, err := runtime.CallAPITyped(
189187
"POST",
190188
slideURL,
191189
map[string]interface{}{"revision_id": -1},
@@ -194,9 +192,7 @@ var SlidesCreate = common.Shortcut{
194192
},
195193
)
196194
if err != nil {
197-
return output.Errorf(output.ExitAPI, "api_error",
198-
"slide %d/%d failed: %v (presentation %s was created; %d slide(s) added before failure)",
199-
i+1, len(slides), err, presentationID, i)
195+
return appendSlidesProgressHint(err, fmt.Sprintf("adding slide %d/%d failed; presentation %s was created, %d slide(s) added before failure", i+1, len(slides), presentationID, i))
200196
}
201197
if sid := common.GetString(slideData, "slide_id"); sid != "" {
202198
slideIDs = append(slideIDs, sid)
@@ -256,18 +252,18 @@ func uploadSlidesPlaceholders(runtime *common.RuntimeContext, presentationID str
256252
for i, path := range paths {
257253
stat, err := runtime.FileIO().Stat(path)
258254
if err != nil {
259-
return tokens, i, common.WrapInputStatError(err, fmt.Sprintf("@%s: file not found", path))
255+
return tokens, i, slidesInputStatError(err, "--slides", fmt.Sprintf("@%s: file not found", path))
260256
}
261257
if !stat.Mode().IsRegular() {
262-
return tokens, i, output.ErrValidation("@%s: must be a regular file", path)
258+
return tokens, i, errs.NewValidationError(errs.SubtypeInvalidArgument, "@%s: must be a regular file", path).WithParam("--slides")
263259
}
264260
fileName := filepath.Base(path)
265261
fmt.Fprintf(runtime.IO().ErrOut, "Uploading image %d/%d: %s (%s)\n",
266262
i+1, len(paths), fileName, common.FormatSize(stat.Size()))
267263

268264
token, err := uploadSlidesMedia(runtime, path, fileName, stat.Size(), presentationID)
269265
if err != nil {
270-
return tokens, i, fmt.Errorf("@%s: %w", path, err)
266+
return tokens, i, fmt.Errorf("@%s: %w", path, err) //nolint:forbidigo // intermediate; preserves typed cause via %w, reclassified by appendSlidesProgressHint at the call site
271267
}
272268
tokens[path] = token
273269
}

shortcuts/slides/slides_create_test.go

Lines changed: 80 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,14 @@ package slides
66
import (
77
"bytes"
88
"encoding/json"
9+
"errors"
910
"os"
1011
"strings"
1112
"testing"
1213

1314
"github.com/spf13/cobra"
1415

16+
"github.com/larksuite/cli/errs"
1517
"github.com/larksuite/cli/internal/cmdutil"
1618
"github.com/larksuite/cli/internal/core"
1719
"github.com/larksuite/cli/internal/httpmock"
@@ -400,15 +402,21 @@ func TestSlidesCreateWithSlidesPartialFailure(t *testing.T) {
400402
if err == nil {
401403
t.Fatal("expected error for partial failure, got nil")
402404
}
403-
errMsg := err.Error()
404-
if !strings.Contains(errMsg, "pres_partial") {
405-
t.Fatalf("error should contain presentation ID, got: %s", errMsg)
405+
p, ok := errs.ProblemOf(err)
406+
if !ok {
407+
t.Fatalf("expected a typed errs.* error, got %v", err)
406408
}
407-
if !strings.Contains(errMsg, "slide 2/2") {
408-
t.Fatalf("error should indicate slide 2/2 failed, got: %s", errMsg)
409+
// The presentation was created but a slide add failed; the recovery hint
410+
// carries the partial-progress context (which presentation exists, how many
411+
// slides landed) so the caller can resume without recreating.
412+
if !strings.Contains(p.Hint, "pres_partial") {
413+
t.Fatalf("hint should contain presentation ID, got: %s", p.Hint)
409414
}
410-
if !strings.Contains(errMsg, "1 slide(s) added") {
411-
t.Fatalf("error should report 1 slide added before failure, got: %s", errMsg)
415+
if !strings.Contains(p.Hint, "slide 2/2") {
416+
t.Fatalf("hint should indicate slide 2/2 failed, got: %s", p.Hint)
417+
}
418+
if !strings.Contains(p.Hint, "1 slide(s) added") {
419+
t.Fatalf("hint should report 1 slide added before failure, got: %s", p.Hint)
412420
}
413421
}
414422

@@ -457,6 +465,71 @@ func TestSlidesCreateWithSlidesExceedsMax(t *testing.T) {
457465
}
458466
}
459467

468+
// TestSlidesCreateValidationParam locks Param=="--slides" on the pure
469+
// validation rejections, so callers route on the typed field rather than the
470+
// message.
471+
func TestSlidesCreateValidationParam(t *testing.T) {
472+
t.Parallel()
473+
474+
elems := make([]string, 11)
475+
for i := range elems {
476+
elems[i] = `"<slide/>"`
477+
}
478+
exceedsMax := "[" + strings.Join(elems, ",") + "]"
479+
480+
tests := []struct {
481+
name string
482+
slides string
483+
}{
484+
{"invalid JSON", "not json"},
485+
{"exceeds max", exceedsMax},
486+
}
487+
488+
for _, tt := range tests {
489+
t.Run(tt.name, func(t *testing.T) {
490+
t.Parallel()
491+
f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, ""))
492+
err := runSlidesCreateShortcut(t, f, stdout, []string{
493+
"+create",
494+
"--slides", tt.slides,
495+
"--as", "user",
496+
})
497+
498+
var ve *errs.ValidationError
499+
if !errors.As(err, &ve) {
500+
t.Fatalf("err = %v, want *errs.ValidationError", err)
501+
}
502+
if ve.Param != "--slides" {
503+
t.Fatalf("Param = %q, want --slides", ve.Param)
504+
}
505+
})
506+
}
507+
}
508+
509+
// TestSlidesCreatePlaceholderMissingParam guards the create.go caller wiring:
510+
// a missing @-placeholder file must surface a --slides-tagged validation error
511+
// through the shared slidesInputStatError helper.
512+
func TestSlidesCreatePlaceholderMissingParam(t *testing.T) {
513+
dir := t.TempDir()
514+
withSlidesTestWorkingDir(t, dir)
515+
516+
f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, ""))
517+
slidesJSON := `["<slide><data><img src=\"@./missing.png\"/></data></slide>"]`
518+
err := runSlidesCreateShortcut(t, f, stdout, []string{
519+
"+create",
520+
"--slides", slidesJSON,
521+
"--as", "user",
522+
})
523+
524+
var ve *errs.ValidationError
525+
if !errors.As(err, &ve) {
526+
t.Fatalf("err = %v, want *errs.ValidationError", err)
527+
}
528+
if ve.Param != "--slides" {
529+
t.Fatalf("Param = %q, want --slides", ve.Param)
530+
}
531+
}
532+
460533
// TestSlidesCreateWithSlidesEmptyArray verifies that --slides '[]' behaves like no --slides.
461534
func TestSlidesCreateWithSlidesEmptyArray(t *testing.T) {
462535
t.Parallel()

0 commit comments

Comments
 (0)