diff --git a/.golangci.yml b/.golangci.yml index e74e71b08..84fa6de12 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -73,20 +73,20 @@ linters: - forbidigo # errs-typed-only enforced on paths already migrated to errs.NewXxxError. # Add a path when its migration is complete. - - 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/mail/) + - 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/mail/|shortcuts/base/) text: errs-typed-only linters: - forbidigo # errs-no-bare-wrap enforced on paths fully migrated to typed final # errors. Scoped separately from errs-typed-only because cmd/auth/, # cmd/config/ still have residual fmt.Errorf and must not be caught. - - path-except: (shortcuts/drive/|shortcuts/mail/|shortcuts/calendar/helpers\.go|shortcuts/common/mcp_client\.go) + - path-except: (shortcuts/drive/|shortcuts/mail/|shortcuts/base/|shortcuts/calendar/helpers\.go|shortcuts/common/mcp_client\.go) text: errs-no-bare-wrap linters: - forbidigo # errs-no-legacy-helper is scoped to migrated domains: the shared helpers # it bans are still used by other domains until their later migration phase. - - path-except: (shortcuts/drive/|shortcuts/mail/) + - path-except: (shortcuts/drive/|shortcuts/mail/|shortcuts/base/) text: errs-no-legacy-helper linters: - forbidigo diff --git a/lint/errscontract/rule_no_legacy_common_helper_call.go b/lint/errscontract/rule_no_legacy_common_helper_call.go index d2ba4ecd4..2023c28ac 100644 --- a/lint/errscontract/rule_no_legacy_common_helper_call.go +++ b/lint/errscontract/rule_no_legacy_common_helper_call.go @@ -15,6 +15,7 @@ import ( // legacy validation/save helpers are forbidden; callers must use the typed // common replacements or construct an errs.* typed error directly. var migratedCommonHelperPaths = []string{ + "shortcuts/base/", "shortcuts/drive/", "shortcuts/mail/", } diff --git a/lint/errscontract/rule_no_legacy_envelope_literal.go b/lint/errscontract/rule_no_legacy_envelope_literal.go index 76b9df7eb..57db1cc36 100644 --- a/lint/errscontract/rule_no_legacy_envelope_literal.go +++ b/lint/errscontract/rule_no_legacy_envelope_literal.go @@ -16,6 +16,7 @@ import ( // call sites must return a typed errs.* error instead. Future domains opt in by // appending their path prefix here. var migratedEnvelopePaths = []string{ + "shortcuts/base/", "shortcuts/drive/", "shortcuts/mail/", } diff --git a/shortcuts/base/base_advperm_disable.go b/shortcuts/base/base_advperm_disable.go index 59266bb19..7d403aa58 100644 --- a/shortcuts/base/base_advperm_disable.go +++ b/shortcuts/base/base_advperm_disable.go @@ -31,7 +31,7 @@ var BaseAdvpermDisable = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("base-token")) == "" { - return common.FlagErrorf("--base-token must not be blank") + return baseFlagErrorf("--base-token must not be blank") } return nil }, @@ -55,6 +55,6 @@ var BaseAdvpermDisable = common.Shortcut{ return err } - return handleRoleResponse(runtime, apiResp.RawBody, "disable advanced permissions failed") + return handleRoleAPIResponse(runtime, apiResp, "disable advanced permissions failed") }, } diff --git a/shortcuts/base/base_advperm_enable.go b/shortcuts/base/base_advperm_enable.go index 03a248e23..f7c4d8afc 100644 --- a/shortcuts/base/base_advperm_enable.go +++ b/shortcuts/base/base_advperm_enable.go @@ -30,7 +30,7 @@ var BaseAdvpermEnable = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("base-token")) == "" { - return common.FlagErrorf("--base-token must not be blank") + return baseFlagErrorf("--base-token must not be blank") } return nil }, @@ -54,6 +54,6 @@ var BaseAdvpermEnable = common.Shortcut{ return err } - return handleRoleResponse(runtime, apiResp.RawBody, "enable advanced permissions failed") + return handleRoleAPIResponse(runtime, apiResp, "enable advanced permissions failed") }, } diff --git a/shortcuts/base/base_advperm_test.go b/shortcuts/base/base_advperm_test.go index 9b34a3522..f44c0dd8c 100644 --- a/shortcuts/base/base_advperm_test.go +++ b/shortcuts/base/base_advperm_test.go @@ -196,9 +196,7 @@ func TestBaseAdvpermEnableExecuteAPIError(t *testing.T) { }, }) args := []string{"+advperm-enable", "--base-token", "app_x"} - if err := runShortcut(t, BaseAdvpermEnable, args, factory, stdout); err == nil || !strings.Contains(err.Error(), "190001") { - t.Fatalf("err=%v", err) - } + assertProblemCode(t, runShortcut(t, BaseAdvpermEnable, args, factory, stdout), 190001, "bad request") } func TestBaseAdvpermDisableExecuteTransportError(t *testing.T) { @@ -226,7 +224,5 @@ func TestBaseAdvpermDisableExecuteAPIError(t *testing.T) { }, }) args := []string{"+advperm-disable", "--base-token", "app_x", "--yes"} - if err := runShortcut(t, BaseAdvpermDisable, args, factory, stdout); err == nil || !strings.Contains(err.Error(), "190002") { - t.Fatalf("err=%v", err) - } + assertProblemCode(t, runShortcut(t, BaseAdvpermDisable, args, factory, stdout), 190002, "permission denied") } diff --git a/shortcuts/base/base_block_ops.go b/shortcuts/base/base_block_ops.go index 706368c81..68586f619 100644 --- a/shortcuts/base/base_block_ops.go +++ b/shortcuts/base/base_block_ops.go @@ -55,24 +55,24 @@ func dryRunBaseBlockDelete(_ context.Context, runtime *common.RuntimeContext) *c func validateBaseBlockCreate(runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("name")) == "" { - return common.FlagErrorf("--name must not be blank") + return baseFlagErrorf("--name must not be blank") } if strings.TrimSpace(runtime.Str("type")) == "" { - return common.FlagErrorf("--type must not be blank") + return baseFlagErrorf("--type must not be blank") } return nil } func validateBaseBlockMove(runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("before-id")) != "" && strings.TrimSpace(runtime.Str("after-id")) != "" { - return common.FlagErrorf("--before-id and --after-id are mutually exclusive") + return baseFlagErrorf("--before-id and --after-id are mutually exclusive") } return nil } func validateBaseBlockRename(runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("name")) == "" { - return common.FlagErrorf("--name must not be blank") + return baseFlagErrorf("--name must not be blank") } return nil } diff --git a/shortcuts/base/base_data_query.go b/shortcuts/base/base_data_query.go index 22af5c9fb..616680a7c 100644 --- a/shortcuts/base/base_data_query.go +++ b/shortcuts/base/base_data_query.go @@ -32,12 +32,12 @@ var BaseDataQuery = common.Shortcut{ dec := json.NewDecoder(bytes.NewReader([]byte(runtime.Str("dsl")))) dec.UseNumber() if err := dec.Decode(&dsl); err != nil { - return common.FlagErrorf("--dsl invalid JSON: %v", err) + return baseFlagErrorf("--dsl invalid JSON: %v", err) } _, hasDim := dsl["dimensions"] _, hasMeas := dsl["measures"] if !hasDim && !hasMeas { - return common.FlagErrorf("--dsl must contain at least one of 'dimensions' or 'measures'") + return baseFlagErrorf("--dsl must contain at least one of 'dimensions' or 'measures'") } return nil }, diff --git a/shortcuts/base/base_errors.go b/shortcuts/base/base_errors.go index f70825465..f9ca40596 100644 --- a/shortcuts/base/base_errors.go +++ b/shortcuts/base/base_errors.go @@ -4,9 +4,13 @@ package base import ( + "errors" + "fmt" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/extension/fileio" + "github.com/larksuite/cli/internal/errclass" "github.com/larksuite/cli/internal/util" ) @@ -24,74 +28,196 @@ func handleBaseAPIResult(result interface{}, err error, action string) (map[stri // structured ErrAPI, with server-provided message/hint promoted to the top level. func handleBaseAPIResultAny(result interface{}, err error, action string) (interface{}, error) { if err != nil { - return nil, output.Errorf(output.ExitAPI, "api_error", "%s: %s", action, err) + return nil, baseAPIBoundaryError(err, action) } - resultMap, _ := result.(map[string]interface{}) - code, _ := util.ToFloat64(resultMap["code"]) + resultMap, ok := result.(map[string]interface{}) + if !ok || resultMap == nil { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "%s: API returned a malformed response envelope", action) + } + if _, exists := resultMap["code"]; !exists { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "%s: API response is missing code", action) + } + code, numeric := util.ToFloat64(resultMap["code"]) + if !numeric { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "%s: API response code is not numeric", action) + } if code == 0 { return resultMap["data"], nil } - larkCode := int(code) - msg := extractDataErrorMessage(resultMap) - if strings.TrimSpace(msg) == "" { - msg, _ = resultMap["msg"].(string) + return nil, baseAPIErrorFromResult(resultMap, errclass.ClassifyContext{}) +} + +// baseFlagErrorf marks flag-usage failures; it shares baseValidationErrorf's +// typed envelope and exists so call sites read as flag rejections. +func baseFlagErrorf(format string, args ...any) error { + return baseValidationErrorf(format, args...) +} + +func baseValidationErrorf(format string, args ...any) error { + msg := fmt.Sprintf(format, args...) + err := errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", msg) + if params := flagParams(msg); len(params) > 0 { + err = err.WithParam(params[0].Name).WithParams(params...) + } + if cause := firstErrorArg(args); cause != nil { + err = err.WithCause(cause) } + return err +} - detail := extractErrorDetail(resultMap) - apiErr := output.ErrAPI(larkCode, msg, detail) - hint := extractErrorHint(resultMap) - if apiErr.Detail != nil && apiErr.Detail.Hint == "" && hint != "" { - apiErr.Detail.Hint = hint +func flagParams(msg string) []errs.InvalidParam { + reason := msg + seen := map[string]bool{} + params := []errs.InvalidParam{} + for start := strings.Index(msg, "--"); start >= 0; start = strings.Index(msg, "--") { + end := start + 2 + for end < len(msg) { + ch := msg[end] + if (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') || ch == '-' { + end++ + continue + } + break + } + if end > start+2 { + name := msg[start:end] + if !seen[name] { + seen[name] = true + params = append(params, errs.InvalidParam{Name: name, Reason: reason}) + } + } + msg = msg[end:] } - if apiErr.Detail != nil { - apiErr.Detail.Detail = cleanEmptyBaseErrorDetail(detail) + return params +} + +func firstErrorArg(args []any) error { + for _, arg := range args { + if err, ok := arg.(error); ok { + return err + } } - return nil, apiErr + return nil +} + +// baseMissingFileIOError reports a broken runtime wiring: a command that needs +// local file access was constructed without a FileIO provider. The user cannot +// fix this by changing flags, so it classifies as internal, not validation. +func baseMissingFileIOError(format string, args ...any) error { + return errs.NewInternalError(errs.SubtypeFileIO, format, args...) } -func cleanEmptyBaseErrorDetail(detail interface{}) interface{} { - detailMap, ok := detail.(map[string]interface{}) - if !ok { +func baseInputStatError(err error) error { + if err == nil { return nil } - for key, value := range detailMap { - if value == nil { - delete(detailMap, key) - } + if errors.Is(err, fileio.ErrPathValidation) { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe file path: %s", err).WithCause(err) } - if len(detailMap) == 0 { + return errs.NewValidationError(errs.SubtypeInvalidArgument, "cannot read file: %s", err).WithCause(err) +} + +func baseSaveError(err error) error { + if err == nil { return nil } - return detailMap + var me *fileio.MkdirError + switch { + case errors.Is(err, fileio.ErrPathValidation): + return errs.NewValidationError(errs.SubtypeInvalidArgument, "unsafe output path: %s", err).WithCause(err) + case errors.As(err, &me): + return errs.NewInternalError(errs.SubtypeFileIO, "cannot create parent directory: %s", err).WithCause(err) + default: + return errs.NewInternalError(errs.SubtypeFileIO, "cannot create file: %s", err).WithCause(err) + } } -func extractErrorDetail(resultMap map[string]interface{}) interface{} { - if detail, ok := nonNilMapValue(resultMap, "error"); ok { - return detail +func baseAPIBoundaryError(err error, action string) error { + if _, ok := errs.ProblemOf(err); ok { + return err } - data, _ := resultMap["data"].(map[string]interface{}) - if detail, ok := nonNilMapValue(data, "error"); ok { - return detail + return errs.NewNetworkError(errs.SubtypeNetworkTransport, "%s: %s", action, err).WithCause(err) +} + +func baseUploadAttachmentError(filePath string, err error) error { + if p, ok := errs.ProblemOf(err); ok { + p.Message = fmt.Sprintf("failed to upload attachment %s: %s", filePath, p.Message) + return err } - return nil + return errs.NewInternalError(errs.SubtypeSDKError, "failed to upload attachment %s: %s", filePath, err).WithCause(err) } -func nonNilMapValue(src map[string]interface{}, key string) (interface{}, bool) { - if src == nil { - return nil, false +func baseAPIErrorFromResult(resultMap map[string]interface{}, cc errclass.ClassifyContext) error { + if resultMap == nil { + return errs.NewInternalError(errs.SubtypeInvalidResponse, "API returned a malformed response envelope") } - value, ok := src[key] - if !ok { - return nil, false + if msg := extractDataErrorMessage(resultMap); msg != "" { + resultMap["msg"] = msg } - switch value.(type) { - case nil: - return nil, false - default: - return value, true + hint := extractErrorHint(resultMap) + if logID := extractBaseErrorLogID(resultMap); logID != "" { + resultMap["log_id"] = logID + } + err := errclass.BuildAPIError(resultMap, cc) + if err == nil { + return nil + } + if p, ok := errs.ProblemOf(err); ok && hint != "" { + p.Hint = hint + } + return err +} + +func enrichBaseAPIErrorFromBody(err error, body []byte, cc errclass.ClassifyContext) error { + if _, ok := errs.ProblemOf(err); !ok { + return err + } + result, parseErr := decodeBaseV3Response(body) + if parseErr != nil { + return err + } + enriched := baseAPIErrorFromResult(result, cc) + if enriched == nil { + return err + } + src, _ := errs.ProblemOf(enriched) + dst, _ := errs.ProblemOf(err) + if src != nil && dst != nil { + dst.Message = src.Message + dst.Hint = src.Hint + // A body without log_id must not erase a header-derived LogID + // already carried by err. + if src.LogID != "" { + dst.LogID = src.LogID + } } + return err +} + +func extractBaseErrorLogID(resultMap map[string]interface{}) string { + for _, key := range []string{"log_id", "logid"} { + if logID, _ := resultMap[key].(string); strings.TrimSpace(logID) != "" { + return strings.TrimSpace(logID) + } + } + if detail, ok := resultMap["error"].(map[string]interface{}); ok { + for _, key := range []string{"log_id", "logid"} { + if logID, _ := detail[key].(string); strings.TrimSpace(logID) != "" { + return strings.TrimSpace(logID) + } + } + } + data, _ := resultMap["data"].(map[string]interface{}) + if detail, ok := data["error"].(map[string]interface{}); ok { + for _, key := range []string{"log_id", "logid"} { + if logID, _ := detail[key].(string); strings.TrimSpace(logID) != "" { + return strings.TrimSpace(logID) + } + } + } + return "" } func extractErrorHint(resultMap map[string]interface{}) string { diff --git a/shortcuts/base/base_errors_test.go b/shortcuts/base/base_errors_test.go index e731ec525..61d408371 100644 --- a/shortcuts/base/base_errors_test.go +++ b/shortcuts/base/base_errors_test.go @@ -4,30 +4,15 @@ package base import ( - "errors" "strings" "testing" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/errclass" ) func TestErrorDetailHelpers(t *testing.T) { - if value, ok := nonNilMapValue(nil, "error"); ok || value != nil { - t.Fatalf("nil map should not return value") - } - if value, ok := nonNilMapValue(map[string]interface{}{"error": nil}, "error"); ok || value != nil { - t.Fatalf("nil entry should not return value") - } detail := map[string]interface{}{"message": "boom", "hint": "retry later"} - if value, ok := nonNilMapValue(map[string]interface{}{"error": detail}, "error"); !ok || value == nil { - t.Fatalf("expected non-nil detail") - } - if got := extractErrorDetail(map[string]interface{}{"error": detail}); got == nil { - t.Fatalf("expected root detail") - } - if got := extractErrorDetail(map[string]interface{}{"data": map[string]interface{}{"error": detail}}); got == nil { - t.Fatalf("expected nested detail") - } if got := extractErrorHint(map[string]interface{}{"data": map[string]interface{}{"error": detail}}); got != "retry later" { t.Fatalf("hint=%q", got) } @@ -53,9 +38,12 @@ func TestHandleBaseAPIResultErrorPaths(t *testing.T) { if _, err := handleBaseAPIResultAny(result, nil, "set filter"); err == nil || !strings.Contains(err.Error(), "invalid filter") { t.Fatalf("err=%v", err) } else { - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil || exitErr.Detail.Code != 190001 { - t.Fatalf("expected structured code 190001, got %v", err) + p, ok := errs.ProblemOf(err) + if !ok || p.Code != 190001 { + t.Fatalf("expected typed code 190001, got %T %v", err, err) + } + if p.Hint != "check field name" { + t.Fatalf("hint=%q", p.Hint) } } if _, err := handleBaseAPIResult(result, nil, "set filter"); err == nil { @@ -63,7 +51,7 @@ func TestHandleBaseAPIResultErrorPaths(t *testing.T) { } } -func TestHandleBaseAPIResultCleansBaseErrorDetail(t *testing.T) { +func TestHandleBaseAPIResultPromotesBaseErrorFields(t *testing.T) { result := map[string]interface{}{ "code": 800010407, "msg": "cell value invalid", @@ -87,55 +75,27 @@ func TestHandleBaseAPIResultCleansBaseErrorDetail(t *testing.T) { } _, err := handleBaseAPIResultAny(result, nil, "API call failed") - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured exit error, got %v", err) - } - - errDetail := exitErr.Detail - if errDetail.Code != 800010407 { - t.Fatalf("code=%d", errDetail.Code) - } - if errDetail.Hint != "Provide a number value." { - t.Fatalf("hint=%q", errDetail.Hint) - } - detail, _ := errDetail.Detail.(map[string]interface{}) - if detail == nil { - t.Fatalf("expected cleaned detail, got %#v", errDetail.Detail) + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed error, got %T %v", err, err) } - if _, exists := detail["message"]; exists { - t.Fatalf("detail should not repeat message: %#v", detail) + if p.Code != 800010407 { + t.Fatalf("code=%d", p.Code) } - if _, exists := detail["hint"]; exists { - t.Fatalf("detail should not repeat hint: %#v", detail) + if p.Message != "The cell value does not match the expected input shape." { + t.Fatalf("message=%q", p.Message) } - if _, exists := detail["docs_url"]; exists { - t.Fatalf("detail should omit nil docs_url: %#v", detail) + if p.Hint != "Provide a number value." { + t.Fatalf("hint=%q", p.Hint) } - if detail["level"] != "error" { - t.Fatalf("detail should preserve non-duplicate fields: %#v", detail) - } - if detail["extra_context"] != "future detail field" { - t.Fatalf("detail should pass through unknown non-nil fields: %#v", detail) - } - if detail["path"] != "Amount" || detail["value"] != "abc" { - t.Fatalf("cleaned detail mismatch: %#v", detail) - } - if detail["logid"] != "20260508160000000000000000000000" { - t.Fatalf("logid=%q", detail["logid"]) - } - if retryable, ok := detail["retryable"].(bool); !ok || retryable { - t.Fatalf("retryable=%v", detail["retryable"]) - } - table, _ := detail["table"].(map[string]interface{}) - if table["id"] != "tbl_1" || table["name"] != "Orders" { - t.Fatalf("table=%#v", detail["table"]) + if p.LogID != "20260508160000000000000000000000" { + t.Fatalf("logID=%q", p.LogID) } } -func TestHandleBaseAPIResultAlwaysRemovesMessageAndHintFromDetail(t *testing.T) { +func TestHandleBaseAPIResultClassifiesKnownPermissionCode(t *testing.T) { result := map[string]interface{}{ - "code": output.LarkErrTokenNoPermission, + "code": 99991676, "msg": "permission denied", "data": map[string]interface{}{ "error": map[string]interface{}{ @@ -146,15 +106,15 @@ func TestHandleBaseAPIResultAlwaysRemovesMessageAndHintFromDetail(t *testing.T) } _, err := handleBaseAPIResultAny(result, nil, "API call failed") - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured exit error, got %v", err) + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed error, got %T %v", err, err) } - if exitErr.Detail.Message != "Permission denied [99991676]" { - t.Fatalf("message=%q", exitErr.Detail.Message) + if p.Code != 99991676 { + t.Fatalf("code=%d", p.Code) } - if exitErr.Detail.Detail != nil { - t.Fatalf("detail should be empty after removing message and hint: %#v", exitErr.Detail.Detail) + if p.Category != errs.CategoryAuthorization || p.Subtype != errs.SubtypeTokenScopeInsufficient { + t.Fatalf("category/subtype=%s/%s", p.Category, p.Subtype) } } @@ -167,16 +127,91 @@ func TestAttachBaseResponseLogIDFromHeader(t *testing.T) { attachBaseErrorLogID(result, "20260508170000000000000000000000") _, err := handleBaseAPIResultAny(result, nil, "API call failed") - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured exit error, got %v", err) + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed error, got %T %v", err, err) + } + if p.LogID != "20260508170000000000000000000000" { + t.Fatalf("logID=%q", p.LogID) + } +} + +func TestHandleBaseAPIResultRejectsNonNumericCode(t *testing.T) { + for _, code := range []interface{}{"oops", map[string]interface{}{}, nil} { + result := map[string]interface{}{"code": code, "msg": "weird envelope"} + _, err := handleBaseAPIResultAny(result, nil, "list tables") + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("code=%#v: expected typed error, got %T %v", code, err, err) + } + if p.Category != errs.CategoryInternal || p.Subtype != errs.SubtypeInvalidResponse { + t.Fatalf("code=%#v: category/subtype=%s/%s", code, p.Category, p.Subtype) + } + if !strings.Contains(p.Message, "list tables") { + t.Fatalf("code=%#v: message=%q", code, p.Message) + } + } +} + +func TestEnrichBaseAPIErrorFromBodyLogIDMerge(t *testing.T) { + t.Run("body without log_id keeps header-derived LogID", func(t *testing.T) { + outer := errs.NewAPIError(errs.SubtypeUnknown, "outer failure").WithCode(190001).WithLogID("header-log-id") + err := enrichBaseAPIErrorFromBody(outer, []byte(`{"code":190001,"msg":"boom"}`), errclass.ClassifyContext{}) + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed error, got %T %v", err, err) + } + if p.Message != "boom" { + t.Fatalf("message=%q", p.Message) + } + if p.LogID != "header-log-id" { + t.Fatalf("logID=%q, want header-log-id", p.LogID) + } + }) + + t.Run("body log_id overrides header-derived LogID", func(t *testing.T) { + outer := errs.NewAPIError(errs.SubtypeUnknown, "outer failure").WithCode(190001).WithLogID("header-log-id") + body := `{"code":190001,"msg":"boom","data":{"error":{"logid":"body-log-id"}}}` + err := enrichBaseAPIErrorFromBody(outer, []byte(body), errclass.ClassifyContext{}) + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed error, got %T %v", err, err) + } + if p.LogID != "body-log-id" { + t.Fatalf("logID=%q, want body-log-id", p.LogID) + } + }) +} + +func TestBaseMissingFileIOErrorIsInternal(t *testing.T) { + p, ok := errs.ProblemOf(baseMissingFileIOError("file operations require a FileIO provider")) + if !ok { + t.Fatal("expected typed error") } - detail, _ := exitErr.Detail.Detail.(map[string]interface{}) - if detail["logid"] != "20260508170000000000000000000000" { - t.Fatalf("logid=%q", detail["logid"]) + if p.Category != errs.CategoryInternal || p.Subtype != errs.SubtypeFileIO { + t.Fatalf("category/subtype=%s/%s", p.Category, p.Subtype) } } type assertErr struct{} func (assertErr) Error() string { return "network timeout" } + +func assertProblemCode(t *testing.T, err error, code int, messageParts ...string) { + t.Helper() + if err == nil { + t.Fatalf("expected error with code %d", code) + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed problem, got %T %v", err, err) + } + if p.Code != code { + t.Fatalf("code=%d, want %d; err=%v", p.Code, code, err) + } + for _, part := range messageParts { + if !strings.Contains(p.Message, part) { + t.Fatalf("message=%q missing %q", p.Message, part) + } + } +} diff --git a/shortcuts/base/base_execute_test.go b/shortcuts/base/base_execute_test.go index 1d5ca1587..22626be2a 100644 --- a/shortcuts/base/base_execute_test.go +++ b/shortcuts/base/base_execute_test.go @@ -18,6 +18,7 @@ import ( "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/httpmock" @@ -513,6 +514,65 @@ func TestBaseBlockExecuteShortcuts(t *testing.T) { } } +func TestBaseBlockValidationReturnsTypedErrors(t *testing.T) { + factory, stdout, _ := newExecuteFactory(t) + tests := []struct { + name string + shortcut common.Shortcut + args []string + params []string + }{ + { + name: "create blank name", + shortcut: BaseBaseBlockCreate, + args: []string{"+base-block-create", "--base-token", "app_x", "--type", "docx", "--name", " "}, + params: []string{"--name"}, + }, + { + name: "move conflicting sibling anchors", + shortcut: BaseBaseBlockMove, + args: []string{"+base-block-move", "--base-token", "app_x", "--block-id", "blk_doc", "--before-id", "blk_a", "--after-id", "blk_b"}, + params: []string{"--before-id", "--after-id"}, + }, + { + name: "rename blank name", + shortcut: BaseBaseBlockRename, + args: []string{"+base-block-rename", "--base-token", "app_x", "--block-id", "blk_doc", "--name", " "}, + params: []string{"--name"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := runShortcut(t, tt.shortcut, tt.args, factory, stdout) + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed problem, got %T %v", err, err) + } + if p.Category != errs.CategoryValidation || p.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("category/subtype=%s/%s", p.Category, p.Subtype) + } + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) { + t.Fatalf("expected ValidationError, got %T %v", err, err) + } + if validationErr.Param != tt.params[0] { + t.Fatalf("param=%q, want %q", validationErr.Param, tt.params[0]) + } + if len(validationErr.Params) != len(tt.params) { + t.Fatalf("params=%#v, want %v", validationErr.Params, tt.params) + } + for i, param := range tt.params { + if validationErr.Params[i].Name != param { + t.Fatalf("params=%#v, want %v", validationErr.Params, tt.params) + } + if validationErr.Params[i].Reason == "" { + t.Fatalf("params[%d] missing reason: %#v", i, validationErr.Params) + } + } + }) + } +} + func TestBaseHistoryExecute(t *testing.T) { factory, stdout, reg := newExecuteFactory(t) reg.Register(&httpmock.Stub{ @@ -871,10 +931,10 @@ func TestBaseTableExecuteReadAndDelete(t *testing.T) { t.Run("list-http-404", func(t *testing.T) { factory, stdout, reg := newExecuteFactory(t) reg.Register(&httpmock.Stub{ - Method: "GET", - URL: "/open-apis/base/v3/bases/app_x/tables", - Status: 404, - Body: "404 page not found", + Method: "GET", + URL: "/open-apis/base/v3/bases/app_x/tables", + Status: 404, + RawBody: []byte("404 page not found"), Headers: map[string][]string{ "Content-Type": {"text/plain"}, }, @@ -2093,6 +2153,9 @@ func TestBaseRecordExecuteReadCreateDelete(t *testing.T) { if !strings.Contains(err.Error(), "exceeds 2GB limit") { t.Fatalf("err=%v", err) } + if !strings.Contains(err.Error(), filepath.Base(tmpFile.Name())) { + t.Fatalf("err=%v should name the offending file", err) + } }) t.Run("upload attachment rejects deprecated name flag", func(t *testing.T) { @@ -2262,6 +2325,23 @@ func TestBaseRecordExecuteReadCreateDelete(t *testing.T) { } }) + t.Run("download surfaces unsafe output path instead of directory hint", func(t *testing.T) { + factory, stdout, _ := newExecuteFactory(t) + tmpDir := t.TempDir() + withBaseWorkingDir(t, tmpDir) + + err := runShortcut(t, BaseRecordDownloadAttachment, []string{ + "+record-download-attachment", + "--base-token", "app_x", + "--table-id", "tbl_x", + "--record-id", "rec_x", + "--output", "../escape", + }, factory, stdout) + if err == nil || !strings.Contains(err.Error(), "unsafe output path") { + t.Fatalf("err=%v", err) + } + }) + t.Run("download all disambiguates duplicate attachment names with file token", func(t *testing.T) { factory, stdout, reg := newExecuteFactory(t) reg.Register(&httpmock.Stub{ @@ -2458,21 +2538,37 @@ func TestBaseRecordExecuteReadCreateDelete(t *testing.T) { "--record-id", "rec_x", "--output", "downloads", }, factory, stdout) - if err == nil || !strings.Contains(err.Error(), "download failed after 1 attachment(s) succeeded and 1 failed") { + if err == nil { t.Fatalf("err=%v", err) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected structured error, got %T %v", err, err) + var partialErr *output.PartialFailureError + if !errors.As(err, &partialErr) { + t.Fatalf("expected partial failure error, got %T %v", err, err) + } + + var envelope map[string]interface{} + if err := json.Unmarshal(stdout.Bytes(), &envelope); err != nil { + t.Fatalf("failed to decode partial failure output: %v\nraw=%s", err, stdout.String()) + } + if envelope["ok"] != false { + t.Fatalf("ok=%#v, want false; envelope=%#v", envelope["ok"], envelope) + } + data, _ := envelope["data"].(map[string]interface{}) + if msg, _ := data["message"].(string); !strings.Contains(msg, "download failed after 1 attachment(s) succeeded and 1 failed") { + t.Fatalf("message=%q", msg) + } + downloaded, _ := data["downloaded"].([]interface{}) + failed, _ := data["failed"].([]interface{}) + if len(downloaded) != 1 || len(failed) != 1 { + t.Fatalf("data=%#v", data) } - detail, _ := exitErr.Detail.Detail.(map[string]interface{}) - downloaded, _ := detail["downloaded"].([]map[string]interface{}) - failed, _ := detail["failed"].([]map[string]interface{}) - if len(downloaded) != 1 || downloaded[0]["file_token"] != "box_a" || len(failed) != 1 || failed[0]["file_token"] != "box_b" { - t.Fatalf("detail=%#v", exitErr.Detail.Detail) + downloadedItem, _ := downloaded[0].(map[string]interface{}) + failedItem, _ := failed[0].(map[string]interface{}) + if downloadedItem["file_token"] != "box_a" || failedItem["file_token"] != "box_b" { + t.Fatalf("data=%#v", data) } - if detail["log_id"] != "202605270001" { - t.Fatalf("detail=%#v, want log_id", exitErr.Detail.Detail) + if data["log_id"] != "202605270001" { + t.Fatalf("data=%#v, want log_id", data) } if _, err := os.Stat(filepath.Join(tmpDir, "downloads", "a.txt")); err != nil { t.Fatalf("expected first file to remain: %v", err) diff --git a/shortcuts/base/base_form_questions_create.go b/shortcuts/base/base_form_questions_create.go index c9c796422..bd60ac997 100644 --- a/shortcuts/base/base_form_questions_create.go +++ b/shortcuts/base/base_form_questions_create.go @@ -42,7 +42,7 @@ var BaseFormQuestionsCreate = common.Shortcut{ var questions []interface{} if err := json.Unmarshal([]byte(questionsJSON), &questions); err != nil { - return output.Errorf(output.ExitValidation, "invalid_json", "--questions must be a valid JSON array: %s", err) + return baseValidationErrorf("--questions must be a valid JSON array: %s", err) } data, err := baseV3Call(runtime, "POST", diff --git a/shortcuts/base/base_form_questions_delete.go b/shortcuts/base/base_form_questions_delete.go index 8b1472220..b2875e462 100644 --- a/shortcuts/base/base_form_questions_delete.go +++ b/shortcuts/base/base_form_questions_delete.go @@ -7,7 +7,6 @@ import ( "context" "encoding/json" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -43,7 +42,7 @@ var BaseFormQuestionsDelete = common.Shortcut{ var questionIds []string if err := json.Unmarshal([]byte(questionIdsJSON), &questionIds); err != nil { - return output.Errorf(output.ExitValidation, "invalid_json", "--question-ids must be a valid JSON array of strings: %s", err) + return baseValidationErrorf("--question-ids must be a valid JSON array of strings: %s", err) } _, err := baseV3Call(runtime, "DELETE", diff --git a/shortcuts/base/base_form_questions_update.go b/shortcuts/base/base_form_questions_update.go index 1abc35bef..77831e5e5 100644 --- a/shortcuts/base/base_form_questions_update.go +++ b/shortcuts/base/base_form_questions_update.go @@ -42,7 +42,7 @@ var BaseFormQuestionsUpdate = common.Shortcut{ var questions []interface{} if err := json.Unmarshal([]byte(questionsJSON), &questions); err != nil { - return output.Errorf(output.ExitValidation, "invalid_json", "--questions must be a valid JSON array: %s", err) + return baseValidationErrorf("--questions must be a valid JSON array: %s", err) } data, err := baseV3Call(runtime, "PATCH", diff --git a/shortcuts/base/base_form_submit.go b/shortcuts/base/base_form_submit.go index 7c1aeb173..e23284954 100644 --- a/shortcuts/base/base_form_submit.go +++ b/shortcuts/base/base_form_submit.go @@ -14,7 +14,6 @@ import ( "golang.org/x/sync/errgroup" "github.com/larksuite/cli/extension/fileio" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -62,31 +61,31 @@ func validateFormSubmit(runtime *common.RuntimeContext) error { attachments, hasAttachments := raw["attachments"] if !hasAttachments && fields == nil { - return common.FlagErrorf("--json must contain at least \"fields\" or \"attachments\"") + return baseFlagErrorf("--json must contain at least \"fields\" or \"attachments\"") } if hasAttachments { // 有附件时 --base-token 必填（上传附件到 Base Drive Media 需要） if runtime.Str("base-token") == "" { - return common.FlagErrorf("--base-token is required when --json contains \"attachments\"") + return baseFlagErrorf("--base-token is required when --json contains \"attachments\"") } attMap, ok := attachments.(map[string]interface{}) if !ok { - return common.FlagErrorf("--json.attachments must be a JSON object mapping field names to file path arrays") + return baseFlagErrorf("--json.attachments must be a JSON object mapping field names to file path arrays") } for fieldName, value := range attMap { paths, ok := value.([]interface{}) if !ok { - return common.FlagErrorf("--json.attachments.%q must be a file path array, got %T", fieldName, value) + return baseFlagErrorf("--json.attachments.%q must be a file path array, got %T", fieldName, value) } for i, item := range paths { if _, ok := item.(string); !ok { - return common.FlagErrorf("--json.attachments.%q[%d] must be a file path string, got %T", fieldName, i, item) + return baseFlagErrorf("--json.attachments.%q[%d] must be a file path string, got %T", fieldName, i, item) } } if len(paths) == 0 { - return common.FlagErrorf("--json.attachments.%q must not be empty; remove it or provide at least one file path", fieldName) + return baseFlagErrorf("--json.attachments.%q must not be empty; remove it or provide at least one file path", fieldName) } } } @@ -111,21 +110,21 @@ func parseFormSubmitJSON(runtime *common.RuntimeContext) (map[string]interface{} if attachments, ok := raw["attachments"]; ok { attObj, ok := attachments.(map[string]interface{}) if !ok { - return nil, nil, common.FlagErrorf(`--json.attachments must be a JSON object mapping field names to file path arrays`) + return nil, nil, baseFlagErrorf(`--json.attachments must be a JSON object mapping field names to file path arrays`) } if len(attObj) > 0 { attMap = make(map[string][]string, len(attObj)) for fieldName, value := range attObj { paths, ok := value.([]interface{}) if !ok { - return nil, nil, common.FlagErrorf("--json.attachments.%q must be a file path array, got %T", fieldName, value) + return nil, nil, baseFlagErrorf("--json.attachments.%q must be a file path array, got %T", fieldName, value) } filePaths := make([]string, 0, len(paths)) for _, item := range paths { if s, ok := item.(string); ok { filePaths = append(filePaths, s) } else { - return nil, nil, common.FlagErrorf("--json.attachments.%q must contain file path strings only, got %T", fieldName, item) + return nil, nil, baseFlagErrorf("--json.attachments.%q must contain file path strings only, got %T", fieldName, item) } } if len(filePaths) > 0 { @@ -195,33 +194,33 @@ func executeFormSubmit(runtime *common.RuntimeContext) error { baseToken := runtime.Str("base-token") fio := runtime.FileIO() if fio == nil { - return output.ErrValidation("file operations require a FileIO provider (needed for attachments in --json)") + return baseMissingFileIOError("file operations require a FileIO provider (needed for attachments in --json)") } // Step 1: 收集所有唯一路径（跨字段去重） allPaths := collectUniquePaths(attachmentMap) if len(allPaths) == 0 { - return common.FlagErrorf("attachments in --json contains no valid file paths") + return baseFlagErrorf("attachments in --json contains no valid file paths") } // Step 2: 前置校验所有文件路径安全性与可访问性，同时收集文件大小供上传使用 sizeMap := make(map[string]int64, len(allPaths)) for _, filePath := range allPaths { if _, err := validate.SafeInputPath(filePath); err != nil { - return output.ErrValidation("unsafe attachment file path: %s: %v", filePath, err) + return baseValidationErrorf("unsafe attachment file path: %s: %v", filePath, err) } fileInfo, err := fio.Stat(filePath) if err != nil { if errors.Is(err, fileio.ErrPathValidation) { - return output.ErrValidation("unsafe attachment file path: %s: %v", filePath, err) + return baseValidationErrorf("unsafe attachment file path: %s: %v", filePath, err) } - return output.ErrValidation("attachment file not accessible: %s: %v", filePath, err) + return baseValidationErrorf("attachment file not accessible: %s: %v", filePath, err) } if fileInfo.Size() > baseAttachmentUploadMaxFileSize { - return output.ErrValidation("attachment file %s exceeds 2GB limit", filePath) + return baseValidationErrorf("attachment file %s exceeds 2GB limit", filePath) } if !fileInfo.Mode().IsRegular() { - return output.ErrValidation("attachment file %s is not a regular file", filePath) + return baseValidationErrorf("attachment file %s is not a regular file", filePath) } sizeMap[filePath] = fileInfo.Size() } @@ -328,7 +327,7 @@ func uploadAttachmentsParallel(runtime *common.RuntimeContext, paths []string, t func uploadSingleAttachment(runtime *common.RuntimeContext, filePath, fileName string, fileSize int64, target baseAttachmentUploadTarget) (interface{}, error) { att, err := uploadAttachmentToBase(runtime, filePath, fileName, fileSize, target) if err != nil { - return nil, fmt.Errorf("failed to upload attachment %s: %w", filePath, err) + return nil, baseUploadAttachmentError(filePath, err) } return att, nil } diff --git a/shortcuts/base/base_role_common.go b/shortcuts/base/base_role_common.go index ab2190910..137ffb3fd 100644 --- a/shortcuts/base/base_role_common.go +++ b/shortcuts/base/base_role_common.go @@ -5,9 +5,10 @@ package base import ( "encoding/json" - "fmt" - "github.com/larksuite/cli/internal/output" + larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -17,6 +18,14 @@ import ( // - Inner: business-level code/message inside the data object // // The data field may be a JSON object (actual behavior) or a JSON string (per doc). +func handleRoleAPIResponse(runtime *common.RuntimeContext, apiResp *larkcore.ApiResp, action string) error { + if _, err := runtime.ClassifyAPIResponse(apiResp); err != nil { + enriched := enrichBaseAPIErrorFromBody(err, apiResp.RawBody, runtime.APIClassifyContext()) + return prefixRoleActionError(enriched, action) + } + return handleRoleResponse(runtime, apiResp.RawBody, action) +} + func handleRoleResponse(runtime *common.RuntimeContext, rawBody []byte, action string) error { var resp struct { Code int `json:"code"` @@ -24,23 +33,17 @@ func handleRoleResponse(runtime *common.RuntimeContext, rawBody []byte, action s Data json.RawMessage `json:"data"` } if err := json.Unmarshal(rawBody, &resp); err != nil { - return fmt.Errorf("failed to parse response: %v", err) + return errs.NewInternalError(errs.SubtypeInvalidResponse, "%s: failed to parse response: %v", action, err).WithCause(err) } if resp.Code != 0 { - msg := resp.Msg - // When outer msg is empty, try to extract error details from data.error.message - if msg == "" && len(resp.Data) > 0 { - var errData struct { - Error struct { - Message string `json:"message"` - Hint string `json:"hint"` - } `json:"error"` - } - if json.Unmarshal(resp.Data, &errData) == nil && errData.Error.Message != "" { - msg = errData.Error.Message + result := map[string]interface{}{"code": resp.Code, "msg": resp.Msg} + if len(resp.Data) > 0 { + var data interface{} + if json.Unmarshal(resp.Data, &data) == nil { + result["data"] = data } } - return output.ErrAPI(resp.Code, fmt.Sprintf("%s: [%d] %s", action, resp.Code, msg), nil) + return baseRoleAPIError(runtime, result, action) } if len(resp.Data) == 0 || string(resp.Data) == "null" || string(resp.Data) == `""` { @@ -75,7 +78,8 @@ func handleRoleResponse(runtime *common.RuntimeContext, rawBody []byte, action s } if codeInt != 0 { msg, _ := m["message"].(string) - return output.ErrAPI(codeInt, fmt.Sprintf("%s: [%d] %s", action, codeInt, msg), nil) + result := map[string]interface{}{"code": codeInt, "msg": msg, "data": m} + return baseRoleAPIError(runtime, result, action) } // code == 0, extract the inner data if present if innerData, hasInner := m["data"]; hasInner { @@ -98,3 +102,20 @@ func handleRoleResponse(runtime *common.RuntimeContext, rawBody []byte, action s runtime.Out(data, nil) return nil } + +func baseRoleAPIError(runtime *common.RuntimeContext, result map[string]interface{}, action string) error { + return prefixRoleActionError(baseAPIErrorFromResult(result, runtime.APIClassifyContext()), action) +} + +// prefixRoleActionError prepends the failed role action ("create role failed", +// "get role failed", ...) to a typed error's message so both the classified +// outer-response path and the parsed-body path carry the same context. +func prefixRoleActionError(err error, action string) error { + if err == nil { + return nil + } + if p, ok := errs.ProblemOf(err); ok && action != "" { + p.Message = action + ": " + p.Message + } + return err +} diff --git a/shortcuts/base/base_role_create.go b/shortcuts/base/base_role_create.go index cf887ece5..7e557d59f 100644 --- a/shortcuts/base/base_role_create.go +++ b/shortcuts/base/base_role_create.go @@ -34,11 +34,11 @@ var BaseRoleCreate = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("base-token")) == "" { - return common.FlagErrorf("--base-token must not be blank") + return baseFlagErrorf("--base-token must not be blank") } var body map[string]any if err := json.Unmarshal([]byte(runtime.Str("json")), &body); err != nil { - return common.FlagErrorf("--json must be valid JSON: %v", err) + return baseFlagErrorf("--json must be valid JSON: %v", err) } return nil }, @@ -64,6 +64,6 @@ var BaseRoleCreate = common.Shortcut{ return err } - return handleRoleResponse(runtime, apiResp.RawBody, "create role failed") + return handleRoleAPIResponse(runtime, apiResp, "create role failed") }, } diff --git a/shortcuts/base/base_role_delete.go b/shortcuts/base/base_role_delete.go index 9d7f14c4b..c36de7c5b 100644 --- a/shortcuts/base/base_role_delete.go +++ b/shortcuts/base/base_role_delete.go @@ -34,10 +34,10 @@ var BaseRoleDelete = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("base-token")) == "" { - return common.FlagErrorf("--base-token must not be blank") + return baseFlagErrorf("--base-token must not be blank") } if strings.TrimSpace(runtime.Str("role-id")) == "" { - return common.FlagErrorf("--role-id must not be blank") + return baseFlagErrorf("--role-id must not be blank") } return nil }, @@ -60,6 +60,6 @@ var BaseRoleDelete = common.Shortcut{ return err } - return handleRoleResponse(runtime, apiResp.RawBody, "delete role failed") + return handleRoleAPIResponse(runtime, apiResp, "delete role failed") }, } diff --git a/shortcuts/base/base_role_get.go b/shortcuts/base/base_role_get.go index 626b7f925..a064ceec6 100644 --- a/shortcuts/base/base_role_get.go +++ b/shortcuts/base/base_role_get.go @@ -33,10 +33,10 @@ var BaseRoleGet = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("base-token")) == "" { - return common.FlagErrorf("--base-token must not be blank") + return baseFlagErrorf("--base-token must not be blank") } if strings.TrimSpace(runtime.Str("role-id")) == "" { - return common.FlagErrorf("--role-id must not be blank") + return baseFlagErrorf("--role-id must not be blank") } return nil }, @@ -58,6 +58,6 @@ var BaseRoleGet = common.Shortcut{ return err } - return handleRoleResponse(runtime, apiResp.RawBody, "get role failed") + return handleRoleAPIResponse(runtime, apiResp, "get role failed") }, } diff --git a/shortcuts/base/base_role_list.go b/shortcuts/base/base_role_list.go index 110192c12..93a52e9fb 100644 --- a/shortcuts/base/base_role_list.go +++ b/shortcuts/base/base_role_list.go @@ -32,7 +32,7 @@ var BaseRoleList = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("base-token")) == "" { - return common.FlagErrorf("--base-token must not be blank") + return baseFlagErrorf("--base-token must not be blank") } return nil }, @@ -52,6 +52,6 @@ var BaseRoleList = common.Shortcut{ return err } - return handleRoleResponse(runtime, apiResp.RawBody, "list roles failed") + return handleRoleAPIResponse(runtime, apiResp, "list roles failed") }, } diff --git a/shortcuts/base/base_role_test.go b/shortcuts/base/base_role_test.go index 1b998cea2..17f8a601a 100644 --- a/shortcuts/base/base_role_test.go +++ b/shortcuts/base/base_role_test.go @@ -375,9 +375,7 @@ func TestBaseRoleCreateExecuteAPIError(t *testing.T) { }, }) args := []string{"+role-create", "--base-token", "app_x", "--json", `{"role_name":"Bad"}`} - if err := runShortcut(t, BaseRoleCreate, args, factory, stdout); err == nil || !strings.Contains(err.Error(), "190001") { - t.Fatalf("err=%v", err) - } + assertProblemCode(t, runShortcut(t, BaseRoleCreate, args, factory, stdout), 190001, "create role failed", "bad request") } func TestBaseRoleListExecuteTransportError(t *testing.T) { @@ -405,9 +403,7 @@ func TestBaseRoleListExecuteAPIError(t *testing.T) { }, }) args := []string{"+role-list", "--base-token", "app_x"} - if err := runShortcut(t, BaseRoleList, args, factory, stdout); err == nil || !strings.Contains(err.Error(), "190002") { - t.Fatalf("err=%v", err) - } + assertProblemCode(t, runShortcut(t, BaseRoleList, args, factory, stdout), 190002, "not found") } func TestBaseRoleDeleteExecuteAPIError(t *testing.T) { @@ -421,9 +417,7 @@ func TestBaseRoleDeleteExecuteAPIError(t *testing.T) { }, }) args := []string{"+role-delete", "--base-token", "app_x", "--role-id", "rol_1", "--yes"} - if err := runShortcut(t, BaseRoleDelete, args, factory, stdout); err == nil || !strings.Contains(err.Error(), "190003") { - t.Fatalf("err=%v", err) - } + assertProblemCode(t, runShortcut(t, BaseRoleDelete, args, factory, stdout), 190003, "forbidden") } func TestBaseRoleUpdateExecuteAPIError(t *testing.T) { @@ -437,9 +431,7 @@ func TestBaseRoleUpdateExecuteAPIError(t *testing.T) { }, }) args := []string{"+role-update", "--base-token", "app_x", "--role-id", "rol_1", "--json", `{"role_name":"X"}`, "--yes"} - if err := runShortcut(t, BaseRoleUpdate, args, factory, stdout); err == nil || !strings.Contains(err.Error(), "190004") { - t.Fatalf("err=%v", err) - } + assertProblemCode(t, runShortcut(t, BaseRoleUpdate, args, factory, stdout), 190004, "invalid params") } func TestBaseRoleGetExecuteBusinessError(t *testing.T) { @@ -457,9 +449,7 @@ func TestBaseRoleGetExecuteBusinessError(t *testing.T) { }, }) args := []string{"+role-get", "--base-token", "app_x", "--role-id", "rol_bad"} - if err := runShortcut(t, BaseRoleGet, args, factory, stdout); err == nil || !strings.Contains(err.Error(), "100001") || !strings.Contains(err.Error(), "role not found") { - t.Fatalf("err=%v", err) - } + assertProblemCode(t, runShortcut(t, BaseRoleGet, args, factory, stdout), 100001, "role not found") } // --------------------------------------------------------------------------- @@ -487,9 +477,7 @@ func TestHandleRoleResponse(t *testing.T) { t.Run("outer error code", func(t *testing.T) { rt := newRoleResponseRuntime(t) - if err := handleRoleResponse(rt, []byte(`{"code":999,"msg":"outer error"}`), "test"); err == nil || !strings.Contains(err.Error(), "999") { - t.Fatalf("err=%v", err) - } + assertProblemCode(t, handleRoleResponse(rt, []byte(`{"code":999,"msg":"outer error"}`), "test"), 999, "outer error") }) t.Run("outer error code with empty msg and data.error.message", func(t *testing.T) { @@ -574,9 +562,7 @@ func TestHandleRoleResponse(t *testing.T) { t.Run("business code non-zero", func(t *testing.T) { rt := newRoleResponseRuntime(t) body := `{"code":0,"msg":"ok","data":{"code":50001,"message":"permission denied"}}` - if err := handleRoleResponse(rt, []byte(body), "test"); err == nil || !strings.Contains(err.Error(), "50001") { - t.Fatalf("err=%v", err) - } + assertProblemCode(t, handleRoleResponse(rt, []byte(body), "test"), 50001, "permission denied") }) t.Run("data is array", func(t *testing.T) { diff --git a/shortcuts/base/base_role_update.go b/shortcuts/base/base_role_update.go index 30e1e4ac3..1baa96705 100644 --- a/shortcuts/base/base_role_update.go +++ b/shortcuts/base/base_role_update.go @@ -36,14 +36,14 @@ var BaseRoleUpdate = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("base-token")) == "" { - return common.FlagErrorf("--base-token must not be blank") + return baseFlagErrorf("--base-token must not be blank") } if strings.TrimSpace(runtime.Str("role-id")) == "" { - return common.FlagErrorf("--role-id must not be blank") + return baseFlagErrorf("--role-id must not be blank") } var body map[string]any if err := json.Unmarshal([]byte(runtime.Str("json")), &body); err != nil { - return common.FlagErrorf("--json must be valid JSON: %v", err) + return baseFlagErrorf("--json must be valid JSON: %v", err) } return nil }, @@ -72,6 +72,6 @@ var BaseRoleUpdate = common.Shortcut{ return err } - return handleRoleResponse(runtime, apiResp.RawBody, "update role failed") + return handleRoleAPIResponse(runtime, apiResp, "update role failed") }, } diff --git a/shortcuts/base/base_shortcut_helpers.go b/shortcuts/base/base_shortcut_helpers.go index 8d42fe4da..fad33fe3a 100644 --- a/shortcuts/base/base_shortcut_helpers.go +++ b/shortcuts/base/base_shortcut_helpers.go @@ -30,34 +30,34 @@ func baseTableID(runtime *common.RuntimeContext) string { func loadJSONInput(pc *parseCtx, raw string, flagName string) (string, error) { raw = strings.TrimSpace(raw) if raw == "" { - return "", common.FlagErrorf("--%s cannot be empty", flagName) + return "", baseFlagErrorf("--%s cannot be empty", flagName) } if !strings.HasPrefix(raw, "@") { return raw, nil } path := strings.TrimSpace(strings.TrimPrefix(raw, "@")) if path == "" { - return "", common.FlagErrorf("--%s file path cannot be empty after @", flagName) + return "", baseFlagErrorf("--%s file path cannot be empty after @", flagName) } if pc.fio == nil { - return "", common.FlagErrorf("--%s @file inputs require a FileIO provider", flagName) + return "", baseMissingFileIOError("--%s @file inputs require a FileIO provider", flagName) } f, err := pc.fio.Open(path) if err != nil { var pathErr *fileio.PathValidationError if errors.As(err, &pathErr) { - return "", common.FlagErrorf("--%s invalid JSON file path %q: %v", flagName, path, pathErr.Err) + return "", baseFlagErrorf("--%s invalid JSON file path %q: %v", flagName, path, pathErr.Err) } - return "", common.FlagErrorf("--%s cannot open JSON file %q: %v", flagName, path, err) + return "", baseFlagErrorf("--%s cannot open JSON file %q: %v", flagName, path, err) } defer f.Close() data, err := io.ReadAll(f) if err != nil { - return "", common.FlagErrorf("--%s cannot read JSON file %q: %v", flagName, path, err) + return "", baseFlagErrorf("--%s cannot read JSON file %q: %v", flagName, path, err) } content := strings.TrimSpace(string(data)) if content == "" { - return "", common.FlagErrorf("--%s JSON file %q is empty", flagName, path) + return "", baseFlagErrorf("--%s JSON file %q is empty", flagName, path) } return content, nil } @@ -68,15 +68,15 @@ func jsonInputTip(flagName string) string { func formatJSONError(flagName string, target string, err error) error { if syntaxErr, ok := err.(*json.SyntaxError); ok { - return common.FlagErrorf("--%s invalid JSON %s near byte %d (%v); %s", flagName, target, syntaxErr.Offset, err, jsonInputTip(flagName)) + return baseFlagErrorf("--%s invalid JSON %s near byte %d (%v); %s", flagName, target, syntaxErr.Offset, err, jsonInputTip(flagName)) } if typeErr, ok := err.(*json.UnmarshalTypeError); ok { if typeErr.Field != "" { - return common.FlagErrorf("--%s invalid JSON %s at field %q (%v); %s", flagName, target, typeErr.Field, err, jsonInputTip(flagName)) + return baseFlagErrorf("--%s invalid JSON %s at field %q (%v); %s", flagName, target, typeErr.Field, err, jsonInputTip(flagName)) } - return common.FlagErrorf("--%s invalid JSON %s (%v); %s", flagName, target, err, jsonInputTip(flagName)) + return baseFlagErrorf("--%s invalid JSON %s (%v); %s", flagName, target, err, jsonInputTip(flagName)) } - return common.FlagErrorf("--%s invalid JSON %s (%v); %s", flagName, target, err, jsonInputTip(flagName)) + return baseFlagErrorf("--%s invalid JSON %s (%v); %s", flagName, target, err, jsonInputTip(flagName)) } func baseAction(runtime *common.RuntimeContext, boolFlags []string, stringFlags []string) (string, error) { @@ -92,14 +92,14 @@ func baseAction(runtime *common.RuntimeContext, boolFlags []string, stringFlags } } if len(active) == 0 { - return "", common.FlagErrorf("specify one action") + return "", baseFlagErrorf("specify one action") } if len(active) > 1 { flags := make([]string, 0, len(active)) for _, item := range active { flags = append(flags, "--"+item) } - return "", common.FlagErrorf("actions are mutually exclusive: %s", strings.Join(flags, ", ")) + return "", baseFlagErrorf("actions are mutually exclusive: %s", strings.Join(flags, ", ")) } return active[0], nil } @@ -123,7 +123,7 @@ func parseObjectList(pc *parseCtx, raw string, flagName string) ([]map[string]in for idx, item := range arr { obj, ok := item.(map[string]interface{}) if !ok { - return nil, common.FlagErrorf("--%s item %d must be an object", flagName, idx+1) + return nil, baseFlagErrorf("--%s item %d must be an object", flagName, idx+1) } items = append(items, obj) } @@ -150,6 +150,6 @@ func parseJSONValue(pc *parseCtx, raw string, flagName string) (interface{}, err case map[string]interface{}, []interface{}: return value, nil default: - return nil, common.FlagErrorf("--%s must be a JSON object or array", flagName) + return nil, baseFlagErrorf("--%s must be a JSON object or array", flagName) } } diff --git a/shortcuts/base/dashboard_block_create.go b/shortcuts/base/dashboard_block_create.go index ec8af8d3b..0cd8d4ed5 100644 --- a/shortcuts/base/dashboard_block_create.go +++ b/shortcuts/base/dashboard_block_create.go @@ -6,9 +6,9 @@ package base import ( "context" "encoding/json" - "fmt" "strings" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -47,7 +47,7 @@ var BaseDashboardBlockCreate = common.Shortcut{ if strings.TrimSpace(raw) == "" { // text 类型必须提供 data-config（含 text 内容） if strings.ToLower(runtime.Str("type")) == "text" { - return fmt.Errorf("text 类型组件必须提供 data-config，包含必填字段 text") + return errs.NewValidationError(errs.SubtypeInvalidArgument, "text 类型组件必须提供 data-config，包含必填字段 text").WithParam("--data-config") } return nil } diff --git a/shortcuts/base/field_ops.go b/shortcuts/base/field_ops.go index 75ea309d2..4774c95a7 100644 --- a/shortcuts/base/field_ops.go +++ b/shortcuts/base/field_ops.go @@ -91,7 +91,7 @@ func validateFormulaLookupGuideAck(runtime *common.RuntimeContext, command strin if fieldType == "lookup" { guidePath = "skills/lark-base/references/lookup-field-guide.md" } - return common.FlagErrorf("--i-have-read-guide is required for %s when --json.type is %q; read %s first, then retry with --i-have-read-guide", command, fieldType, guidePath) + return baseFlagErrorf("--i-have-read-guide is required for %s when --json.type is %q; read %s first, then retry with --i-have-read-guide", command, fieldType, guidePath) } return nil } diff --git a/shortcuts/base/helpers.go b/shortcuts/base/helpers.go index 7b362ae50..b2dbcc05d 100644 --- a/shortcuts/base/helpers.go +++ b/shortcuts/base/helpers.go @@ -17,6 +17,7 @@ import ( larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/shortcuts/common" ) @@ -41,10 +42,10 @@ func parseJSONObject(pc *parseCtx, raw string, flagName string) (map[string]inte if errors.As(err, &syntaxErr) { return nil, formatJSONError(flagName, "object", err) } - return nil, common.FlagErrorf("--%s must be a JSON object; %s", flagName, jsonInputTip(flagName)) + return nil, baseFlagErrorf("--%s must be a JSON object; %s", flagName, jsonInputTip(flagName)) } if result == nil { - return nil, common.FlagErrorf("--%s must be a JSON object; %s", flagName, jsonInputTip(flagName)) + return nil, baseFlagErrorf("--%s must be a JSON object; %s", flagName, jsonInputTip(flagName)) } return result, nil } @@ -152,7 +153,7 @@ func cloneValue(value interface{}) interface{} { func resolveFieldTypeSpec(typeName string) (fieldTypeSpec, error) { trimmed := strings.TrimSpace(typeName) if trimmed == "" { - return fieldTypeSpec{}, fmt.Errorf("field type cannot be empty") + return fieldTypeSpec{}, baseValidationErrorf("field type cannot be empty") } switch strings.ToLower(trimmed) { case "text", "phone", "url", "email", "barcode": @@ -192,7 +193,7 @@ func resolveFieldTypeSpec(typeName string) (fieldTypeSpec, error) { case "modifiedtime", "modified_time", "modified-time": return fieldTypeSpec{Type: "updated_at", Extra: map[string]interface{}{"style": map[string]interface{}{"format": "yyyy/MM/dd"}}}, nil default: - return fieldTypeSpec{}, fmt.Errorf("unsupported field type %q in base/v3", typeName) + return fieldTypeSpec{}, baseValidationErrorf("unsupported field type %q in base/v3", typeName) } } @@ -252,10 +253,10 @@ func normalizeSelectOptions(raw interface{}) []interface{} { func buildFieldBody(fieldName string, typeName string, property map[string]interface{}, uiType string, description string, isPrimary bool, isHidden bool) (map[string]interface{}, error) { if isPrimary { - return nil, fmt.Errorf("base/v3 does not support setting primary field in field body") + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "base/v3 does not support setting primary field in field body") } if isHidden { - return nil, fmt.Errorf("base/v3 does not support hidden field creation in field body") + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "base/v3 does not support hidden field creation in field body") } spec, err := resolveFieldTypeSpec(typeName) if err != nil { @@ -354,7 +355,7 @@ func buildTableFieldBodies(rawFields string, rawFieldSpecs string) ([]interface{ if rawFields != "" { var fields []interface{} if err := common.ParseJSON([]byte(rawFields), &fields); err != nil { - return nil, fmt.Errorf("--fields invalid JSON, must be a field definition array") + return nil, baseValidationErrorf("--fields invalid JSON, must be a field definition array") } return fields, nil } @@ -366,7 +367,7 @@ func buildTableFieldBodies(rawFields string, rawFieldSpecs string) ([]interface{ for _, spec := range specs { body, err := buildFieldBody(spec.Name, normalizeFieldTypeName(spec.Type), nil, "", "", false, false) if err != nil { - return nil, fmt.Errorf("field %q: %w", spec.Name, err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "field %q: %s", spec.Name, err).WithCause(err) } fields = append(fields, body) } @@ -410,20 +411,15 @@ func baseV3Raw(runtime *common.RuntimeContext, method, path string, params map[s h.Set("X-App-Id", runtime.Config.AppID) resp, err := runtime.DoAPI(req, larkcore.WithHeaders(h)) if err != nil { - return nil, err + return nil, baseAPIBoundaryError(err, "API call failed") } - result, parseErr := decodeBaseV3Response(resp.RawBody) - if parseErr == nil && baseV3ResultCode(result) != 0 { - attachBaseErrorLogID(result, baseResponseLogID(resp)) - return result, nil - } - if resp.StatusCode >= http.StatusBadRequest { - body := strings.TrimSpace(string(resp.RawBody)) - if body == "" { - return nil, fmt.Errorf("HTTP %d", resp.StatusCode) + if _, err := runtime.ClassifyAPIResponse(resp); err != nil { + if statusErr := baseHTTPStatusErrorFromInvalidResponse(resp, err); statusErr != nil { + return nil, statusErr } - return nil, fmt.Errorf("HTTP %d: %s", resp.StatusCode, body) + return nil, enrichBaseAPIErrorFromBody(err, resp.RawBody, runtime.APIClassifyContext()) } + result, parseErr := decodeBaseV3Response(resp.RawBody) if parseErr != nil { return nil, parseErr } @@ -435,16 +431,12 @@ func decodeBaseV3Response(body []byte) (map[string]interface{}, error) { dec := json.NewDecoder(bytes.NewReader(body)) dec.UseNumber() if err := dec.Decode(&result); err != nil { - return nil, fmt.Errorf("response parse error: %w", err) + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "API returned an invalid JSON response: %v", err).WithCause(err) } - return result, nil -} - -func baseV3ResultCode(result map[string]interface{}) int { if result == nil { - return 0 + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "API returned a non-object JSON response") } - return toInt(result["code"]) + return result, nil } func attachBaseErrorLogID(result map[string]interface{}, logID string) { @@ -480,6 +472,33 @@ func baseResponseLogID(resp *larkcore.ApiResp) string { return strings.TrimSpace(resp.Header.Get("x-tt-logid")) } +func baseHTTPStatusErrorFromInvalidResponse(resp *larkcore.ApiResp, classified error) error { + if resp == nil || resp.StatusCode < http.StatusBadRequest { + return nil + } + p, ok := errs.ProblemOf(classified) + if !ok || p.Category != errs.CategoryInternal || p.Subtype != errs.SubtypeInvalidResponse { + return nil + } + body := strings.TrimSpace(string(resp.RawBody)) + if resp.StatusCode >= http.StatusInternalServerError { + err := errs.NewNetworkError(errs.SubtypeNetworkServer, "HTTP %d: %s", resp.StatusCode, body).WithCode(resp.StatusCode).WithRetryable() + if logID := baseResponseLogID(resp); logID != "" { + err = err.WithLogID(logID) + } + return err + } + subtype := errs.SubtypeUnknown + if resp.StatusCode == http.StatusNotFound { + subtype = errs.SubtypeNotFound + } + err := errs.NewAPIError(subtype, "HTTP %d: %s", resp.StatusCode, body).WithCode(resp.StatusCode) + if logID := baseResponseLogID(resp); logID != "" { + err = err.WithLogID(logID) + } + return err +} + func baseV3Call(runtime *common.RuntimeContext, method, path string, params map[string]interface{}, data interface{}) (map[string]interface{}, error) { result, err := baseV3Raw(runtime, method, path, params, data) return handleBaseAPIResult(result, err, "API call failed") @@ -525,7 +544,7 @@ func toStringSlice(v interface{}) []string { func listAllTables(runtime *common.RuntimeContext, baseToken string, offset, limit int) ([]map[string]interface{}, int, error) { if limit <= 0 { - return nil, 0, fmt.Errorf("limit must be greater than 0") + return nil, 0, errs.NewInternalError(errs.SubtypeSDKError, "limit must be greater than 0") } data, err := baseV3Call(runtime, "GET", baseV3Path("bases", baseToken, "tables"), map[string]interface{}{"offset": offset, "limit": limit}, nil) if err != nil { @@ -555,7 +574,7 @@ func listAllTables(runtime *common.RuntimeContext, baseToken string, offset, lim func listAllFields(runtime *common.RuntimeContext, baseToken, tableID string, offset, limit int) ([]map[string]interface{}, int, error) { if limit <= 0 { - return nil, 0, fmt.Errorf("limit must be greater than 0") + return nil, 0, errs.NewInternalError(errs.SubtypeSDKError, "limit must be greater than 0") } data, err := baseV3Call(runtime, "GET", baseV3Path("bases", baseToken, "tables", tableID, "fields"), map[string]interface{}{"offset": offset, "limit": limit}, nil) if err != nil { @@ -577,7 +596,7 @@ func listAllFields(runtime *common.RuntimeContext, baseToken, tableID string, of func listAllViews(runtime *common.RuntimeContext, baseToken, tableID string, offset, limit int) ([]map[string]interface{}, int, error) { if limit <= 0 { - return nil, 0, fmt.Errorf("limit must be greater than 0") + return nil, 0, errs.NewInternalError(errs.SubtypeSDKError, "limit must be greater than 0") } data, err := baseV3Call(runtime, "GET", baseV3Path("bases", baseToken, "tables", tableID, "views"), map[string]interface{}{"offset": offset, "limit": limit}, nil) if err != nil { @@ -603,7 +622,7 @@ func resolveFieldRef(fields []map[string]interface{}, ref string) (map[string]in return field, nil } } - return nil, fmt.Errorf("field %q not found", ref) + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "field %q not found", ref) } func resolveTableRef(tables []map[string]interface{}, ref string) (map[string]interface{}, error) { @@ -612,7 +631,7 @@ func resolveTableRef(tables []map[string]interface{}, ref string) (map[string]in return table, nil } } - return nil, fmt.Errorf("table %q not found", ref) + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "table %q not found", ref) } func resolveViewRef(views []map[string]interface{}, ref string) (map[string]interface{}, error) { @@ -621,7 +640,7 @@ func resolveViewRef(views []map[string]interface{}, ref string) (map[string]inte return view, nil } } - return nil, fmt.Errorf("view %q not found", ref) + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, "view %q not found", ref) } func chunkRecords(records []map[string]interface{}, size int) [][]map[string]interface{} { @@ -738,18 +757,18 @@ func canonicalValue(v interface{}) string { func parseNamedTypeSpecs(raw string, flagName string) ([]namedTypeSpec, error) { var tuples []interface{} if err := common.ParseJSON([]byte(raw), &tuples); err != nil { - return nil, fmt.Errorf("--%s invalid JSON array", flagName) + return nil, baseValidationErrorf("--%s invalid JSON array", flagName) } result := make([]namedTypeSpec, 0, len(tuples)) for idx, item := range tuples { pair, ok := item.([]interface{}) if !ok || len(pair) != 2 { - return nil, fmt.Errorf("--%s item %d must be [name, type]", flagName, idx+1) + return nil, baseValidationErrorf("--%s item %d must be [name, type]", flagName, idx+1) } name, ok1 := pair[0].(string) typeName, ok2 := pair[1].(string) if !ok1 || !ok2 { - return nil, fmt.Errorf("--%s item %d must be [string, string]", flagName, idx+1) + return nil, baseValidationErrorf("--%s item %d must be [string, string]", flagName, idx+1) } result = append(result, namedTypeSpec{Name: name, Type: typeName}) } @@ -1155,9 +1174,9 @@ func validateBlockDataConfig(blockType string, cfg map[string]interface{}) []str return errs } -func formatDataConfigErrors(errs []string) error { - if len(errs) == 0 { +func formatDataConfigErrors(problems []string) error { + if len(problems) == 0 { return nil } - return fmt.Errorf("data_config 校验失败:\n- %s\n参考: skills/lark-base/references/dashboard-block-data-config.md", strings.Join(errs, "\n- ")) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "data_config 校验失败:\n- %s\n参考: skills/lark-base/references/dashboard-block-data-config.md", strings.Join(problems, "\n- ")) } diff --git a/shortcuts/base/record_markdown.go b/shortcuts/base/record_markdown.go index c379b716b..ea5c86b9d 100644 --- a/shortcuts/base/record_markdown.go +++ b/shortcuts/base/record_markdown.go @@ -8,6 +8,7 @@ import ( "fmt" "strings" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -19,7 +20,7 @@ func validateRecordReadFormat(runtime *common.RuntimeContext) error { case "", "json", "markdown": return nil default: - return output.ErrValidation("--format must be json or markdown") + return baseValidationErrorf("--format must be json or markdown") } } @@ -33,7 +34,7 @@ func outputRecordMarkdownWithRenderer(runtime *common.RuntimeContext, data map[s runtime.Out(data, nil) return nil } - return output.ErrValidation("--jq and --format markdown are mutually exclusive") + return baseValidationErrorf("--jq and --format markdown are mutually exclusive") } rendered, err := renderer(data) if err != nil { @@ -43,7 +44,7 @@ func outputRecordMarkdownWithRenderer(runtime *common.RuntimeContext, data map[s } scanResult := output.ScanForSafety(runtime.Cmd.CommandPath(), data, runtime.IO().ErrOut) if scanResult.Blocked { - return scanResult.BlockErr + return baseContentSafetyBlockError(scanResult) } if scanResult.Alert != nil { output.WriteAlertWarning(runtime.IO().ErrOut, scanResult.Alert) @@ -52,6 +53,20 @@ func outputRecordMarkdownWithRenderer(runtime *common.RuntimeContext, data map[s return nil } +func baseContentSafetyBlockError(scanResult output.ScanResult) error { + message := "content safety violation detected" + var rules []string + if scanResult.Alert != nil { + rules = scanResult.Alert.MatchedRules + } + if len(rules) > 0 { + message = fmt.Sprintf("content safety violation detected (rules: %s)", strings.Join(rules, ", ")) + } + return errs.NewContentSafetyError(errs.SubtypeUnknown, "%s", message). + WithRules(rules...). + WithCause(scanResult.BlockErr) +} + func outputRecordGetMarkdown(runtime *common.RuntimeContext, data map[string]interface{}) error { return outputRecordMarkdownWithRenderer(runtime, data, renderRecordGetMarkdown) } @@ -61,7 +76,7 @@ func renderRecordGetMarkdown(data map[string]interface{}) (string, error) { recordIDs := stringSliceValue(data["record_id_list"]) rows, ok := data["data"].([]interface{}) if len(fields) == 0 || !ok { - return "", output.ErrValidation("--format markdown requires record matrix response with fields, record_id_list, and data") + return "", baseValidationErrorf("--format markdown requires record matrix response with fields, record_id_list, and data") } if len(recordIDs) == 1 && len(rows) == 1 { rowItems, _ := rows[0].([]interface{}) @@ -78,7 +93,7 @@ func renderRecordMarkdown(data map[string]interface{}) (string, error) { recordIDs := stringSliceValue(data["record_id_list"]) rows, ok := data["data"].([]interface{}) if len(fields) == 0 || !ok { - return "", output.ErrValidation("--format markdown requires record matrix response with fields, record_id_list, and data") + return "", baseValidationErrorf("--format markdown requires record matrix response with fields, record_id_list, and data") } var b strings.Builder diff --git a/shortcuts/base/record_markdown_test.go b/shortcuts/base/record_markdown_test.go index d361a620d..09775b2f1 100644 --- a/shortcuts/base/record_markdown_test.go +++ b/shortcuts/base/record_markdown_test.go @@ -14,6 +14,7 @@ import ( "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" extcs "github.com/larksuite/cli/extension/contentsafety" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" @@ -212,9 +213,12 @@ func TestOutputRecordMarkdownContentSafetyBlockDoesNotWriteStdout(t *testing.T) "record_id_list": []interface{}{"rec_1"}, "data": []interface{}{[]interface{}{"Alice"}}, }) - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Code != output.ExitContentSafety { - t.Fatalf("err=%v, want content safety exit error", err) + var csErr *errs.ContentSafetyError + if !errors.As(err, &csErr) { + t.Fatalf("err=%v, want typed content safety error", err) + } + if len(csErr.Rules) != 1 || csErr.Rules[0] != "r1" { + t.Fatalf("rules=%v", csErr.Rules) } if stdout.Len() > 0 { t.Fatalf("block mode should not write stdout, got:\n%s", stdout.String()) diff --git a/shortcuts/base/record_ops.go b/shortcuts/base/record_ops.go index af48e6e72..1f5264097 100644 --- a/shortcuts/base/record_ops.go +++ b/shortcuts/base/record_ops.go @@ -49,7 +49,7 @@ func resolveRecordSelection(runtime *common.RuntimeContext) (recordSelection, er fieldIDs := runtime.StrArray("field-id") jsonRaw := strings.TrimSpace(runtime.Str("json")) if len(recordIDs) > 0 && jsonRaw != "" { - return recordSelection{}, common.FlagErrorf("--record-id and --json are mutually exclusive") + return recordSelection{}, baseFlagErrorf("--record-id and --json are mutually exclusive") } if jsonRaw != "" { pc := newParseCtx(runtime) @@ -59,11 +59,11 @@ func resolveRecordSelection(runtime *common.RuntimeContext) (recordSelection, er } recordIDListValue, ok := body["record_id_list"] if !ok { - return recordSelection{}, common.FlagErrorf(`--json must include "record_id_list" as a non-empty string array; %s`, jsonInputTip("json")) + return recordSelection{}, baseFlagErrorf(`--json must include "record_id_list" as a non-empty string array; %s`, jsonInputTip("json")) } recordIDItems, ok := recordIDListValue.([]interface{}) if !ok { - return recordSelection{}, common.FlagErrorf(`--json field "record_id_list" must be a string array; %s`, jsonInputTip("json")) + return recordSelection{}, baseFlagErrorf(`--json field "record_id_list" must be a string array; %s`, jsonInputTip("json")) } normalized, err := normalizeRecordIDs(recordIDItems) if err != nil { @@ -117,14 +117,14 @@ func resolveRecordGetSelectFields(flagFields []string, body map[string]interface return fromFlags, nil } if len(fromFlags) > 0 { - return nil, common.FlagErrorf(`--field-id and --json field "select_fields" are mutually exclusive`) + return nil, baseFlagErrorf(`--field-id and --json field "select_fields" are mutually exclusive`) } items, ok := rawJSONFields.([]interface{}) if !ok { - return nil, common.FlagErrorf(`--json field "select_fields" must be a string array; %s`, jsonInputTip("json")) + return nil, baseFlagErrorf(`--json field "select_fields" must be a string array; %s`, jsonInputTip("json")) } if len(items) == 0 { - return nil, common.FlagErrorf(`--json field "select_fields" must not be empty; %s`, jsonInputTip("json")) + return nil, baseFlagErrorf(`--json field "select_fields" must not be empty; %s`, jsonInputTip("json")) } normalized, err := normalizeRecordGetSelectFields(items) if err != nil { @@ -152,7 +152,7 @@ func normalizeStringList(values interface{}, opts stringListNormalizeOptions) ([ if opts.allowNil { return nil, nil } - return nil, common.FlagErrorf(opts.typeError) + return nil, baseFlagErrorf(opts.typeError) case []interface{}: rawItems = typed case []string: @@ -161,30 +161,30 @@ func normalizeStringList(values interface{}, opts stringListNormalizeOptions) ([ rawItems = append(rawItems, item) } default: - return nil, common.FlagErrorf(opts.typeError) + return nil, baseFlagErrorf(opts.typeError) } if len(rawItems) == 0 { if opts.allowEmpty { return nil, nil } - return nil, common.FlagErrorf(opts.emptyError) + return nil, baseFlagErrorf(opts.emptyError) } if opts.max > 0 && len(rawItems) > opts.max { - return nil, common.FlagErrorf("%s exceeds maximum limit of %d (got %d)", opts.limitName, opts.max, len(rawItems)) + return nil, baseFlagErrorf("%s exceeds maximum limit of %d (got %d)", opts.limitName, opts.max, len(rawItems)) } seen := make(map[string]int, len(rawItems)) result := make([]string, 0, len(rawItems)) for index, value := range rawItems { item, ok := value.(string) if !ok { - return nil, common.FlagErrorf("%s %d must be a string", opts.itemName, index+1) + return nil, baseFlagErrorf("%s %d must be a string", opts.itemName, index+1) } item = strings.TrimSpace(item) if item == "" { - return nil, common.FlagErrorf("%s %d must not be empty", opts.itemName, index+1) + return nil, baseFlagErrorf("%s %d must not be empty", opts.itemName, index+1) } if first, exists := seen[item]; exists { - return nil, common.FlagErrorf("duplicate %s %q at positions %d and %d", opts.duplicateName, item, first, index+1) + return nil, baseFlagErrorf("duplicate %s %q at positions %d and %d", opts.duplicateName, item, first, index+1) } seen[item] = index + 1 result = append(result, item) @@ -332,10 +332,10 @@ const maxShareBatchSize = 100 func validateRecordShareBatch(runtime *common.RuntimeContext) error { recordIDs := deduplicateRecordIDs(runtime) if len(recordIDs) == 0 { - return common.FlagErrorf("--record-ids is required and must not be empty") + return baseFlagErrorf("--record-ids is required and must not be empty") } if len(recordIDs) > maxShareBatchSize { - return common.FlagErrorf("--record-ids exceeds maximum limit of %d (got %d)", maxShareBatchSize, len(recordIDs)) + return baseFlagErrorf("--record-ids exceeds maximum limit of %d (got %d)", maxShareBatchSize, len(recordIDs)) } return nil } diff --git a/shortcuts/base/record_query.go b/shortcuts/base/record_query.go index 0bf37403b..3341d66b0 100644 --- a/shortcuts/base/record_query.go +++ b/shortcuts/base/record_query.go @@ -71,18 +71,18 @@ func normalizeRecordSortValue(value interface{}, label string) ([]interface{}, e } else if obj, ok := value.(map[string]interface{}); ok { rawSortConfig, ok := obj["sort_config"] if !ok { - return nil, common.FlagErrorf("%s must be a JSON array or an object with sort_config array", label) + return nil, baseFlagErrorf("%s must be a JSON array or an object with sort_config array", label) } parsed, ok := rawSortConfig.([]interface{}) if !ok { - return nil, common.FlagErrorf("%s.sort_config must be a JSON array", label) + return nil, baseFlagErrorf("%s.sort_config must be a JSON array", label) } sortConfig = parsed } else { - return nil, common.FlagErrorf("%s must be a JSON array or an object with sort_config array", label) + return nil, baseFlagErrorf("%s must be a JSON array or an object with sort_config array", label) } if len(sortConfig) > recordSortMaxCount { - return nil, common.FlagErrorf("sort supports at most %d sort conditions; got %d", recordSortMaxCount, len(sortConfig)) + return nil, baseFlagErrorf("sort supports at most %d sort conditions; got %d", recordSortMaxCount, len(sortConfig)) } return sortConfig, nil } @@ -90,7 +90,7 @@ func normalizeRecordSortValue(value interface{}, label string) ([]interface{}, e func marshalRecordQueryFlag(flagName string, value interface{}) (string, error) { data, err := json.Marshal(value) if err != nil { - return "", common.FlagErrorf("--%s cannot encode JSON: %v", flagName, err) + return "", baseFlagErrorf("--%s cannot encode JSON: %v", flagName, err) } return string(data), nil } @@ -220,16 +220,16 @@ func validateRecordSearchFlags(runtime *common.RuntimeContext) error { jsonRaw := strings.TrimSpace(runtime.Str("json")) if jsonRaw != "" { if recordSearchHasJSONExclusiveFlagInputs(runtime) { - return common.FlagErrorf("--json is mutually exclusive with keyword/search/projection/pagination flags; put those fields inside --json, or omit --json") + return baseFlagErrorf("--json is mutually exclusive with keyword/search/projection/pagination flags; put those fields inside --json, or omit --json") } _, err := recordSearchJSONBody(runtime) return err } if strings.TrimSpace(runtime.Str("keyword")) == "" { - return common.FlagErrorf("--keyword is required unless --json is used") + return baseFlagErrorf("--keyword is required unless --json is used") } if len(runtime.StrArray("search-field")) == 0 { - return common.FlagErrorf("--search-field is required unless --json is used") + return baseFlagErrorf("--search-field is required unless --json is used") } return validateRecordQueryOptions(runtime) } diff --git a/shortcuts/base/record_upload_attachment.go b/shortcuts/base/record_upload_attachment.go index 3bc564162..c469194df 100644 --- a/shortcuts/base/record_upload_attachment.go +++ b/shortcuts/base/record_upload_attachment.go @@ -22,7 +22,6 @@ import ( "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/util" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" @@ -225,7 +224,7 @@ func dryRunRecordRemoveAttachment(_ context.Context, runtime *common.RuntimeCont func validateRecordUploadAttachment(runtime *common.RuntimeContext) error { if runtime.Changed("name") { - return common.FlagErrorf("--name is no longer supported; uploaded attachment names are derived from local file basenames") + return baseFlagErrorf("--name is no longer supported; uploaded attachment names are derived from local file basenames") } files, err := normalizeAttachmentFiles(runtime.StrArray("file")) if err != nil { @@ -245,9 +244,16 @@ func validateRecordDownloadAttachment(runtime *common.RuntimeContext) error { return err } if len(tokens) != 1 { + const outputDirRequired = "--output must be an existing directory when downloading multiple attachments or when --file-token is omitted" info, statErr := runtime.FileIO().Stat(runtime.Str("output")) - if statErr != nil || !info.IsDir() { - return common.FlagErrorf("--output must be an existing directory when downloading multiple attachments or when --file-token is omitted") + if statErr != nil { + if errors.Is(statErr, fileio.ErrPathValidation) { + return baseValidationErrorf("unsafe output path: %s", statErr) + } + return baseFlagErrorf(outputDirRequired) + } + if !info.IsDir() { + return baseFlagErrorf(outputDirRequired) } } return nil @@ -269,7 +275,7 @@ func executeRecordUploadAttachment(runtime *common.RuntimeContext) error { return err } if normalized := normalizeFieldTypeName(fieldTypeName(field)); normalized != "attachment" { - return output.ErrValidation("field %q is type %q, expected attachment", fieldName(field), normalized) + return baseValidationErrorf("field %q is type %q, expected attachment", fieldName(field), normalized) } resolvedFieldID := fieldID(field) if resolvedFieldID == "" { @@ -316,7 +322,7 @@ func executeRecordRemoveAttachment(runtime *common.RuntimeContext) error { return err } if normalized := normalizeFieldTypeName(fieldTypeName(field)); normalized != "attachment" { - return output.ErrValidation("field %q is type %q, expected attachment", fieldName(field), normalized) + return baseValidationErrorf("field %q is type %q, expected attachment", fieldName(field), normalized) } resolvedFieldID := fieldID(field) if resolvedFieldID == "" { @@ -353,7 +359,7 @@ func executeRecordDownloadAttachment(ctx context.Context, runtime *common.Runtim saved, err := downloadBaseAttachment(ctx, runtime, target.Item, target.TargetPath, runtime.Bool("overwrite")) if err != nil { failed := attachmentDownloadFailure(target, err) - return attachmentDownloadProgressError(err, downloaded, []map[string]interface{}{failed}) + return attachmentDownloadProgressError(runtime, err, downloaded, []map[string]interface{}{failed}) } downloaded = append(downloaded, saved) } @@ -364,20 +370,20 @@ func executeRecordDownloadAttachment(ctx context.Context, runtime *common.Runtim func validateAttachmentInputFile(runtime *common.RuntimeContext, filePath string) (fileio.FileInfo, error) { fio := runtime.FileIO() if fio == nil { - return nil, output.ErrValidation("file operations require a FileIO provider") + return nil, baseValidationErrorf("file operations require a FileIO provider") } fileInfo, err := fio.Stat(filePath) if err != nil { if errors.Is(err, fileio.ErrPathValidation) { - return nil, output.ErrValidation("unsafe file path: %s", err) + return nil, baseValidationErrorf("unsafe file path: %s", err) } - return nil, output.ErrValidation("file not accessible: %s: %v", filePath, err) + return nil, baseValidationErrorf("file not accessible: %s: %v", filePath, err) } if fileInfo.IsDir() { - return nil, output.ErrValidation("file path is a directory: %s", filePath) + return nil, baseValidationErrorf("file path is a directory: %s", filePath) } if fileInfo.Size() > baseAttachmentUploadMaxFileSize { - return nil, output.ErrValidation("file %s exceeds 2GB limit", common.FormatSize(fileInfo.Size())) + return nil, baseValidationErrorf("file %s exceeds 2GB limit (size: %s)", filePath, common.FormatSize(fileInfo.Size())) } return fileInfo, nil } @@ -412,13 +418,13 @@ func normalizeOptionalDownloadAttachmentFileTokens(tokens []string) ([]string, e for index, token := range tokens { token = strings.TrimSpace(token) if token == "" { - return nil, common.FlagErrorf("attachment file token %d must not be empty", index+1) + return nil, baseFlagErrorf("attachment file token %d must not be empty", index+1) } normalized = append(normalized, token) } normalized = dedupeStringsPreserveOrder(normalized) if len(normalized) > baseAttachmentMaxBatchSize { - return nil, common.FlagErrorf("attachment file token count exceeds maximum limit of %d (got %d)", baseAttachmentMaxBatchSize, len(normalized)) + return nil, baseFlagErrorf("attachment file token count exceeds maximum limit of %d (got %d)", baseAttachmentMaxBatchSize, len(normalized)) } return normalized, nil } @@ -453,10 +459,10 @@ func fetchBaseField(runtime *common.RuntimeContext, baseToken, tableIDValue, fie func fetchBaseAttachments(runtime *common.RuntimeContext, baseToken, tableIDValue string, recordIDs []string) (map[string]interface{}, error) { if len(recordIDs) == 0 { - return nil, output.ErrValidation("provide at least one record id") + return nil, baseValidationErrorf("provide at least one record id") } if len(recordIDs) > baseAttachmentGetMaxRecords { - return nil, output.ErrValidation("get attachments record selection exceeds maximum limit of %d (got %d)", baseAttachmentGetMaxRecords, len(recordIDs)) + return nil, baseValidationErrorf("get attachments record selection exceeds maximum limit of %d (got %d)", baseAttachmentGetMaxRecords, len(recordIDs)) } data, err := baseV3Call(runtime, "POST", baseV3Path("bases", baseToken, "tables", tableIDValue, "get_attachments"), nil, map[string]interface{}{ "record_id_list": recordIDs, @@ -560,14 +566,14 @@ func detectAttachmentMIMEType(fio fileio.FileIO, filePath, fileName string) (str f, err := fio.Open(filePath) if err != nil { - return "", common.WrapInputStatError(err) + return "", baseInputStatError(err) } defer f.Close() buf := make([]byte, 512) n, readErr := f.Read(buf) if readErr != nil && !errors.Is(readErr, io.EOF) { - return "", output.ErrValidation("cannot read file: %s", readErr) + return "", baseValidationErrorf("cannot read file: %s", readErr) } return detectAttachmentMIMEFromContent(buf[:n]), nil } @@ -617,11 +623,11 @@ type baseAttachmentDownloadTarget struct { func selectAttachmentDownloadItems(attachments map[string]interface{}, recordID string, tokens []string) ([]baseAttachmentDownloadItem, error) { recordRaw, ok := attachments[recordID] if !ok { - return nil, output.ErrValidation("record %q has no attachment metadata; verify the record-id", recordID) + return nil, baseValidationErrorf("record %q has no attachment metadata; verify the record-id", recordID) } fields, ok := recordRaw.(map[string]interface{}) if !ok { - return nil, output.ErrValidation("record %q attachment metadata has unexpected type %T", recordID, recordRaw) + return nil, baseValidationErrorf("record %q attachment metadata has unexpected type %T", recordID, recordRaw) } byToken := map[string]baseAttachmentDownloadItem{} fieldIDs := make([]string, 0, len(fields)) @@ -633,12 +639,12 @@ func selectAttachmentDownloadItems(attachments map[string]interface{}, recordID rawList := fields[currentFieldID] items, ok := rawList.([]interface{}) if !ok { - return nil, output.ErrValidation("record %q field %q attachment metadata has unexpected type %T", recordID, currentFieldID, rawList) + return nil, baseValidationErrorf("record %q field %q attachment metadata has unexpected type %T", recordID, currentFieldID, rawList) } for _, rawItem := range items { item, ok := rawItem.(map[string]interface{}) if !ok { - return nil, output.ErrValidation("record %q field %q contains unexpected attachment item type %T", recordID, currentFieldID, rawItem) + return nil, baseValidationErrorf("record %q field %q contains unexpected attachment item type %T", recordID, currentFieldID, rawItem) } fileToken, _ := item["file_token"].(string) if fileToken == "" { @@ -668,7 +674,7 @@ func selectAttachmentDownloadItems(attachments map[string]interface{}, recordID result = append(result, item) } if len(result) == 0 { - return nil, output.ErrValidation("record %q has no attachments to download", recordID) + return nil, baseValidationErrorf("record %q has no attachments to download", recordID) } sort.SliceStable(result, func(i, j int) bool { leftName := strings.ToLower(baseAttachmentDownloadName(result[i])) @@ -683,7 +689,7 @@ func selectAttachmentDownloadItems(attachments map[string]interface{}, recordID for _, token := range tokens { item, ok := byToken[token] if !ok { - return nil, output.ErrValidation("attachment file_token %q not found in record %q; verify the record-id/file-token pair", token, recordID) + return nil, baseValidationErrorf("attachment file_token %q not found in record %q; verify the record-id/file-token pair", token, recordID) } result = append(result, item) } @@ -702,15 +708,15 @@ func planAttachmentDownloadTargets(runtime *common.RuntimeContext, items []baseA } resolved, err := runtime.ResolveSavePath(targetPath) if err != nil { - return nil, output.ErrValidation("unsafe output path: %s", err) + return nil, baseValidationErrorf("unsafe output path: %s", err) } if previous, exists := seen[resolved]; exists { - return nil, output.ErrValidation("multiple attachments resolve to the same output path %q (%s and %s); download them separately or choose a different directory", resolved, previous.FileToken, item.FileToken) + return nil, baseValidationErrorf("multiple attachments resolve to the same output path %q (%s and %s); download them separately or choose a different directory", resolved, previous.FileToken, item.FileToken) } seen[resolved] = item if !overwrite { if _, statErr := runtime.FileIO().Stat(targetPath); statErr == nil { - return nil, output.ErrValidation("output file already exists: %s (use --overwrite to replace)", targetPath) + return nil, baseValidationErrorf("output file already exists: %s (use --overwrite to replace)", targetPath) } } targets = append(targets, baseAttachmentDownloadTarget{ @@ -776,7 +782,7 @@ func safeAttachmentFileTokenSuffix(fileToken string) string { func downloadBaseAttachment(ctx context.Context, runtime *common.RuntimeContext, item baseAttachmentDownloadItem, targetPath string, overwrite bool) (map[string]interface{}, error) { if _, err := runtime.ResolveSavePath(targetPath); err != nil { - return nil, output.ErrValidation("unsafe output path: %s", err) + return nil, baseValidationErrorf("unsafe output path: %s", err) } query := larkcore.QueryParams{} @@ -795,7 +801,7 @@ func downloadBaseAttachment(ctx context.Context, runtime *common.RuntimeContext, if !overwrite { if _, statErr := runtime.FileIO().Stat(targetPath); statErr == nil { - return nil, output.ErrValidation("output file already exists: %s (use --overwrite to replace)", targetPath) + return nil, baseValidationErrorf("output file already exists: %s (use --overwrite to replace)", targetPath) } } result, err := runtime.FileIO().Save(targetPath, fileio.SaveOptions{ @@ -803,7 +809,7 @@ func downloadBaseAttachment(ctx context.Context, runtime *common.RuntimeContext, ContentLength: resp.ContentLength, }, resp.Body) if err != nil { - return nil, common.WrapSaveErrorByCategory(err, "io") + return nil, baseSaveError(err) } savedPath, _ := runtime.ResolveSavePath(targetPath) if savedPath == "" { @@ -822,7 +828,7 @@ func downloadBaseAttachment(ctx context.Context, runtime *common.RuntimeContext, } func attachmentDownloadFailure(target baseAttachmentDownloadTarget, err error) map[string]interface{} { - return map[string]interface{}{ + failure := map[string]interface{}{ "record_id": target.Item.RecordID, "field_id": target.Item.FieldID, "file_token": target.Item.FileToken, @@ -831,72 +837,45 @@ func attachmentDownloadFailure(target baseAttachmentDownloadTarget, err error) m "resolved_path": target.ResolvedPath, "error": err.Error(), } + if p, ok := errs.ProblemOf(err); ok { + failure["type"] = string(p.Category) + failure["subtype"] = string(p.Subtype) + if p.Code != 0 { + failure["code"] = p.Code + } + if p.LogID != "" { + failure["log_id"] = p.LogID + } + } + return failure } -func attachmentDownloadProgressError(err error, downloaded []map[string]interface{}, failed []map[string]interface{}) error { +func attachmentDownloadProgressError(runtime *common.RuntimeContext, err error, downloaded []map[string]interface{}, failed []map[string]interface{}) error { msg := fmt.Sprintf("download failed after %d attachment(s) succeeded and %d failed: %v", len(downloaded), len(failed), err) - detail := map[string]interface{}{ + payload := map[string]interface{}{ + "message": msg, "downloaded": downloaded, "failed": failed, } - if logID := baseAttachmentDownloadLogID(err); logID != "" { - detail["log_id"] = logID - } - const hint = "Some files may already have been saved. Inspect error.detail.downloaded before retrying, or rerun with --overwrite if the failed target now exists." - - var exitErr *output.ExitError - if errors.As(err, &exitErr) && exitErr.Detail != nil { - return &output.ExitError{ - Code: exitErr.Code, - Detail: &output.ErrDetail{ - Type: exitErr.Detail.Type, - Code: exitErr.Detail.Code, - Message: msg, - Hint: hint, - Detail: detail, - }, - Err: err, - } - } - var netErr *errs.NetworkError - if errors.As(err, &netErr) { - return &output.ExitError{ - Code: output.ExitNetwork, - Detail: &output.ErrDetail{ - Type: "network", - Code: netErr.Code, - Message: msg, - Hint: hint, - Detail: detail, - }, - Err: err, + const hint = "Some files may already have been saved. Inspect downloaded before retrying, or rerun with --overwrite if the failed target now exists." + payload["hint"] = hint + if p, ok := errs.ProblemOf(err); ok { + payload["type"] = string(p.Category) + payload["subtype"] = string(p.Subtype) + if p.Code != 0 { + payload["code"] = p.Code } } - return &output.ExitError{ - Code: output.ExitInternal, - Detail: &output.ErrDetail{ - Type: "io", - Message: msg, - Hint: hint, - Detail: detail, - }, - Err: err, + if logID := baseAttachmentDownloadLogID(err); logID != "" { + payload["log_id"] = logID } + return runtime.OutPartialFailure(payload, nil) } func baseAttachmentDownloadLogID(err error) string { - var netErr *errs.NetworkError - if errors.As(err, &netErr) { - if id := strings.TrimSpace(netErr.LogID); id != "" { - return id - } - } - var exitErr *output.ExitError - if errors.As(err, &exitErr) && exitErr.Detail != nil { - if detail, ok := exitErr.Detail.Detail.(map[string]interface{}); ok { - if logID, _ := detail["log_id"].(string); logID != "" { - return strings.TrimSpace(logID) - } + if p, ok := errs.ProblemOf(err); ok { + if logID := strings.TrimSpace(p.LogID); logID != "" { + return logID } } return "" diff --git a/shortcuts/base/table_ops.go b/shortcuts/base/table_ops.go index 9c546b5db..909709122 100644 --- a/shortcuts/base/table_ops.go +++ b/shortcuts/base/table_ops.go @@ -5,7 +5,6 @@ package base import ( "context" - "fmt" "github.com/larksuite/cli/shortcuts/common" ) @@ -117,7 +116,7 @@ func executeTableCreate(runtime *common.RuntimeContext) error { for idx, item := range fieldItems { body, ok := item.(map[string]interface{}) if !ok { - return fmt.Errorf("--fields item %d must be an object", idx+1) + return baseValidationErrorf("--fields item %d must be an object", idx+1) } if idx == 0 && len(defaultFields) > 0 { fieldData, err := baseV3Call(runtime, "PUT", baseV3Path("bases", baseToken, "tables", tableIDValue, "fields", fieldID(defaultFields[0])), nil, body) diff --git a/shortcuts/base/workflow_create.go b/shortcuts/base/workflow_create.go index 30c7beb09..9bf9fe562 100644 --- a/shortcuts/base/workflow_create.go +++ b/shortcuts/base/workflow_create.go @@ -31,7 +31,7 @@ var BaseWorkflowCreate = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("base-token")) == "" { - return common.FlagErrorf("--base-token must not be blank") + return baseFlagErrorf("--base-token must not be blank") } pc := newParseCtx(runtime) raw, err := loadJSONInput(pc, runtime.Str("json"), "json") diff --git a/shortcuts/base/workflow_disable.go b/shortcuts/base/workflow_disable.go index 582c6f88b..945114ffe 100644 --- a/shortcuts/base/workflow_disable.go +++ b/shortcuts/base/workflow_disable.go @@ -27,10 +27,10 @@ var BaseWorkflowDisable = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("base-token")) == "" { - return common.FlagErrorf("--base-token must not be blank") + return baseFlagErrorf("--base-token must not be blank") } if strings.TrimSpace(runtime.Str("workflow-id")) == "" { - return common.FlagErrorf("--workflow-id must not be blank") + return baseFlagErrorf("--workflow-id must not be blank") } return nil }, diff --git a/shortcuts/base/workflow_enable.go b/shortcuts/base/workflow_enable.go index 98cba38c0..3cca469c3 100644 --- a/shortcuts/base/workflow_enable.go +++ b/shortcuts/base/workflow_enable.go @@ -28,10 +28,10 @@ var BaseWorkflowEnable = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("base-token")) == "" { - return common.FlagErrorf("--base-token must not be blank") + return baseFlagErrorf("--base-token must not be blank") } if strings.TrimSpace(runtime.Str("workflow-id")) == "" { - return common.FlagErrorf("--workflow-id must not be blank") + return baseFlagErrorf("--workflow-id must not be blank") } return nil }, diff --git a/shortcuts/base/workflow_get.go b/shortcuts/base/workflow_get.go index f0d591f4a..5e5abbb17 100644 --- a/shortcuts/base/workflow_get.go +++ b/shortcuts/base/workflow_get.go @@ -30,10 +30,10 @@ var BaseWorkflowGet = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("base-token")) == "" { - return common.FlagErrorf("--base-token must not be blank") + return baseFlagErrorf("--base-token must not be blank") } if strings.TrimSpace(runtime.Str("workflow-id")) == "" { - return common.FlagErrorf("--workflow-id must not be blank") + return baseFlagErrorf("--workflow-id must not be blank") } return nil }, diff --git a/shortcuts/base/workflow_list.go b/shortcuts/base/workflow_list.go index 2ee6a17b7..4277bc6bd 100644 --- a/shortcuts/base/workflow_list.go +++ b/shortcuts/base/workflow_list.go @@ -28,7 +28,7 @@ var BaseWorkflowList = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("base-token")) == "" { - return common.FlagErrorf("--base-token must not be blank") + return baseFlagErrorf("--base-token must not be blank") } return nil }, diff --git a/shortcuts/base/workflow_update.go b/shortcuts/base/workflow_update.go index e9ffa4de8..ea13a3e17 100644 --- a/shortcuts/base/workflow_update.go +++ b/shortcuts/base/workflow_update.go @@ -33,10 +33,10 @@ var BaseWorkflowUpdate = common.Shortcut{ }, Validate: func(ctx context.Context, runtime *common.RuntimeContext) error { if strings.TrimSpace(runtime.Str("base-token")) == "" { - return common.FlagErrorf("--base-token must not be blank") + return baseFlagErrorf("--base-token must not be blank") } if strings.TrimSpace(runtime.Str("workflow-id")) == "" { - return common.FlagErrorf("--workflow-id must not be blank") + return baseFlagErrorf("--workflow-id must not be blank") } pc := newParseCtx(runtime) if _, err := parseJSONObject(pc, runtime.Str("json"), "json"); err != nil {