diff --git a/.golangci.yml b/.golangci.yml index 175ee254a..04eaf2542 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/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/|shortcuts/im/) + - 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/apps/|shortcuts/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/|shortcuts/im/) 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/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/|shortcuts/im/|shortcuts/common/mcp_client\.go) + - path-except: (shortcuts/apps/|shortcuts/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/|shortcuts/im/|shortcuts/common/mcp_client\.go) text: errs-no-bare-wrap linters: - forbidigo # errs-no-legacy-helper enforced on domains whose shared validation/save # helpers have migrated to typed final errors. - - path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/|shortcuts/im/) + - path-except: (shortcuts/apps/|shortcuts/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/|shortcuts/im/) 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 a6f05127e..d08d18cb1 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/apps/", "shortcuts/base/", "shortcuts/calendar/", "shortcuts/drive/", diff --git a/lint/errscontract/rule_no_legacy_envelope_literal.go b/lint/errscontract/rule_no_legacy_envelope_literal.go index 37cada331..410c52edd 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/apps/", "shortcuts/base/", "shortcuts/calendar/", "shortcuts/drive/", diff --git a/shortcuts/apps/apps_access_scope_get.go b/shortcuts/apps/apps_access_scope_get.go index df1a8cfe2..c3d755466 100644 --- a/shortcuts/apps/apps_access_scope_get.go +++ b/shortcuts/apps/apps_access_scope_get.go @@ -9,7 +9,6 @@ import ( "io" "strings" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -29,7 +28,7 @@ var AppsAccessScopeGet = common.Shortcut{ }, Validate: func(ctx context.Context, rctx *common.RuntimeContext) error { if strings.TrimSpace(rctx.Str("app-id")) == "" { - return output.ErrValidation("--app-id is required") + return appsValidationParamError("--app-id", "--app-id is required") } return nil }, @@ -42,7 +41,7 @@ var AppsAccessScopeGet = common.Shortcut{ Execute: func(ctx context.Context, rctx *common.RuntimeContext) error { appID := strings.TrimSpace(rctx.Str("app-id")) path := fmt.Sprintf("%s/apps/%s/access-scope", apiBasePath, validate.EncodePathSegment(appID)) - data, err := rctx.CallAPI("GET", path, nil, nil) + data, err := rctx.CallAPITyped("GET", path, nil, nil) if err != nil { return err } diff --git a/shortcuts/apps/apps_access_scope_set.go b/shortcuts/apps/apps_access_scope_set.go index f9f474fae..e8d3e43c8 100644 --- a/shortcuts/apps/apps_access_scope_set.go +++ b/shortcuts/apps/apps_access_scope_set.go @@ -10,7 +10,6 @@ import ( "io" "strings" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -40,7 +39,7 @@ var AppsAccessScopeSet = common.Shortcut{ }, Validate: func(ctx context.Context, rctx *common.RuntimeContext) error { if strings.TrimSpace(rctx.Str("app-id")) == "" { - return output.ErrValidation("--app-id is required") + return appsValidationParamError("--app-id", "--app-id is required") } return validateAccessScopeFlags(rctx) }, @@ -64,7 +63,7 @@ var AppsAccessScopeSet = common.Shortcut{ } appID := strings.TrimSpace(rctx.Str("app-id")) path := fmt.Sprintf("%s/apps/%s/access-scope", apiBasePath, validate.EncodePathSegment(appID)) - data, err := rctx.CallAPI("PUT", path, nil, body) + data, err := rctx.CallAPITyped("PUT", path, nil, body) if err != nil { return err } @@ -85,36 +84,42 @@ func validateAccessScopeFlags(rctx *common.RuntimeContext) error { switch scope { case "specific": if targets == "" { - return output.ErrValidation("--targets is required when --scope=specific") + return appsValidationParamError("--targets", "--targets is required when --scope=specific") } if err := validateTargetsJSON(targets); err != nil { return err } if approver != "" && !applyEnabled { - return output.ErrValidation("--approver requires --apply-enabled") + return appsValidationParamError("--approver", "--approver requires --apply-enabled") } if requireLogin { - return output.ErrValidation("--require-login is not allowed when --scope=specific") + return appsValidationParamError("--require-login", "--require-login is not allowed when --scope=specific") } case "public": if targets != "" { - return output.ErrValidation("--targets is not allowed when --scope=public") + return appsValidationParamError("--targets", "--targets is not allowed when --scope=public") } if applyEnabled { - return output.ErrValidation("--apply-enabled is not allowed when --scope=public") + return appsValidationParamError("--apply-enabled", "--apply-enabled is not allowed when --scope=public") } if approver != "" { - return output.ErrValidation("--approver is not allowed when --scope=public") + return appsValidationParamError("--approver", "--approver is not allowed when --scope=public") } if !rctx.Cmd.Flags().Changed("require-login") { - return output.ErrValidation("--require-login is required when --scope=public (pass true or false explicitly; do not rely on the default)") + return appsValidationParamError("--require-login", "--require-login is required when --scope=public (pass true or false explicitly; do not rely on the default)") } case "tenant": if targets != "" || applyEnabled || approver != "" || requireLogin { - return output.ErrValidation("no extra flags allowed when --scope=tenant") + return appsValidationError("no extra flags allowed when --scope=tenant"). + WithParams( + appsInvalidParam("--targets", "not allowed when --scope=tenant"), + appsInvalidParam("--apply-enabled", "not allowed when --scope=tenant"), + appsInvalidParam("--approver", "not allowed when --scope=tenant"), + appsInvalidParam("--require-login", "not allowed when --scope=tenant"), + ) } default: - return output.ErrValidation("--scope must be specific / public / tenant") + return appsValidationParamError("--scope", "--scope must be specific / public / tenant") } return nil } @@ -122,18 +127,18 @@ func validateAccessScopeFlags(rctx *common.RuntimeContext) error { func validateTargetsJSON(targetsJSON string) error { var items []map[string]interface{} if err := json.Unmarshal([]byte(targetsJSON), &items); err != nil { - return output.ErrValidation("--targets is not valid JSON: %v", err) + return appsValidationParamError("--targets", "--targets is not valid JSON: %v", err).WithCause(err) } if len(items) == 0 { - return output.ErrValidation("--targets must contain at least one entry; specific scope requires concrete user/department/chat ids") + return appsValidationParamError("--targets", "--targets must contain at least one entry; specific scope requires concrete user/department/chat ids") } for i, t := range items { typ, _ := t["type"].(string) if !allowedAccessTargetTypes[typ] { - return output.ErrValidation("--targets[%d].type %q must be one of: user / department / chat", i, typ) + return appsValidationParamError("--targets", "--targets[%d].type %q must be one of: user / department / chat", i, typ) } if id, _ := t["id"].(string); strings.TrimSpace(id) == "" { - return output.ErrValidation("--targets[%d].id is empty", i) + return appsValidationParamError("--targets", "--targets[%d].id is empty", i) } } return nil @@ -152,7 +157,7 @@ func buildAccessScopeBody(rctx *common.RuntimeContext) (map[string]interface{}, scope := rctx.Str("scope") enum, ok := scopeStringToServerEnum[scope] if !ok { - return nil, output.ErrValidation("--scope must be specific / public / tenant, got %q", scope) + return nil, appsValidationParamError("--scope", "--scope must be specific / public / tenant, got %q", scope) } body := map[string]interface{}{"scope": enum} @@ -161,7 +166,7 @@ func buildAccessScopeBody(rctx *common.RuntimeContext) (map[string]interface{}, // 用户传统一格式 [{type:user|department|chat, id:...}]，body 里拆 3 个并列数组发后端。 var targets []map[string]interface{} if err := json.Unmarshal([]byte(rctx.Str("targets")), &targets); err != nil { - return nil, output.ErrValidation("--targets is not valid JSON: %v", err) + return nil, appsValidationParamError("--targets", "--targets is not valid JSON: %v", err).WithCause(err) } users, departments, chats := splitAccessScopeTargets(targets) if len(users) > 0 { diff --git a/shortcuts/apps/apps_create.go b/shortcuts/apps/apps_create.go index 215b4ccd2..47c26c35f 100644 --- a/shortcuts/apps/apps_create.go +++ b/shortcuts/apps/apps_create.go @@ -9,7 +9,6 @@ import ( "io" "strings" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -30,14 +29,14 @@ var AppsCreate = common.Shortcut{ }, Validate: func(ctx context.Context, rctx *common.RuntimeContext) error { if strings.TrimSpace(rctx.Str("name")) == "" { - return output.ErrValidation("--name is required") + return appsValidationParamError("--name", "--name is required") } appType := strings.TrimSpace(rctx.Str("app-type")) if appType == "" { - return output.ErrValidation("--app-type is required") + return appsValidationParamError("--app-type", "--app-type is required") } if !validAppTypes[appType] { - return output.ErrValidation(fmt.Sprintf("--app-type %q is not supported (allowed: HTML)", appType)) + return appsValidationParamError("--app-type", "--app-type %q is not supported (allowed: HTML)", appType) } return nil }, @@ -48,7 +47,7 @@ var AppsCreate = common.Shortcut{ Body(buildAppsCreateBody(rctx)) }, Execute: func(ctx context.Context, rctx *common.RuntimeContext) error { - data, err := rctx.CallAPI("POST", apiBasePath+"/apps", nil, buildAppsCreateBody(rctx)) + data, err := rctx.CallAPITyped("POST", apiBasePath+"/apps", nil, buildAppsCreateBody(rctx)) if err != nil { return err } diff --git a/shortcuts/apps/apps_create_test.go b/shortcuts/apps/apps_create_test.go index 2cee3e581..6b9a69e06 100644 --- a/shortcuts/apps/apps_create_test.go +++ b/shortcuts/apps/apps_create_test.go @@ -12,6 +12,7 @@ import ( "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/httpmock" @@ -46,6 +47,31 @@ func runAppsShortcut(t *testing.T, sc common.Shortcut, args []string, factory *c return parent.ExecuteContext(context.Background()) } +func requireAppsProblem(t *testing.T, err error, category errs.Category) *errs.Problem { + t.Helper() + if err == nil { + t.Fatalf("expected error") + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed problem, got %T: %v", err, err) + } + if p.Category != category { + t.Fatalf("error category = %q, want %q", p.Category, category) + } + return p +} + +func requireAppsValidationProblem(t *testing.T, err error) *errs.Problem { + t.Helper() + return requireAppsProblem(t, err, errs.CategoryValidation) +} + +func requireAppsAPIProblem(t *testing.T, err error) *errs.Problem { + t.Helper() + return requireAppsProblem(t, err, errs.CategoryAPI) +} + // +create 测试 func TestAppsCreate_Success(t *testing.T) { diff --git a/shortcuts/apps/apps_errors.go b/shortcuts/apps/apps_errors.go new file mode 100644 index 000000000..73b3d2d8a --- /dev/null +++ b/shortcuts/apps/apps_errors.go @@ -0,0 +1,81 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package apps + +import ( + "errors" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/extension/fileio" + "github.com/larksuite/cli/internal/client" +) + +func appsValidationError(format string, args ...any) *errs.ValidationError { + return errs.NewValidationError(errs.SubtypeInvalidArgument, format, args...) +} + +func appsValidationParamError(param, format string, args ...any) *errs.ValidationError { + return appsValidationError(format, args...).WithParam(param) +} + +func appsInvalidParam(name, reason string) errs.InvalidParam { + return errs.InvalidParam{Name: name, Reason: reason} +} + +func appsFailedPreconditionParamError(param, format string, args ...any) *errs.ValidationError { + return errs.NewValidationError(errs.SubtypeFailedPrecondition, format, args...).WithParam(param) +} + +func appsInputPathError(err error) error { + if err == nil { + return nil + } + if errors.Is(err, fileio.ErrPathValidation) { + return appsValidationParamError("--path", "unsafe --path: %s", err).WithCause(err) + } + return appsValidationParamError("--path", "cannot read --path: %s", err).WithCause(err) +} + +func appsInputPathEntryError(path string, err error) error { + if err == nil { + return nil + } + if errors.Is(err, fileio.ErrPathValidation) { + return appsValidationParamError("--path", "unsafe --path entry %s: %s", path, err).WithCause(err) + } + return appsValidationParamError("--path", "cannot read --path entry %s: %s", path, err).WithCause(err) +} + +func appsFileIOError(err error, format string, args ...any) *errs.InternalError { + return errs.NewInternalError(errs.SubtypeFileIO, format, args...).WithCause(err) +} + +// enrichHTMLPublishAPIError adapts a typed failure from the HTML publish +// endpoint: refines endpoint-scoped business codes, prefixes the message with +// command context, and attaches endpoint-specific recovery hints. A +// still-untyped error is lifted at the SDK boundary instead. +func enrichHTMLPublishAPIError(err error) error { + if err == nil { + return nil + } + p, ok := errs.ProblemOf(err) + if !ok { + return client.WrapDoAPIError(err) + } + // The HTML publish business codes (90001/90002) are scoped to this + // endpoint, not service-global, so their subtype classification lives + // here instead of the global errclass code table. Only an + // otherwise-unclassified API error is refined; a stronger upstream + // classification is never overridden. + if p.Category == errs.CategoryAPI && p.Subtype == errs.SubtypeUnknown && p.Code == errCodeAppNotFound { + p.Subtype = errs.SubtypeNotFound + } + if p.Message != "" { + p.Message = "html-publish failed: " + p.Message + } + if hint := buildHTMLPublishFailureHint(p.Code); hint != "" { + p.Hint = hint + } + return err +} diff --git a/shortcuts/apps/apps_errors_test.go b/shortcuts/apps/apps_errors_test.go new file mode 100644 index 000000000..ccab1d7cb --- /dev/null +++ b/shortcuts/apps/apps_errors_test.go @@ -0,0 +1,113 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package apps + +import ( + "errors" + "strings" + "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/extension/fileio" +) + +func TestAppsInputPathError_ClassifiesPathValidation(t *testing.T) { + cause := errors.New("path escapes working directory") + err := appsInputPathError(&fileio.PathValidationError{Err: cause}) + + problem := requireAppsValidationProblem(t, err) + if problem.Subtype != errs.SubtypeInvalidArgument { + t.Fatalf("subtype = %q, want %q", problem.Subtype, errs.SubtypeInvalidArgument) + } + if !strings.Contains(problem.Message, "unsafe --path") { + t.Fatalf("message = %q, want unsafe --path context", problem.Message) + } + var validationErr *errs.ValidationError + if !errors.As(err, &validationErr) || validationErr.Param != "--path" { + t.Fatalf("param = %q, want --path", validationErr.Param) + } + if !errors.Is(err, fileio.ErrPathValidation) || !errors.Is(err, cause) { + t.Fatalf("path validation cause chain not preserved: %v", err) + } +} + +func TestAppsInputPathEntryError_ClassifiesReadFailure(t *testing.T) { + cause := errors.New("permission denied") + err := appsInputPathEntryError("dist/index.html", cause) + + problem := requireAppsValidationProblem(t, err) + if !strings.Contains(problem.Message, "cannot read --path entry dist/index.html") { + t.Fatalf("message = %q, want entry read context", problem.Message) + } + if !errors.Is(err, cause) { + t.Fatalf("cause chain not preserved: %v", err) + } +} + +func TestAppsFileIOError_ClassifiesInternalFileIO(t *testing.T) { + cause := errors.New("archive writer failed") + err := appsFileIOError(cause, "pack failed: %v", cause) + + problem := requireAppsProblem(t, err, errs.CategoryInternal) + if problem.Subtype != errs.SubtypeFileIO { + t.Fatalf("subtype = %q, want %q", problem.Subtype, errs.SubtypeFileIO) + } + if !errors.Is(err, cause) { + t.Fatalf("cause chain not preserved: %v", err) + } +} + +func TestEnrichHTMLPublishAPIError_LiftsUntypedBoundaryError(t *testing.T) { + err := enrichHTMLPublishAPIError(errors.New("connection reset by peer")) + + problem := requireAppsProblem(t, err, errs.CategoryNetwork) + if problem.Subtype != errs.SubtypeNetworkTransport { + t.Fatalf("subtype = %q, want %q", problem.Subtype, errs.SubtypeNetworkTransport) + } +} + +func TestEnrichHTMLPublishAPIError_PreservesClassificationAndAddsHint(t *testing.T) { + err := errs.NewAPIError(errs.SubtypeUnknown, "build failed"). + WithCode(errCodeBuildFailed). + WithLogID("logid-build-failed") + + got := enrichHTMLPublishAPIError(err) + if got != err { + t.Fatalf("typed error should be enriched in place") + } + problem := requireAppsAPIProblem(t, got) + if problem.Subtype != errs.SubtypeUnknown { + t.Fatalf("subtype = %q, want %q unchanged", problem.Subtype, errs.SubtypeUnknown) + } + if problem.Code != errCodeBuildFailed { + t.Fatalf("code = %d, want %d", problem.Code, errCodeBuildFailed) + } + if problem.LogID != "logid-build-failed" { + t.Fatalf("log_id = %q, want preserved", problem.LogID) + } + if !strings.Contains(problem.Message, "html-publish failed") { + t.Fatalf("message = %q, want html-publish context", problem.Message) + } + if problem.Hint == "" { + t.Fatalf("expected known-code recovery hint") + } +} + +func TestEnrichHTMLPublishAPIError_ClassifiesAppNotFoundLocally(t *testing.T) { + err := errs.NewAPIError(errs.SubtypeUnknown, "app not found").WithCode(errCodeAppNotFound) + + problem := requireAppsAPIProblem(t, enrichHTMLPublishAPIError(err)) + if problem.Subtype != errs.SubtypeNotFound { + t.Fatalf("subtype = %q, want %q", problem.Subtype, errs.SubtypeNotFound) + } +} + +func TestEnrichHTMLPublishAPIError_KeepsStrongerClassification(t *testing.T) { + err := errs.NewAPIError(errs.SubtypeRateLimit, "throttled").WithCode(errCodeAppNotFound) + + problem := requireAppsAPIProblem(t, enrichHTMLPublishAPIError(err)) + if problem.Subtype != errs.SubtypeRateLimit { + t.Fatalf("subtype = %q, want %q unchanged", problem.Subtype, errs.SubtypeRateLimit) + } +} diff --git a/shortcuts/apps/apps_html_publish.go b/shortcuts/apps/apps_html_publish.go index 64cc5ae79..95b3ff8e4 100644 --- a/shortcuts/apps/apps_html_publish.go +++ b/shortcuts/apps/apps_html_publish.go @@ -10,7 +10,7 @@ import ( "strings" "github.com/larksuite/cli/extension/fileio" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/internal/client" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -31,11 +31,11 @@ var AppsHTMLPublish = common.Shortcut{ }, Validate: func(ctx context.Context, rctx *common.RuntimeContext) error { if strings.TrimSpace(rctx.Str("app-id")) == "" { - return output.ErrValidation("--app-id is required") + return appsValidationParamError("--app-id", "--app-id is required") } path := strings.TrimSpace(rctx.Str("path")) if path == "" { - return output.ErrValidation("--path is required") + return appsValidationParamError("--path", "--path is required") } // Block well-known credential files in the publish payload unless the // caller explicitly opts in. Lives in Validate (not DryRun) so that @@ -146,9 +146,9 @@ func sensitiveCandidatesError(hits []string) error { sample = strings.Join(hits[:maxSensitiveListInError], ", ") + fmt.Sprintf(" (and %d more)", len(hits)-maxSensitiveListInError) } - return output.ErrWithHint(output.ExitValidation, "validation", - fmt.Sprintf("--path contains %d credential file(s) that should not be published: %s", len(hits), sample), - "remove these files from the publish payload, OR pass --allow-sensitive if shipping them is intentional (e.g. a docs site demoing credential-file formats)") + return appsValidationParamError("--path", + "--path contains %d credential file(s) that should not be published: %s", len(hits), sample). + WithHint("remove these files from the publish payload, OR pass --allow-sensitive if shipping them is intentional (e.g. a docs site demoing credential-file formats)") } // maxHTMLPublishTarballBytes 是 client 端 tar.gz 包体上限，对齐 OAPI 设计 20MB 约束。 @@ -174,15 +174,14 @@ func ensureIndexHTML(candidates []htmlPublishCandidate) error { return nil } } - return output.ErrWithHint(output.ExitAPI, "validation", - "--path 中缺少 index.html", - "妙搭以 index.html 作为应用入口；目录形态把首页放在根目录命名 index.html，单文件形态把文件命名为 index.html") + return appsFailedPreconditionParamError("--path", "--path is missing index.html"). + WithHint("Miaoda uses index.html as the app entrypoint; for a directory put index.html at the root, or pass a single file named index.html") } -func runHTMLPublish(ctx context.Context, fio fileio.FileIO, client appsHTMLPublishClient, spec appsHTMLPublishSpec) (map[string]interface{}, error) { +func runHTMLPublish(ctx context.Context, fio fileio.FileIO, publisher appsHTMLPublishClient, spec appsHTMLPublishSpec) (map[string]interface{}, error) { candidates, err := walkHTMLPublishCandidates(fio, spec.Path) if err != nil { - return nil, output.Errorf(output.ExitAPI, "io", "scan --path %s: %v", spec.Path, err) + return nil, err } if err := ensureIndexHTML(candidates); err != nil { return nil, err @@ -192,24 +191,24 @@ func runHTMLPublish(ctx context.Context, fio fileio.FileIO, client appsHTMLPubli rawTotal += c.Size } if rawTotal > maxHTMLPublishRawBytes { - return nil, output.ErrWithHint(output.ExitAPI, "validation", - fmt.Sprintf("--path total raw bytes %d exceeds %d bytes limit (uncompressed pre-pack cap)", rawTotal, maxHTMLPublishRawBytes), - "在 tar+gzip 进入内存前拦截，避免 OOM；精简 --path 内容或选择更小的子目录") + return nil, appsValidationParamError("--path", + "--path total raw bytes %d exceeds %d bytes limit (uncompressed pre-pack cap)", rawTotal, maxHTMLPublishRawBytes). + WithHint("reduce --path contents or choose a smaller subdirectory before packaging") } tarball, err := buildHTMLPublishTarball(fio, candidates) if err != nil { - return nil, output.Errorf(output.ExitAPI, "io", "pack: %v", err) + return nil, err } if tarball.Size > maxHTMLPublishTarballBytes { - return nil, output.ErrWithHint(output.ExitAPI, "validation", - fmt.Sprintf("packed tar.gz size %d bytes exceeds %d bytes limit", tarball.Size, maxHTMLPublishTarballBytes), - "请精简 --path 目录（去掉无关大文件 / 压缩资源）后重试；本期接口上限 20MB") + return nil, appsValidationParamError("--path", + "packed tar.gz size %d bytes exceeds %d bytes limit", tarball.Size, maxHTMLPublishTarballBytes). + WithHint("reduce --path contents, remove unrelated large files, then retry") } - resp, err := client.HTMLPublish(ctx, spec.AppID, tarball) + resp, err := publisher.HTMLPublish(ctx, spec.AppID, tarball) if err != nil { - return nil, err + return nil, client.WrapDoAPIError(err) } out := map[string]interface{}{} diff --git a/shortcuts/apps/apps_html_publish_test.go b/shortcuts/apps/apps_html_publish_test.go index 1d8e942fe..788e1c067 100644 --- a/shortcuts/apps/apps_html_publish_test.go +++ b/shortcuts/apps/apps_html_publish_test.go @@ -10,8 +10,6 @@ import ( "path/filepath" "strings" "testing" - - "github.com/larksuite/cli/internal/output" ) type fakeAppsHTMLPublishClient struct { @@ -105,17 +103,11 @@ func TestRunHTMLPublish_DirRequiresIndexHTML(t *testing.T) { if err == nil { t.Fatalf("expected error for missing index.html") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected ExitError with detail, got %v", err) - } - if exitErr.Detail.Type != "validation" { - t.Fatalf("error.type = %q, want validation", exitErr.Detail.Type) + problem := requireAppsValidationProblem(t, err) + if !strings.Contains(problem.Message, "index.html") { + t.Fatalf("message missing 'index.html': %v", problem.Message) } - if !strings.Contains(exitErr.Detail.Message, "index.html") { - t.Fatalf("message missing 'index.html': %v", exitErr.Detail.Message) - } - if exitErr.Detail.Hint == "" { + if problem.Hint == "" { t.Fatalf("expected non-empty hint") } if len(fake.calls) != 0 { @@ -153,10 +145,7 @@ func TestRunHTMLPublish_SingleFileRejectedIfNotNamedIndex(t *testing.T) { if err == nil { t.Fatalf("single-file path 'foo.html' should be rejected (not named index.html)") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil || exitErr.Detail.Type != "validation" { - t.Fatalf("expected ExitError type=validation, got %v", err) - } + requireAppsValidationProblem(t, err) if len(fake.calls) != 0 { t.Fatalf("client must not be called when index.html missing") } @@ -199,17 +188,11 @@ func TestRunHTMLPublish_RejectsOversizeTarball(t *testing.T) { if err == nil { t.Fatalf("expected oversize error") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected ExitError with detail, got %v", err) + problem := requireAppsValidationProblem(t, err) + if !strings.Contains(problem.Message, "exceeds") { + t.Fatalf("message missing 'exceeds': %v", problem.Message) } - if exitErr.Detail.Type != "validation" { - t.Fatalf("error.type = %q, want validation", exitErr.Detail.Type) - } - if !strings.Contains(exitErr.Detail.Message, "exceeds") { - t.Fatalf("message missing 'exceeds': %v", exitErr.Detail.Message) - } - if exitErr.Detail.Hint == "" { + if problem.Hint == "" { t.Fatalf("expected non-empty hint") } if len(fake.calls) != 0 { @@ -337,18 +320,12 @@ func TestAppsHTMLPublish_SensitiveBlocksValidate(t *testing.T) { if err == nil { t.Fatalf("dry-run with sensitive file should fail") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected ExitError with detail, got %v", err) - } - if exitErr.Detail.Type != "validation" { - t.Fatalf("error.type = %q, want validation", exitErr.Detail.Type) - } - if !strings.Contains(exitErr.Detail.Message, ".env") { - t.Fatalf("error message should list the offending file, got %q", exitErr.Detail.Message) + problem := requireAppsValidationProblem(t, err) + if !strings.Contains(problem.Message, ".env") { + t.Fatalf("error message should list the offending file, got %q", problem.Message) } - if !strings.Contains(exitErr.Detail.Hint, "--allow-sensitive") { - t.Fatalf("error hint should mention --allow-sensitive escape hatch, got %q", exitErr.Detail.Hint) + if !strings.Contains(problem.Hint, "--allow-sensitive") { + t.Fatalf("error hint should mention --allow-sensitive escape hatch, got %q", problem.Hint) } } @@ -438,15 +415,9 @@ func TestAppsHTMLPublish_SensitiveBlocksWhenPathIsCredentialParentDir(t *testing if err == nil { t.Fatalf("expected rejection when --path is %s/ (would leak %s), got success", tc.parent, tc.fileName) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected ExitError with detail, got %v", err) - } - if exitErr.Detail.Type != "validation" { - t.Fatalf("error.type = %q, want validation", exitErr.Detail.Type) - } - if !strings.Contains(exitErr.Detail.Message, tc.wantSubstr) { - t.Fatalf("error message should name the leaked file, got %q", exitErr.Detail.Message) + problem := requireAppsValidationProblem(t, err) + if !strings.Contains(problem.Message, tc.wantSubstr) { + t.Fatalf("error message should name the leaked file, got %q", problem.Message) } }) } @@ -480,15 +451,9 @@ func TestAppsHTMLPublish_SensitiveBlocksWhenPathIsCredentialFileItself(t *testin if err == nil { t.Fatalf("expected rejection when --path points directly at .aws/credentials, got success") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected ExitError with detail, got %v", err) - } - if exitErr.Detail.Type != "validation" { - t.Fatalf("error.type = %q, want validation", exitErr.Detail.Type) - } - if !strings.Contains(exitErr.Detail.Message, "credentials") { - t.Fatalf("error message should name the leaked file, got %q", exitErr.Detail.Message) + problem := requireAppsValidationProblem(t, err) + if !strings.Contains(problem.Message, "credentials") { + t.Fatalf("error message should name the leaked file, got %q", problem.Message) } } @@ -498,11 +463,7 @@ func TestAppsHTMLPublish_SensitiveBlocksWhenPathIsCredentialFileItself(t *testin func TestSensitiveCandidatesError_Truncation(t *testing.T) { hits := []string{"a.env", "b.env", "c.env", "d.env", "e.env", "f.env", "g.env"} err := sensitiveCandidatesError(hits) - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected ExitError with detail, got %v", err) - } - msg := exitErr.Detail.Message + msg := requireAppsValidationProblem(t, err).Message if !strings.Contains(msg, "7 credential file(s)") { t.Fatalf("message should report the full count, got %q", msg) } @@ -534,15 +495,9 @@ func TestRunHTMLPublish_RejectsOversizeRawCandidates(t *testing.T) { if err == nil { t.Fatalf("expected raw-size cap to fire") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected ExitError with detail, got %v", err) - } - if exitErr.Detail.Type != "validation" { - t.Fatalf("error.type = %q, want validation", exitErr.Detail.Type) - } - if !strings.Contains(exitErr.Detail.Message, "raw") || !strings.Contains(exitErr.Detail.Message, "bytes") { - t.Fatalf("expected message to explain raw-byte cap, got %q", exitErr.Detail.Message) + problem := requireAppsValidationProblem(t, err) + if !strings.Contains(problem.Message, "raw") || !strings.Contains(problem.Message, "bytes") { + t.Fatalf("expected message to explain raw-byte cap, got %q", problem.Message) } if len(fake.calls) != 0 { t.Fatalf("client must not be called when raw cap hit") diff --git a/shortcuts/apps/apps_list.go b/shortcuts/apps/apps_list.go index edfcca54c..a79acd442 100644 --- a/shortcuts/apps/apps_list.go +++ b/shortcuts/apps/apps_list.go @@ -39,7 +39,7 @@ var AppsList = common.Shortcut{ Params(buildAppsListParams(rctx)) }, Execute: func(ctx context.Context, rctx *common.RuntimeContext) error { - data, err := rctx.CallAPI("GET", apiBasePath+"/apps", buildAppsListParams(rctx), nil) + data, err := rctx.CallAPITyped("GET", apiBasePath+"/apps", buildAppsListParams(rctx), nil) if err != nil { return err } diff --git a/shortcuts/apps/apps_update.go b/shortcuts/apps/apps_update.go index 16ea4f364..057cbf23e 100644 --- a/shortcuts/apps/apps_update.go +++ b/shortcuts/apps/apps_update.go @@ -9,7 +9,6 @@ import ( "io" "strings" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -30,11 +29,15 @@ var AppsUpdate = common.Shortcut{ }, Validate: func(ctx context.Context, rctx *common.RuntimeContext) error { if strings.TrimSpace(rctx.Str("app-id")) == "" { - return output.ErrValidation("--app-id is required") + return appsValidationParamError("--app-id", "--app-id is required") } body := buildAppsUpdateBody(rctx) if len(body) == 0 { - return output.ErrValidation("provide at least one of --name or --description") + return appsValidationError("provide at least one of --name or --description"). + WithParams( + appsInvalidParam("--name", "provide at least one of --name or --description"), + appsInvalidParam("--description", "provide at least one of --name or --description"), + ) } return nil }, @@ -48,7 +51,7 @@ var AppsUpdate = common.Shortcut{ Execute: func(ctx context.Context, rctx *common.RuntimeContext) error { appID := strings.TrimSpace(rctx.Str("app-id")) path := fmt.Sprintf("%s/apps/%s", apiBasePath, validate.EncodePathSegment(appID)) - data, err := rctx.CallAPI("PATCH", path, nil, buildAppsUpdateBody(rctx)) + data, err := rctx.CallAPITyped("PATCH", path, nil, buildAppsUpdateBody(rctx)) if err != nil { return err } diff --git a/shortcuts/apps/html_publish_client.go b/shortcuts/apps/html_publish_client.go index c1b05b93b..295873dd0 100644 --- a/shortcuts/apps/html_publish_client.go +++ b/shortcuts/apps/html_publish_client.go @@ -6,13 +6,13 @@ package apps import ( "bytes" "context" - "encoding/json" "fmt" "net/http" larkcore "github.com/larksuite/oapi-sdk-go/v3/core" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/client" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/common" ) @@ -39,28 +39,18 @@ func (api appsHTMLPublishAPI) HTMLPublish(ctx context.Context, appID string, tar Body: fd, }, larkcore.WithFileUpload()) if err != nil { - return nil, err + return nil, client.WrapDoAPIError(err) } - return parseHTMLPublishResponse(apiResp.RawBody) -} - -func parseHTMLPublishResponse(raw []byte) (*htmlPublishResponse, error) { - var envelope struct { - Code int `json:"code"` - Msg string `json:"msg"` - Data struct { - URL string `json:"url"` - } `json:"data"` - } - if err := json.Unmarshal(raw, &envelope); err != nil { - return nil, fmt.Errorf("decode html-publish response: %w", err) + data, err := api.runtime.ClassifyAPIResponse(apiResp) + if err != nil { + return nil, enrichHTMLPublishAPIError(err) } - if envelope.Code != 0 { - return nil, output.ErrWithHint(output.ExitAPI, "api_error", - fmt.Sprintf("html-publish failed (code=%d): %s", envelope.Code, envelope.Msg), - buildHTMLPublishFailureHint(envelope.Code)) + url, _ := data["url"].(string) + if url == "" { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, + "html-publish response is missing the published app url") } - return &htmlPublishResponse{URL: envelope.Data.URL}, nil + return &htmlPublishResponse{URL: url}, nil } // OAPI business error codes returned by the Miaoda @@ -74,9 +64,9 @@ const ( func buildHTMLPublishFailureHint(code int) string { switch code { case errCodeBuildFailed: - return "构建失败：用 `lark-cli apps +html-publish --app-id --path --dry-run` 检查打包文件清单" + return "server-side build failed: run `lark-cli apps +html-publish --app-id --path --dry-run` to inspect the packaged file list" case errCodeAppNotFound: - return "应用不存在或无权访问；请用户确认 app_id（从妙搭应用链接 https://miaoda.feishu.cn/app/app_xxx 的 /app/ 后面提取，或直接给 app_xxx 字符串）" + return "the app does not exist or the caller has no access; ask the user to confirm the app_id (extract it from the Miaoda app URL https://miaoda.feishu.cn/app/app_xxx after /app/, or take the app_xxx string directly)" default: return "" } diff --git a/shortcuts/apps/html_publish_client_test.go b/shortcuts/apps/html_publish_client_test.go index ca0a5949c..c7a0ca488 100644 --- a/shortcuts/apps/html_publish_client_test.go +++ b/shortcuts/apps/html_publish_client_test.go @@ -6,21 +6,21 @@ package apps import ( "bytes" "context" - "errors" "mime" "mime/multipart" "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" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) func newAppsClientRuntime(t *testing.T) (*common.RuntimeContext, *httpmock.Registry) { t.Helper() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) cfg := &core.CliConfig{ AppID: "test-app-" + strings.ToLower(t.Name()), AppSecret: "test-secret", @@ -94,15 +94,57 @@ func TestAppsHTMLPublishAPI_BusinessErrorHasHint(t *testing.T) { if err == nil { t.Fatalf("expected error") } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("expected ExitError with detail, got %v", err) + problem := requireAppsAPIProblem(t, err) + if problem.Code != errCodeBuildFailed { + t.Fatalf("code = %d, want %d", problem.Code, errCodeBuildFailed) } - if exitErr.Detail.Hint == "" { + if problem.Hint == "" { t.Fatalf("expected non-empty hint on code 90001") } - if !strings.Contains(exitErr.Detail.Message, "build failed") { - t.Fatalf("missing failure message: %v", exitErr.Detail.Message) + if !strings.Contains(problem.Message, "build failed") { + t.Fatalf("missing failure message: %v", problem.Message) + } +} + +func TestAppsHTMLPublishAPI_AppNotFoundClassified(t *testing.T) { + rctx, reg := newAppsClientRuntime(t) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/spark/v1/apps/app_missing/upload_and_release_html_code", + Body: map[string]interface{}{ + "code": errCodeAppNotFound, + "msg": "app not found", + }, + }) + + api := appsHTMLPublishAPI{runtime: rctx} + _, err := api.HTMLPublish(context.Background(), "app_missing", &htmlPublishTarball{Body: []byte("fake")}) + problem := requireAppsAPIProblem(t, err) + if problem.Subtype != errs.SubtypeNotFound { + t.Fatalf("subtype = %q, want %q", problem.Subtype, errs.SubtypeNotFound) + } + if problem.Hint == "" { + t.Fatalf("expected app-not-found recovery hint") + } +} + +func TestAppsHTMLPublishAPI_MissingURLIsInvalidResponse(t *testing.T) { + rctx, reg := newAppsClientRuntime(t) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/spark/v1/apps/app_x/upload_and_release_html_code", + Body: map[string]interface{}{ + "code": 0, + "msg": "success", + "data": map[string]interface{}{}, + }, + }) + + api := appsHTMLPublishAPI{runtime: rctx} + _, err := api.HTMLPublish(context.Background(), "app_x", &htmlPublishTarball{Body: []byte("fake")}) + problem := requireAppsProblem(t, err, errs.CategoryInternal) + if problem.Subtype != errs.SubtypeInvalidResponse { + t.Fatalf("subtype = %q, want %q", problem.Subtype, errs.SubtypeInvalidResponse) } } diff --git a/shortcuts/apps/html_publish_tarball.go b/shortcuts/apps/html_publish_tarball.go index b49d4eb77..4439150a8 100644 --- a/shortcuts/apps/html_publish_tarball.go +++ b/shortcuts/apps/html_publish_tarball.go @@ -9,8 +9,6 @@ import ( "compress/gzip" "crypto/sha256" "encoding/hex" - "errors" - "fmt" "io" "github.com/larksuite/cli/extension/fileio" @@ -26,7 +24,7 @@ type htmlPublishTarball struct { func buildHTMLPublishTarball(fio fileio.FileIO, candidates []htmlPublishCandidate) (*htmlPublishTarball, error) { if len(candidates) == 0 { - return nil, errors.New("no files to pack") + return nil, appsValidationParamError("--path", "no files to pack") } var buf bytes.Buffer @@ -45,10 +43,10 @@ func buildHTMLPublishTarball(fio fileio.FileIO, candidates []htmlPublishCandidat if err := tw.Close(); err != nil { _ = gz.Close() - return nil, fmt.Errorf("tar close: %w", err) + return nil, appsFileIOError(err, "tar close: %v", err) } if err := gz.Close(); err != nil { - return nil, fmt.Errorf("gzip close: %w", err) + return nil, appsFileIOError(err, "gzip close: %v", err) } return &htmlPublishTarball{ @@ -60,12 +58,12 @@ func buildHTMLPublishTarball(fio fileio.FileIO, candidates []htmlPublishCandidat func writeHTMLPublishTarEntry(fio fileio.FileIO, tw *tar.Writer, c htmlPublishCandidate) error { if isUnsafeRelPath(c.RelPath) { - return fmt.Errorf("invalid tar entry name %q", c.RelPath) + return appsValidationParamError("--path", "invalid tar entry name %q", c.RelPath) } src, err := fio.Open(c.AbsPath) if err != nil { - return fmt.Errorf("open %s: %w", c.AbsPath, err) + return appsInputPathEntryError(c.AbsPath, err) } defer src.Close() @@ -76,10 +74,10 @@ func writeHTMLPublishTarEntry(fio fileio.FileIO, tw *tar.Writer, c htmlPublishCa Typeflag: tar.TypeReg, } if err := tw.WriteHeader(hdr); err != nil { - return fmt.Errorf("write header %s: %w", c.RelPath, err) + return appsFileIOError(err, "write header %s: %v", c.RelPath, err) } if _, err := io.Copy(tw, src); err != nil { - return fmt.Errorf("copy %s: %w", c.RelPath, err) + return appsFileIOError(err, "copy %s: %v", c.RelPath, err) } return nil } diff --git a/shortcuts/apps/walk_html_publish_candidates.go b/shortcuts/apps/walk_html_publish_candidates.go index a9ef79412..998c65ad7 100644 --- a/shortcuts/apps/walk_html_publish_candidates.go +++ b/shortcuts/apps/walk_html_publish_candidates.go @@ -4,7 +4,6 @@ package apps import ( - "fmt" "io/fs" "path/filepath" "strings" @@ -40,7 +39,7 @@ func isUnsafeRelPath(rel string) bool { func walkHTMLPublishCandidates(fio fileio.FileIO, rootPath string) ([]htmlPublishCandidate, error) { stat, err := fio.Stat(rootPath) if err != nil { - return nil, fmt.Errorf("stat %s: %w", rootPath, err) + return nil, appsInputPathError(err) } if !stat.IsDir() { return []htmlPublishCandidate{{ @@ -54,14 +53,14 @@ func walkHTMLPublishCandidates(fio fileio.FileIO, rootPath string) ([]htmlPublis //nolint:forbidigo // fileio has no WalkDir; rootPath is already validated above via fio.Stat -> SafeInputPath. err = filepath.WalkDir(rootPath, func(path string, d fs.DirEntry, walkErr error) error { if walkErr != nil { - return walkErr + return appsInputPathEntryError(path, walkErr) } if d.IsDir() { return nil } info, err := d.Info() if err != nil { - return err + return appsInputPathEntryError(path, err) } // 只接受 regular file —— symlink / device / pipe / socket 都跳过。 // symlink 不跟随是设计决策（避免 loop + out-of-root 引用），且 fio.Open 也会拒非 regular。 @@ -70,7 +69,7 @@ func walkHTMLPublishCandidates(fio fileio.FileIO, rootPath string) ([]htmlPublis } rel, err := filepath.Rel(rootPath, path) if err != nil { - return err + return appsFileIOError(err, "resolve relative path for %s: %v", path, err) } relSlash := filepath.ToSlash(rel) // Defense in depth: WalkDir + Rel inside rootPath should never yield a @@ -78,7 +77,7 @@ func walkHTMLPublishCandidates(fio fileio.FileIO, rootPath string) ([]htmlPublis // filesystem layout shouldn't be able to inject one into RelPath. // Mirrors the same guard at tar entry write time. if isUnsafeRelPath(relSlash) { - return fmt.Errorf("walker produced unsafe relative path %q for %s", relSlash, path) + return appsValidationParamError("--path", "walker produced unsafe relative path %q for %s", relSlash, path) } out = append(out, htmlPublishCandidate{ RelPath: relSlash,