Skip to content

Commit 0b7e027

Browse files
committed
feat(okr,whiteboard): surface input errors as typed error envelopes
Invalid flags, malformed input payloads, and output-file conflicts in the okr and whiteboard commands now return structured error envelopes — each carrying a stable category, subtype, and the offending flag — instead of flat strings. Scripts and agents can branch on the error shape and exit code rather than scraping messages. API-response errors in these commands keep the existing envelopes for now; they convert in a follow-up once the shared typed API path is available.
1 parent 04932c2 commit 0b7e027

13 files changed

Lines changed: 232 additions & 57 deletions

shortcuts/okr/okr_cycle_detail.go

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

14+
"github.com/larksuite/cli/errs"
1415
"github.com/larksuite/cli/shortcuts/common"
1516
larkcore "github.com/larksuite/oapi-sdk-go/v3/core"
1617
)
@@ -30,10 +31,10 @@ var OKRCycleDetail = common.Shortcut{
3031
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
3132
cycleID := runtime.Str("cycle-id")
3233
if cycleID == "" {
33-
return common.FlagErrorf("--cycle-id is required")
34+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--cycle-id is required").WithParam("--cycle-id")
3435
}
3536
if id, err := strconv.ParseInt(cycleID, 10, 64); err != nil || id <= 0 {
36-
return common.FlagErrorf("--cycle-id must be a positive int64")
37+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--cycle-id must be a positive int64").WithParam("--cycle-id")
3738
}
3839
return nil
3940
},
@@ -71,6 +72,8 @@ var OKRCycleDetail = common.Shortcut{
7172
page++
7273

7374
path := fmt.Sprintf("/open-apis/okr/v2/cycles/%s/objectives", cycleID)
75+
// TODO(errs-migrate): switch to runtime.CallAPITyped once the typed API path lands; today the
76+
// error is the legacy envelope produced inside the shared common.doAPIJSON helper.
7477
data, err := runtime.DoAPIJSON("GET", path, queryParams, nil)
7578
if err != nil {
7679
return err
@@ -123,6 +126,8 @@ var OKRCycleDetail = common.Shortcut{
123126
krPage++
124127

125128
path := fmt.Sprintf("/open-apis/okr/v2/objectives/%s/key_results", obj.ID)
129+
// TODO(errs-migrate): switch to runtime.CallAPITyped once the typed API path lands; today the
130+
// error is the legacy envelope produced inside the shared common.doAPIJSON helper.
126131
data, err := runtime.DoAPIJSON("GET", path, krQuery, nil)
127132
if err != nil {
128133
return err

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: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"strings"
1313
"time"
1414

15+
"github.com/larksuite/cli/errs"
1516
"github.com/larksuite/cli/internal/validate"
1617
"github.com/larksuite/cli/shortcuts/common"
1718
larkcore "github.com/larksuite/oapi-sdk-go/v3/core"
@@ -69,7 +70,7 @@ var OKRListCycles = common.Shortcut{
6970
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
7071
idType := runtime.Str("user-id-type")
7172
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")
73+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--user-id-type must be one of: open_id | union_id | user_id").WithParam("--user-id-type")
7374
}
7475
userID := runtime.Str("user-id")
7576
if err := validate.RejectControlChars(userID, "user-id"); err != nil {
@@ -82,7 +83,7 @@ var OKRListCycles = common.Shortcut{
8283
return err
8384
}
8485
if _, _, err := parseTimeRange(tr); err != nil {
85-
return common.FlagErrorf("--time-range: %s", err)
86+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--time-range: %s", err).WithParam("--time-range")
8687
}
8788
}
8889
return nil
@@ -110,7 +111,7 @@ var OKRListCycles = common.Shortcut{
110111
var err error
111112
rangeStart, rangeEnd, err = parseTimeRange(timeRange)
112113
if err != nil {
113-
return common.FlagErrorf("--time-range: %s", err)
114+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--time-range: %s", err).WithParam("--time-range")
114115
}
115116
hasRange = true
116117
}
@@ -136,6 +137,8 @@ var OKRListCycles = common.Shortcut{
136137
}
137138
page++
138139

140+
// TODO(errs-migrate): switch to runtime.CallAPITyped once the typed API path lands; today the
141+
// error is the legacy envelope produced inside the shared common.doAPIJSON helper.
139142
data, err := runtime.DoAPIJSON("GET", "/open-apis/okr/v2/cycles", queryParams, nil)
140143
if err != nil {
141144
return err

shortcuts/okr/okr_image_upload.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
larkcore "github.com/larksuite/oapi-sdk-go/v3/core"
1616

17+
"github.com/larksuite/cli/errs"
1718
"github.com/larksuite/cli/internal/output"
1819
"github.com/larksuite/cli/shortcuts/common"
1920
)
@@ -43,24 +44,24 @@ var OKRUploadImage = common.Shortcut{
4344
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
4445
filePath := runtime.Str("file")
4546
if filePath == "" {
46-
return common.FlagErrorf("--file is required")
47+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--file is required").WithParam("--file")
4748
}
4849
ext := strings.ToLower(filepath.Ext(filePath))
4950
if !allowedImageExts[ext] {
50-
return common.FlagErrorf("--file must be an image (supported: JPG, JPEG, PNG, GIF, BMP), got %q", ext)
51+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--file must be an image (supported: JPG, JPEG, PNG, GIF, BMP), got %q", ext).WithParam("--file")
5152
}
5253

5354
targetID := runtime.Str("target-id")
5455
if targetID == "" {
55-
return common.FlagErrorf("--target-id is required")
56+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-id is required").WithParam("--target-id")
5657
}
5758
if id, err := strconv.ParseInt(targetID, 10, 64); err != nil || id <= 0 {
58-
return common.FlagErrorf("--target-id must be a positive int64")
59+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-id must be a positive int64").WithParam("--target-id")
5960
}
6061

6162
targetType := runtime.Str("target-type")
6263
if _, ok := targetTypeAllowed[targetType]; !ok {
63-
return common.FlagErrorf("--target-type must be one of: objective | key_result")
64+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-type must be one of: objective | key_result").WithParam("--target-type")
6465
}
6566
return nil
6667
},
@@ -85,6 +86,12 @@ var OKRUploadImage = common.Shortcut{
8586
targetType := runtime.Str("target-type")
8687
targetTypeVal := targetTypeAllowed[targetType]
8788

89+
// TODO(errs-migrate): this whole upload flow is the raw-DoAPI boundary —
90+
// migrate it as one atomic unit once the typed API path lands. Replace the call with
91+
// runtime.CallAPITyped, turn the *output.ExitError pass-through below
92+
// into errs.ProblemOf, and lift the local stat helper to a typed builder.
93+
// Until then it keeps the legacy common.WrapInputStatError / output.Err*
94+
// envelopes so the API classification is not split across two PRs.
8895
info, err := runtime.FileIO().Stat(filePath)
8996
if err != nil {
9097
return common.WrapInputStatError(err)

shortcuts/okr/okr_progress_create.go

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"math"
1212
"strconv"
1313

14+
"github.com/larksuite/cli/errs"
1415
"github.com/larksuite/cli/internal/core"
1516
"github.com/larksuite/cli/internal/validate"
1617
"github.com/larksuite/cli/shortcuts/common"
@@ -39,7 +40,7 @@ func parseCreateProgressRecordParams(runtime *common.RuntimeContext) (*createPro
3940
content := runtime.Str("content")
4041
var cb ContentBlock
4142
if err := json.Unmarshal([]byte(content), &cb); err != nil {
42-
return nil, common.FlagErrorf("--content must be valid ContentBlock JSON: %s", err)
43+
return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--content must be valid ContentBlock JSON: %s", err).WithParam("--content")
4344
}
4445
contentV1 := cb.ToV1()
4546

@@ -60,13 +61,13 @@ func parseCreateProgressRecordParams(runtime *common.RuntimeContext) (*createPro
6061
if v := runtime.Str("progress-percent"); v != "" {
6162
percent, err := strconv.ParseFloat(v, 64)
6263
if err != nil || math.IsNaN(percent) || math.IsInf(percent, 0) || percent < -99999999999 || percent > 99999999999 {
63-
return nil, common.FlagErrorf("--progress-percent must be a number between -99999999999 and 99999999999")
64+
return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-percent must be a number between -99999999999 and 99999999999").WithParam("--progress-percent")
6465
}
6566
progressRate = &ProgressRateV1{Percent: &percent}
6667
if s := runtime.Str("progress-status"); s != "" {
6768
status, ok := ParseProgressStatus(s)
6869
if !ok {
69-
return nil, common.FlagErrorf("--progress-status must be one of: normal | overdue | done")
70+
return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-status must be one of: normal | overdue | done").WithParam("--progress-status")
7071
}
7172
progressRate.Status = int32Ptr(int32(status))
7273
}
@@ -105,31 +106,31 @@ var OKRCreateProgressRecord = common.Shortcut{
105106
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
106107
content := runtime.Str("content")
107108
if content == "" {
108-
return common.FlagErrorf("--content is required")
109+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--content is required").WithParam("--content")
109110
}
110111
if err := validate.RejectControlChars(content, "content"); err != nil {
111112
return err
112113
}
113114
// Validate content is valid JSON and can be parsed as ContentBlock
114115
var cb ContentBlock
115116
if err := json.Unmarshal([]byte(content), &cb); err != nil {
116-
return common.FlagErrorf("--content must be valid ContentBlock JSON: %s", err)
117+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--content must be valid ContentBlock JSON: %s", err).WithParam("--content")
117118
}
118119

119120
targetID := runtime.Str("target-id")
120121
if targetID == "" {
121-
return common.FlagErrorf("--target-id is required")
122+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-id is required").WithParam("--target-id")
122123
}
123124
if err := validate.RejectControlChars(targetID, "target-id"); err != nil {
124125
return err
125126
}
126127
if id, err := strconv.ParseInt(targetID, 10, 64); err != nil || id <= 0 {
127-
return common.FlagErrorf("--target-id must be a positive int64")
128+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-id must be a positive int64").WithParam("--target-id")
128129
}
129130

130131
targetType := runtime.Str("target-type")
131132
if _, ok := targetTypeAllowed[targetType]; !ok {
132-
return common.FlagErrorf("--target-type must be one of: objective | key_result")
133+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--target-type must be one of: objective | key_result").WithParam("--target-type")
133134
}
134135

135136
if v := runtime.Str("source-title"); v != "" {
@@ -146,21 +147,21 @@ var OKRCreateProgressRecord = common.Shortcut{
146147
if v := runtime.Str("progress-percent"); v != "" {
147148
percent, err := strconv.ParseFloat(v, 64)
148149
if err != nil || math.IsNaN(percent) || math.IsInf(percent, 0) || percent < -99999999999 || percent > 99999999999 {
149-
return common.FlagErrorf("--progress-percent must be a number between -99999999999 and 99999999999")
150+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-percent must be a number between -99999999999 and 99999999999").WithParam("--progress-percent")
150151
}
151152
}
152153
if v := runtime.Str("progress-status"); v != "" {
153154
if _, ok := ParseProgressStatus(v); !ok {
154-
return common.FlagErrorf("--progress-status must be one of: normal | overdue | done")
155+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-status must be one of: normal | overdue | done").WithParam("--progress-status")
155156
}
156157
if v := runtime.Str("progress-percent"); v == "" {
157-
return common.FlagErrorf("--progress-percent must provided with --progress-status")
158+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-percent must provided with --progress-status").WithParam("--progress-percent")
158159
}
159160
}
160161

161162
idType := runtime.Str("user-id-type")
162163
if idType != "open_id" && idType != "union_id" && idType != "user_id" {
163-
return common.FlagErrorf("--user-id-type must be one of: open_id | union_id | user_id")
164+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--user-id-type must be one of: open_id | union_id | user_id").WithParam("--user-id-type")
164165
}
165166
return nil
166167
},
@@ -205,6 +206,8 @@ var OKRCreateProgressRecord = common.Shortcut{
205206
queryParams := make(larkcore.QueryParams)
206207
queryParams.Set("user_id_type", p.UserIDType)
207208

209+
// TODO(errs-migrate): switch to runtime.CallAPITyped once the typed API path lands; today the
210+
// error is the legacy envelope produced inside the shared common.doAPIJSON helper.
208211
data, err := runtime.DoAPIJSON("POST", "/open-apis/okr/v1/progress_records/", queryParams, body)
209212
if err != nil {
210213
return err

shortcuts/okr/okr_progress_delete.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"io"
1010
"strconv"
1111

12+
"github.com/larksuite/cli/errs"
1213
"github.com/larksuite/cli/shortcuts/common"
1314
larkcore "github.com/larksuite/oapi-sdk-go/v3/core"
1415
)
@@ -28,10 +29,10 @@ var OKRDeleteProgressRecord = common.Shortcut{
2829
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
2930
progressID := runtime.Str("progress-id")
3031
if progressID == "" {
31-
return common.FlagErrorf("--progress-id is required")
32+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-id is required").WithParam("--progress-id")
3233
}
3334
if id, err := strconv.ParseInt(progressID, 10, 64); err != nil || id <= 0 {
34-
return common.FlagErrorf("--progress-id must be a positive int64")
35+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-id must be a positive int64").WithParam("--progress-id")
3536
}
3637
return nil
3738
},
@@ -46,6 +47,8 @@ var OKRDeleteProgressRecord = common.Shortcut{
4647
progressID := runtime.Str("progress-id")
4748

4849
path := fmt.Sprintf("/open-apis/okr/v1/progress_records/%s", progressID)
50+
// TODO(errs-migrate): switch to runtime.CallAPITyped once the typed API path lands; today the
51+
// error is the legacy envelope produced inside the shared common.doAPIJSON helper.
4952
_, err := runtime.DoAPIJSON("DELETE", path, larkcore.QueryParams{}, nil)
5053
if err != nil {
5154
return err

shortcuts/okr/okr_progress_get.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"io"
1111
"strconv"
1212

13+
"github.com/larksuite/cli/errs"
1314
"github.com/larksuite/cli/shortcuts/common"
1415
larkcore "github.com/larksuite/oapi-sdk-go/v3/core"
1516
)
@@ -30,14 +31,14 @@ var OKRGetProgressRecord = common.Shortcut{
3031
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
3132
progressID := runtime.Str("progress-id")
3233
if progressID == "" {
33-
return common.FlagErrorf("--progress-id is required")
34+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-id is required").WithParam("--progress-id")
3435
}
3536
if id, err := strconv.ParseInt(progressID, 10, 64); err != nil || id <= 0 {
36-
return common.FlagErrorf("--progress-id must be a positive int64")
37+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--progress-id must be a positive int64").WithParam("--progress-id")
3738
}
3839
idType := runtime.Str("user-id-type")
3940
if idType != "open_id" && idType != "union_id" && idType != "user_id" {
40-
return common.FlagErrorf("--user-id-type must be one of: open_id | union_id | user_id")
41+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "--user-id-type must be one of: open_id | union_id | user_id").WithParam("--user-id-type")
4142
}
4243
return nil
4344
},
@@ -60,6 +61,8 @@ var OKRGetProgressRecord = common.Shortcut{
6061
queryParams.Set("user_id_type", userIDType)
6162

6263
path := fmt.Sprintf("/open-apis/okr/v1/progress_records/%s", progressID)
64+
// TODO(errs-migrate): switch to runtime.CallAPITyped once the typed API path lands; today the
65+
// error is the legacy envelope produced inside the shared common.doAPIJSON helper.
6366
data, err := runtime.DoAPIJSON("GET", path, queryParams, nil)
6467
if err != nil {
6568
return err

0 commit comments

Comments
 (0)