Skip to content

Commit 8563b1f

Browse files
committed
feat(base): emit typed error envelopes across the base domain
1 parent c000dc3 commit 8563b1f

38 files changed

Lines changed: 688 additions & 412 deletions

.golangci.yml

Lines changed: 5 additions & 5 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/helpers\.go|shortcuts/drive/)
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/helpers\.go|shortcuts/drive/|shortcuts/base/)
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/calendar/helpers\.go|shortcuts/common/mcp_client\.go)
83+
- path-except: (shortcuts/drive/|shortcuts/base/|shortcuts/calendar/helpers\.go|shortcuts/common/mcp_client\.go)
8484
text: errs-no-bare-wrap
8585
linters:
8686
- forbidigo
87-
# errs-no-legacy-helper is drive-only: the shared helpers it bans are
88-
# still used by other domains until their later migration phase.
89-
- path-except: (shortcuts/drive/)
87+
# errs-no-legacy-helper is scoped to migrated shortcut domains: the shared
88+
# helpers it bans are still used by other domains until their later phase.
89+
- path-except: (shortcuts/drive/|shortcuts/base/)
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
@@ -15,6 +15,7 @@ import (
1515
// legacy validation/save helpers are forbidden; callers must use the typed
1616
// common replacements or construct an errs.* typed error directly.
1717
var migratedCommonHelperPaths = []string{
18+
"shortcuts/base/",
1819
"shortcuts/drive/",
1920
}
2021

lint/errscontract/rule_no_legacy_envelope_literal.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
// call sites must return a typed errs.* error instead. Future domains opt in by
1717
// appending their path prefix here.
1818
var migratedEnvelopePaths = []string{
19+
"shortcuts/base/",
1920
"shortcuts/drive/",
2021
}
2122

shortcuts/base/base_advperm_disable.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ var BaseAdvpermDisable = common.Shortcut{
3131
},
3232
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
3333
if strings.TrimSpace(runtime.Str("base-token")) == "" {
34-
return common.FlagErrorf("--base-token must not be blank")
34+
return baseFlagErrorf("--base-token must not be blank")
3535
}
3636
return nil
3737
},
@@ -55,6 +55,6 @@ var BaseAdvpermDisable = common.Shortcut{
5555
return err
5656
}
5757

58-
return handleRoleResponse(runtime, apiResp.RawBody, "disable advanced permissions failed")
58+
return handleRoleAPIResponse(runtime, apiResp, "disable advanced permissions failed")
5959
},
6060
}

shortcuts/base/base_advperm_enable.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ var BaseAdvpermEnable = common.Shortcut{
3030
},
3131
Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
3232
if strings.TrimSpace(runtime.Str("base-token")) == "" {
33-
return common.FlagErrorf("--base-token must not be blank")
33+
return baseFlagErrorf("--base-token must not be blank")
3434
}
3535
return nil
3636
},
@@ -54,6 +54,6 @@ var BaseAdvpermEnable = common.Shortcut{
5454
return err
5555
}
5656

57-
return handleRoleResponse(runtime, apiResp.RawBody, "enable advanced permissions failed")
57+
return handleRoleAPIResponse(runtime, apiResp, "enable advanced permissions failed")
5858
},
5959
}

shortcuts/base/base_advperm_test.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,7 @@ func TestBaseAdvpermEnableExecuteAPIError(t *testing.T) {
196196
},
197197
})
198198
args := []string{"+advperm-enable", "--base-token", "app_x"}
199-
if err := runShortcut(t, BaseAdvpermEnable, args, factory, stdout); err == nil || !strings.Contains(err.Error(), "190001") {
200-
t.Fatalf("err=%v", err)
201-
}
199+
assertProblemCode(t, runShortcut(t, BaseAdvpermEnable, args, factory, stdout), 190001, "bad request")
202200
}
203201

204202
func TestBaseAdvpermDisableExecuteTransportError(t *testing.T) {
@@ -226,7 +224,5 @@ func TestBaseAdvpermDisableExecuteAPIError(t *testing.T) {
226224
},
227225
})
228226
args := []string{"+advperm-disable", "--base-token", "app_x", "--yes"}
229-
if err := runShortcut(t, BaseAdvpermDisable, args, factory, stdout); err == nil || !strings.Contains(err.Error(), "190002") {
230-
t.Fatalf("err=%v", err)
231-
}
227+
assertProblemCode(t, runShortcut(t, BaseAdvpermDisable, args, factory, stdout), 190002, "permission denied")
232228
}

shortcuts/base/base_block_ops.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,24 +55,24 @@ func dryRunBaseBlockDelete(_ context.Context, runtime *common.RuntimeContext) *c
5555

5656
func validateBaseBlockCreate(runtime *common.RuntimeContext) error {
5757
if strings.TrimSpace(runtime.Str("name")) == "" {
58-
return common.FlagErrorf("--name must not be blank")
58+
return baseFlagErrorf("--name must not be blank")
5959
}
6060
if strings.TrimSpace(runtime.Str("type")) == "" {
61-
return common.FlagErrorf("--type must not be blank")
61+
return baseFlagErrorf("--type must not be blank")
6262
}
6363
return nil
6464
}
6565

6666
func validateBaseBlockMove(runtime *common.RuntimeContext) error {
6767
if strings.TrimSpace(runtime.Str("before-id")) != "" && strings.TrimSpace(runtime.Str("after-id")) != "" {
68-
return common.FlagErrorf("--before-id and --after-id are mutually exclusive")
68+
return baseFlagErrorf("--before-id and --after-id are mutually exclusive")
6969
}
7070
return nil
7171
}
7272

7373
func validateBaseBlockRename(runtime *common.RuntimeContext) error {
7474
if strings.TrimSpace(runtime.Str("name")) == "" {
75-
return common.FlagErrorf("--name must not be blank")
75+
return baseFlagErrorf("--name must not be blank")
7676
}
7777
return nil
7878
}

shortcuts/base/base_data_query.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ var BaseDataQuery = common.Shortcut{
3232
dec := json.NewDecoder(bytes.NewReader([]byte(runtime.Str("dsl"))))
3333
dec.UseNumber()
3434
if err := dec.Decode(&dsl); err != nil {
35-
return common.FlagErrorf("--dsl invalid JSON: %v", err)
35+
return baseFlagErrorf("--dsl invalid JSON: %v", err)
3636
}
3737
_, hasDim := dsl["dimensions"]
3838
_, hasMeas := dsl["measures"]
3939
if !hasDim && !hasMeas {
40-
return common.FlagErrorf("--dsl must contain at least one of 'dimensions' or 'measures'")
40+
return baseFlagErrorf("--dsl must contain at least one of 'dimensions' or 'measures'")
4141
}
4242
return nil
4343
},

shortcuts/base/base_errors.go

Lines changed: 169 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,13 @@
44
package base
55

66
import (
7+
"errors"
8+
"fmt"
79
"strings"
810

9-
"github.com/larksuite/cli/internal/output"
11+
"github.com/larksuite/cli/errs"
12+
"github.com/larksuite/cli/extension/fileio"
13+
"github.com/larksuite/cli/internal/errclass"
1014
"github.com/larksuite/cli/internal/util"
1115
)
1216

@@ -24,74 +28,196 @@ func handleBaseAPIResult(result interface{}, err error, action string) (map[stri
2428
// structured ErrAPI, with server-provided message/hint promoted to the top level.
2529
func handleBaseAPIResultAny(result interface{}, err error, action string) (interface{}, error) {
2630
if err != nil {
27-
return nil, output.Errorf(output.ExitAPI, "api_error", "%s: %s", action, err)
31+
return nil, baseAPIBoundaryError(err, action)
2832
}
2933

30-
resultMap, _ := result.(map[string]interface{})
31-
code, _ := util.ToFloat64(resultMap["code"])
34+
resultMap, ok := result.(map[string]interface{})
35+
if !ok || resultMap == nil {
36+
return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "%s: API returned a malformed response envelope", action)
37+
}
38+
if _, exists := resultMap["code"]; !exists {
39+
return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "%s: API response is missing code", action)
40+
}
41+
code, numeric := util.ToFloat64(resultMap["code"])
42+
if !numeric {
43+
return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "%s: API response code is not numeric", action)
44+
}
3245
if code == 0 {
3346
return resultMap["data"], nil
3447
}
3548

36-
larkCode := int(code)
37-
msg := extractDataErrorMessage(resultMap)
38-
if strings.TrimSpace(msg) == "" {
39-
msg, _ = resultMap["msg"].(string)
49+
return nil, baseAPIErrorFromResult(resultMap, errclass.ClassifyContext{})
50+
}
51+
52+
// baseFlagErrorf marks flag-usage failures; it shares baseValidationErrorf's
53+
// typed envelope and exists so call sites read as flag rejections.
54+
func baseFlagErrorf(format string, args ...any) error {
55+
return baseValidationErrorf(format, args...)
56+
}
57+
58+
func baseValidationErrorf(format string, args ...any) error {
59+
msg := fmt.Sprintf(format, args...)
60+
err := errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", msg)
61+
if params := flagParams(msg); len(params) > 0 {
62+
err = err.WithParam(params[0].Name).WithParams(params...)
63+
}
64+
if cause := firstErrorArg(args); cause != nil {
65+
err = err.WithCause(cause)
4066
}
67+
return err
68+
}
4169

42-
detail := extractErrorDetail(resultMap)
43-
apiErr := output.ErrAPI(larkCode, msg, detail)
44-
hint := extractErrorHint(resultMap)
45-
if apiErr.Detail != nil && apiErr.Detail.Hint == "" && hint != "" {
46-
apiErr.Detail.Hint = hint
70+
func flagParams(msg string) []errs.InvalidParam {
71+
reason := msg
72+
seen := map[string]bool{}
73+
params := []errs.InvalidParam{}
74+
for start := strings.Index(msg, "--"); start >= 0; start = strings.Index(msg, "--") {
75+
end := start + 2
76+
for end < len(msg) {
77+
ch := msg[end]
78+
if (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') || ch == '-' {
79+
end++
80+
continue
81+
}
82+
break
83+
}
84+
if end > start+2 {
85+
name := msg[start:end]
86+
if !seen[name] {
87+
seen[name] = true
88+
params = append(params, errs.InvalidParam{Name: name, Reason: reason})
89+
}
90+
}
91+
msg = msg[end:]
4792
}
48-
if apiErr.Detail != nil {
49-
apiErr.Detail.Detail = cleanEmptyBaseErrorDetail(detail)
93+
return params
94+
}
95+
96+
func firstErrorArg(args []any) error {
97+
for _, arg := range args {
98+
if err, ok := arg.(error); ok {
99+
return err
100+
}
50101
}
51-
return nil, apiErr
102+
return nil
103+
}
104+
105+
// baseMissingFileIOError reports a broken runtime wiring: a command that needs
106+
// local file access was constructed without a FileIO provider. The user cannot
107+
// fix this by changing flags, so it classifies as internal, not validation.
108+
func baseMissingFileIOError(format string, args ...any) error {
109+
return errs.NewInternalError(errs.SubtypeFileIO, format, args...)
52110
}
53111

54-
func cleanEmptyBaseErrorDetail(detail interface{}) interface{} {
55-
detailMap, ok := detail.(map[string]interface{})
56-
if !ok {
112+
func baseInputStatError(err error) error {
113+
if err == nil {
57114
return nil
58115
}
59-
for key, value := range detailMap {
60-
if value == nil {
61-
delete(detailMap, key)
62-
}
116+
if errors.Is(err, fileio.ErrPathValidation) {
117+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe file path: %s", err).WithCause(err)
63118
}
64-
if len(detailMap) == 0 {
119+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot read file: %s", err).WithCause(err)
120+
}
121+
122+
func baseSaveError(err error) error {
123+
if err == nil {
65124
return nil
66125
}
67-
return detailMap
126+
var me *fileio.MkdirError
127+
switch {
128+
case errors.Is(err, fileio.ErrPathValidation):
129+
return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", err).WithCause(err)
130+
case errors.As(err, &me):
131+
return errs.NewInternalError(errs.SubtypeFileIO, "cannot create parent directory: %s", err).WithCause(err)
132+
default:
133+
return errs.NewInternalError(errs.SubtypeFileIO, "cannot create file: %s", err).WithCause(err)
134+
}
68135
}
69136

70-
func extractErrorDetail(resultMap map[string]interface{}) interface{} {
71-
if detail, ok := nonNilMapValue(resultMap, "error"); ok {
72-
return detail
137+
func baseAPIBoundaryError(err error, action string) error {
138+
if _, ok := errs.ProblemOf(err); ok {
139+
return err
73140
}
74-
data, _ := resultMap["data"].(map[string]interface{})
75-
if detail, ok := nonNilMapValue(data, "error"); ok {
76-
return detail
141+
return errs.NewNetworkError(errs.SubtypeNetworkTransport, "%s: %s", action, err).WithCause(err)
142+
}
143+
144+
func baseUploadAttachmentError(filePath string, err error) error {
145+
if p, ok := errs.ProblemOf(err); ok {
146+
p.Message = fmt.Sprintf("failed to upload attachment %s: %s", filePath, p.Message)
147+
return err
77148
}
78-
return nil
149+
return errs.NewInternalError(errs.SubtypeSDKError, "failed to upload attachment %s: %s", filePath, err).WithCause(err)
79150
}
80151

81-
func nonNilMapValue(src map[string]interface{}, key string) (interface{}, bool) {
82-
if src == nil {
83-
return nil, false
152+
func baseAPIErrorFromResult(resultMap map[string]interface{}, cc errclass.ClassifyContext) error {
153+
if resultMap == nil {
154+
return errs.NewInternalError(errs.SubtypeInvalidResponse, "API returned a malformed response envelope")
84155
}
85-
value, ok := src[key]
86-
if !ok {
87-
return nil, false
156+
if msg := extractDataErrorMessage(resultMap); msg != "" {
157+
resultMap["msg"] = msg
88158
}
89-
switch value.(type) {
90-
case nil:
91-
return nil, false
92-
default:
93-
return value, true
159+
hint := extractErrorHint(resultMap)
160+
if logID := extractBaseErrorLogID(resultMap); logID != "" {
161+
resultMap["log_id"] = logID
162+
}
163+
err := errclass.BuildAPIError(resultMap, cc)
164+
if err == nil {
165+
return nil
166+
}
167+
if p, ok := errs.ProblemOf(err); ok && hint != "" {
168+
p.Hint = hint
169+
}
170+
return err
171+
}
172+
173+
func enrichBaseAPIErrorFromBody(err error, body []byte, cc errclass.ClassifyContext) error {
174+
if _, ok := errs.ProblemOf(err); !ok {
175+
return err
176+
}
177+
result, parseErr := decodeBaseV3Response(body)
178+
if parseErr != nil {
179+
return err
180+
}
181+
enriched := baseAPIErrorFromResult(result, cc)
182+
if enriched == nil {
183+
return err
184+
}
185+
src, _ := errs.ProblemOf(enriched)
186+
dst, _ := errs.ProblemOf(err)
187+
if src != nil && dst != nil {
188+
dst.Message = src.Message
189+
dst.Hint = src.Hint
190+
// A body without log_id must not erase a header-derived LogID
191+
// already carried by err.
192+
if src.LogID != "" {
193+
dst.LogID = src.LogID
194+
}
94195
}
196+
return err
197+
}
198+
199+
func extractBaseErrorLogID(resultMap map[string]interface{}) string {
200+
for _, key := range []string{"log_id", "logid"} {
201+
if logID, _ := resultMap[key].(string); strings.TrimSpace(logID) != "" {
202+
return strings.TrimSpace(logID)
203+
}
204+
}
205+
if detail, ok := resultMap["error"].(map[string]interface{}); ok {
206+
for _, key := range []string{"log_id", "logid"} {
207+
if logID, _ := detail[key].(string); strings.TrimSpace(logID) != "" {
208+
return strings.TrimSpace(logID)
209+
}
210+
}
211+
}
212+
data, _ := resultMap["data"].(map[string]interface{})
213+
if detail, ok := data["error"].(map[string]interface{}); ok {
214+
for _, key := range []string{"log_id", "logid"} {
215+
if logID, _ := detail[key].(string); strings.TrimSpace(logID) != "" {
216+
return strings.TrimSpace(logID)
217+
}
218+
}
219+
}
220+
return ""
95221
}
96222

97223
func extractErrorHint(resultMap map[string]interface{}) string {

0 commit comments

Comments
 (0)