Skip to content

Commit 31968d9

Browse files
evandanceSunPeiYang996
authored andcommitted
feat(okr,whiteboard): emit typed error envelopes across both domains (#1236)
The okr and whiteboard commands now report every failure as a typed error envelope. Invalid flags, malformed input, output-file conflicts, and API or transport failures alike carry a stable category, subtype, the offending flag or Lark error code, and a meaningful exit code — so scripts and agents can branch on the error shape instead of scraping message strings.
1 parent 4dd44f9 commit 31968d9

24 files changed

Lines changed: 827 additions & 271 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/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/base/)
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/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/base/|shortcuts/okr/|shortcuts/whiteboard/)
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/drive/|shortcuts/mail/|shortcuts/base/|shortcuts/calendar/|shortcuts/common/mcp_client\.go)
83+
- path-except: (shortcuts/drive/|shortcuts/mail/|shortcuts/base/|shortcuts/calendar/|shortcuts/okr/|shortcuts/whiteboard/|shortcuts/common/mcp_client\.go)
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/drive/|shortcuts/mail/|shortcuts/base/|shortcuts/calendar/)
89+
- path-except: (shortcuts/drive/|shortcuts/mail/|shortcuts/base/|shortcuts/calendar/|shortcuts/okr/|shortcuts/whiteboard/)
9090
text: errs-no-legacy-helper
9191
linters:
9292
- forbidigo

lint/errscontract/rule_no_legacy_common_helper_call.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ var migratedCommonHelperPaths = []string{
1919
"shortcuts/drive/",
2020
"shortcuts/mail/",
2121
"shortcuts/calendar/",
22+
"shortcuts/okr/",
23+
"shortcuts/whiteboard/",
2224
}
2325

2426
const commonImportPath = "github.com/larksuite/cli/shortcuts/common"

lint/errscontract/rule_no_legacy_envelope_literal.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ var migratedEnvelopePaths = []string{
2020
"shortcuts/drive/",
2121
"shortcuts/mail/",
2222
"shortcuts/calendar/",
23+
"shortcuts/okr/",
24+
"shortcuts/whiteboard/",
2325
}
2426

2527
// legacyOutputImportPath is the import path of the package that declares the

lint/errscontract/rules_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -618,6 +618,31 @@ func boom() error {
618618
}
619619
}
620620

621+
func TestCheckNoLegacyEnvelopeLiteral_RejectsExitErrorLiteralOnOkrAndWhiteboardPaths(t *testing.T) {
622+
for _, path := range []string{
623+
"shortcuts/okr/okr_image_upload.go",
624+
"shortcuts/whiteboard/whiteboard_update.go",
625+
} {
626+
t.Run(path, func(t *testing.T) {
627+
src := `package migrated
628+
629+
import "github.com/larksuite/cli/internal/output"
630+
631+
func boom() error {
632+
return &output.ExitError{Code: 1}
633+
}
634+
`
635+
v := CheckNoLegacyEnvelopeLiteral(path, src)
636+
if len(v) != 1 {
637+
t.Fatalf("expected 1 violation, got %d: %+v", len(v), v)
638+
}
639+
if v[0].Action != ActionReject {
640+
t.Errorf("action = %q, want REJECT", v[0].Action)
641+
}
642+
})
643+
}
644+
}
645+
621646
func TestCheckNoLegacyEnvelopeLiteral_RejectsErrDetailLiteralOnDrivePath(t *testing.T) {
622647
src := `package drive
623648
@@ -897,6 +922,8 @@ func TestCheckNoLegacyCommonHelperCall_RejectsLegacyHelpersOnMigratedPath(t *tes
897922
paths := []string{
898923
"shortcuts/drive/drive_search.go",
899924
"shortcuts/mail/mail_send.go",
925+
"shortcuts/okr/okr_progress_create.go",
926+
"shortcuts/whiteboard/whiteboard_query.go",
900927
}
901928
for _, path := range paths {
902929
for _, helper := range helpers {

shortcuts/common/runner.go

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -603,27 +603,6 @@ func (ctx *RuntimeContext) ResolveSavePath(path string) (string, error) {
603603
return resolved, nil
604604
}
605605

606-
// WrapSaveError matches a FileIO.Save error against known categories and wraps
607-
// it with the caller-provided message prefix, preserving backward-compatible
608-
// error text per shortcut.
609-
func WrapSaveError(err error, pathMsg, mkdirMsg, writeMsg string) error {
610-
if err == nil {
611-
return nil
612-
}
613-
var me *fileio.MkdirError
614-
var we *fileio.WriteError
615-
switch {
616-
case errors.Is(err, fileio.ErrPathValidation):
617-
return fmt.Errorf("%s: %w", pathMsg, err)
618-
case errors.As(err, &me):
619-
return fmt.Errorf("%s: %w", mkdirMsg, err)
620-
case errors.As(err, &we):
621-
return fmt.Errorf("%s: %w", writeMsg, err)
622-
default:
623-
return fmt.Errorf("%s: %w", writeMsg, err)
624-
}
625-
}
626-
627606
// WrapOpenError matches a FileIO.Open/Stat error and wraps it with the
628607
// caller-provided message prefix.
629608
func WrapOpenError(err error, pathMsg, readMsg string) error {

shortcuts/okr/okr_cycle_detail.go

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ import (
1111
"strconv"
1212
"time"
1313

14+
"github.com/larksuite/cli/errs"
1415
"github.com/larksuite/cli/shortcuts/common"
15-
larkcore "github.com/larksuite/oapi-sdk-go/v3/core"
1616
)
1717

1818
// OKRCycleDetail lists all objectives and their key results under a given OKR cycle.
@@ -30,10 +30,10 @@ var OKRCycleDetail = common.Shortcut{
3030
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
3131
cycleID := runtime.Str("cycle-id")
3232
if cycleID == "" {
33-
return common.FlagErrorf("--cycle-id is required")
33+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--cycle-id is required").WithParam("--cycle-id")
3434
}
3535
if id, err := strconv.ParseInt(cycleID, 10, 64); err != nil || id <= 0 {
36-
return common.FlagErrorf("--cycle-id must be a positive int64")
36+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--cycle-id must be a positive int64").WithParam("--cycle-id")
3737
}
3838
return nil
3939
},
@@ -52,8 +52,7 @@ var OKRCycleDetail = common.Shortcut{
5252
cycleID := runtime.Str("cycle-id")
5353

5454
// Paginate objectives under the cycle.
55-
queryParams := make(larkcore.QueryParams)
56-
queryParams.Set("page_size", "100")
55+
queryParams := map[string]interface{}{"page_size": "100"}
5756

5857
var objectives []Objective
5958
page := 0
@@ -71,7 +70,7 @@ var OKRCycleDetail = common.Shortcut{
7170
page++
7271

7372
path := fmt.Sprintf("/open-apis/okr/v2/cycles/%s/objectives", cycleID)
74-
data, err := runtime.DoAPIJSON("GET", path, queryParams, nil)
73+
data, err := runtime.CallAPITyped("GET", path, queryParams, nil)
7574
if err != nil {
7675
return err
7776
}
@@ -93,7 +92,7 @@ var OKRCycleDetail = common.Shortcut{
9392
if !hasMore || pageToken == "" {
9493
break
9594
}
96-
queryParams.Set("page_token", pageToken)
95+
queryParams["page_token"] = pageToken
9796
}
9897

9998
// For each objective, paginate key results and convert to response format.
@@ -104,8 +103,7 @@ var OKRCycleDetail = common.Shortcut{
104103
}
105104
obj := &objectives[i]
106105

107-
krQuery := make(larkcore.QueryParams)
108-
krQuery.Set("page_size", "100")
106+
krQuery := map[string]interface{}{"page_size": "100"}
109107

110108
var keyResults []KeyResult
111109
krPage := 0
@@ -123,7 +121,7 @@ var OKRCycleDetail = common.Shortcut{
123121
krPage++
124122

125123
path := fmt.Sprintf("/open-apis/okr/v2/objectives/%s/key_results", obj.ID)
126-
data, err := runtime.DoAPIJSON("GET", path, krQuery, nil)
124+
data, err := runtime.CallAPITyped("GET", path, krQuery, nil)
127125
if err != nil {
128126
return err
129127
}
@@ -145,7 +143,7 @@ var OKRCycleDetail = common.Shortcut{
145143
if !hasMore || pageToken == "" {
146144
break
147145
}
148-
krQuery.Set("page_token", pageToken)
146+
krQuery["page_token"] = pageToken
149147
}
150148

151149
respObj := obj.ToResp()

shortcuts/okr/okr_cycle_detail_test.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,13 @@ package okr
66
import (
77
"bytes"
88
"encoding/json"
9+
"errors"
910
"strings"
1011
"testing"
1112

1213
"github.com/spf13/cobra"
1314

15+
"github.com/larksuite/cli/errs"
1416
"github.com/larksuite/cli/internal/cmdutil"
1517
"github.com/larksuite/cli/internal/core"
1618
"github.com/larksuite/cli/internal/httpmock"
@@ -106,6 +108,31 @@ func TestCycleDetailValidate_InvalidCycleID_Negative(t *testing.T) {
106108
}
107109
}
108110

111+
// TestCycleDetailValidate_TypedError locks the typed-envelope contract shared by
112+
// every okr flag check: an invalid flag surfaces as *errs.ValidationError carrying
113+
// SubtypeInvalidArgument and the offending --flag (readable via errors.As /
114+
// errs.ProblemOf), and maps to the validation exit code rather than a legacy api error.
115+
func TestCycleDetailValidate_TypedError(t *testing.T) {
116+
t.Parallel()
117+
f, stdout, _, _ := cmdutil.TestFactory(t, cycleDetailTestConfig(t))
118+
err := runCycleDetailShortcut(t, f, stdout, []string{"+cycle-detail", "--cycle-id", "0"})
119+
120+
var ve *errs.ValidationError
121+
if !errors.As(err, &ve) {
122+
t.Fatalf("error is not *errs.ValidationError: %T (%v)", err, err)
123+
}
124+
if ve.Subtype != errs.SubtypeInvalidArgument {
125+
t.Errorf("Subtype = %q, want %q", ve.Subtype, errs.SubtypeInvalidArgument)
126+
}
127+
if ve.Param != "--cycle-id" {
128+
t.Errorf("Param = %q, want %q", ve.Param, "--cycle-id")
129+
}
130+
p, ok := errs.ProblemOf(err)
131+
if !ok || p.Category != errs.CategoryValidation {
132+
t.Errorf("ProblemOf category = %v (ok=%v), want %q", p, ok, errs.CategoryValidation)
133+
}
134+
}
135+
109136
func TestCycleDetailValidate_ValidCycleID(t *testing.T) {
110137
t.Parallel()
111138
f, stdout, _, reg := cmdutil.TestFactory(t, cycleDetailTestConfig(t))

shortcuts/okr/okr_cycle_list.go

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -12,30 +12,29 @@ import (
1212
"strings"
1313
"time"
1414

15-
"github.com/larksuite/cli/internal/validate"
15+
"github.com/larksuite/cli/errs"
1616
"github.com/larksuite/cli/shortcuts/common"
17-
larkcore "github.com/larksuite/oapi-sdk-go/v3/core"
1817
)
1918

2019
// parseTimeRange parses a "YYYY-MM--YYYY-MM" string into two time.Time values.
2120
// The start is the first moment of the start month; the end is the last moment of the end month.
2221
func parseTimeRange(s string) (start, end time.Time, err error) {
2322
parts := strings.SplitN(s, "--", 2)
2423
if len(parts) != 2 {
25-
return time.Time{}, time.Time{}, fmt.Errorf("invalid time-range format %q, expected YYYY-MM--YYYY-MM", s)
24+
return time.Time{}, time.Time{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --time-range format %q, expected YYYY-MM--YYYY-MM", s).WithParam("--time-range")
2625
}
2726
start, err = time.Parse("2006-01", strings.TrimSpace(parts[0]))
2827
if err != nil {
29-
return time.Time{}, time.Time{}, fmt.Errorf("invalid start month %q: %w", parts[0], err)
28+
return time.Time{}, time.Time{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --time-range start month %q: %v", parts[0], err).WithParam("--time-range").WithCause(err)
3029
}
3130
end, err = time.Parse("2006-01", strings.TrimSpace(parts[1]))
3231
if err != nil {
33-
return time.Time{}, time.Time{}, fmt.Errorf("invalid end month %q: %w", parts[1], err)
32+
return time.Time{}, time.Time{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --time-range end month %q: %v", parts[1], err).WithParam("--time-range").WithCause(err)
3433
}
3534
// end is the last moment of the end month
3635
end = end.AddDate(0, 1, 0).Add(-time.Millisecond)
3736
if start.After(end) {
38-
return time.Time{}, time.Time{}, fmt.Errorf("start month %s is after end month %s", parts[0], parts[1])
37+
return time.Time{}, time.Time{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid --time-range: start month %s is after end month %s", parts[0], parts[1]).WithParam("--time-range")
3938
}
4039
return start, end, nil
4140
}
@@ -69,20 +68,20 @@ var OKRListCycles = common.Shortcut{
6968
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
7069
idType := runtime.Str("user-id-type")
7170
if idType != "open_id" && idType != "union_id" && idType != "user_id" {
72-
return common.FlagErrorf("--user-id-type must be one of: open_id | union_id | user_id")
71+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--user-id-type must be one of: open_id | union_id | user_id").WithParam("--user-id-type")
7372
}
7473
userID := runtime.Str("user-id")
75-
if err := validate.RejectControlChars(userID, "user-id"); err != nil {
74+
if err := common.RejectDangerousCharsTyped("--user-id", userID); err != nil {
7675
return err
7776
}
7877

7978
tr := runtime.Str("time-range")
8079
if tr != "" {
81-
if err := validate.RejectControlChars(tr, "time-range"); err != nil {
80+
if err := common.RejectDangerousCharsTyped("--time-range", tr); err != nil {
8281
return err
8382
}
8483
if _, _, err := parseTimeRange(tr); err != nil {
85-
return common.FlagErrorf("--time-range: %s", err)
84+
return err
8685
}
8786
}
8887
return nil
@@ -110,16 +109,17 @@ var OKRListCycles = common.Shortcut{
110109
var err error
111110
rangeStart, rangeEnd, err = parseTimeRange(timeRange)
112111
if err != nil {
113-
return common.FlagErrorf("--time-range: %s", err)
112+
return err
114113
}
115114
hasRange = true
116115
}
117116

118117
// Paginated fetch of all cycles
119-
queryParams := make(larkcore.QueryParams)
120-
queryParams.Set("user_id", userID)
121-
queryParams.Set("user_id_type", userIDType)
122-
queryParams.Set("page_size", "100")
118+
queryParams := map[string]interface{}{
119+
"user_id": userID,
120+
"user_id_type": userIDType,
121+
"page_size": "100",
122+
}
123123

124124
var allCycles []Cycle
125125
page := 0
@@ -136,7 +136,7 @@ var OKRListCycles = common.Shortcut{
136136
}
137137
page++
138138

139-
data, err := runtime.DoAPIJSON("GET", "/open-apis/okr/v2/cycles", queryParams, nil)
139+
data, err := runtime.CallAPITyped("GET", "/open-apis/okr/v2/cycles", queryParams, nil)
140140
if err != nil {
141141
return err
142142
}
@@ -158,7 +158,7 @@ var OKRListCycles = common.Shortcut{
158158
if !hasMore || pageToken == "" {
159159
break
160160
}
161-
queryParams.Set("page_token", pageToken)
161+
queryParams["page_token"] = pageToken
162162
}
163163

164164
// Filter by time-range overlap

shortcuts/okr/okr_errors.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
2+
// SPDX-License-Identifier: MIT
3+
4+
package okr
5+
6+
import (
7+
"errors"
8+
9+
"github.com/larksuite/cli/errs"
10+
"github.com/larksuite/cli/extension/fileio"
11+
)
12+
13+
// okrInputStatError maps a FileIO.Stat/Open error for input file validation to
14+
// a typed validation error: path validation failures read as "unsafe file
15+
// path", other errors as "cannot read file".
16+
func okrInputStatError(err error) error {
17+
if err == nil {
18+
return nil
19+
}
20+
if errors.Is(err, fileio.ErrPathValidation) {
21+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe file path: %s", err).
22+
WithParam("--file").
23+
WithCause(err)
24+
}
25+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot read file: %s", err).
26+
WithParam("--file").
27+
WithCause(err)
28+
}
29+
30+
// wrapOkrNetworkErr returns err unchanged when it is already a typed errs.*
31+
// error (preserving subtype / code / log_id from the runtime boundary) and only
32+
// wraps a raw, unclassified error as a transport-level network error.
33+
func wrapOkrNetworkErr(err error, format string, args ...any) error {
34+
if _, ok := errs.ProblemOf(err); ok {
35+
return err
36+
}
37+
return errs.NewNetworkError(errs.SubtypeNetworkTransport, format, args...).WithCause(err)
38+
}

0 commit comments

Comments
 (0)