From 9bc66cc4459950cf4cda872471bc2eb343dd77e8 Mon Sep 17 00:00:00 2001 From: evandance <120630830+evandance@users.noreply.github.com> Date: Thu, 11 Jun 2026 19:04:34 +0800 Subject: [PATCH] feat(apps): emit typed error envelopes across the apps domain (#1288) --- .golangci.yml | 6 +- errs/subtypes.go | 1 + .../rule_no_legacy_common_helper_call.go | 1 + .../rule_no_legacy_envelope_literal.go | 1 + shortcuts/apps/apps_access_scope_get.go | 3 +- shortcuts/apps/apps_access_scope_set.go | 39 +++--- shortcuts/apps/apps_chat.go | 7 +- shortcuts/apps/apps_create.go | 3 +- shortcuts/apps/apps_create_test.go | 26 ++++ shortcuts/apps/apps_db_execute.go | 64 +++++---- shortcuts/apps/apps_db_execute_test.go | 127 +++++++++++++----- shortcuts/apps/apps_db_table_get.go | 3 +- shortcuts/apps/apps_env_pull.go | 18 +-- shortcuts/apps/apps_env_pull_test.go | 25 ++++ shortcuts/apps/apps_errors.go | 104 ++++++++++++++ shortcuts/apps/apps_errors_test.go | 113 ++++++++++++++++ shortcuts/apps/apps_html_publish.go | 39 +++--- shortcuts/apps/apps_html_publish_test.go | 93 ++++--------- shortcuts/apps/apps_init.go | 82 +++++------ shortcuts/apps/apps_init_test.go | 26 ++++ shortcuts/apps/apps_release_create.go | 3 +- shortcuts/apps/apps_release_get.go | 5 +- shortcuts/apps/apps_release_list.go | 2 +- shortcuts/apps/apps_session_create.go | 3 +- shortcuts/apps/apps_session_get.go | 5 +- shortcuts/apps/apps_session_list.go | 2 +- shortcuts/apps/apps_session_stop.go | 7 +- shortcuts/apps/apps_update.go | 9 +- shortcuts/apps/common.go | 21 +-- shortcuts/apps/common_test.go | 44 +++--- shortcuts/apps/db_common.go | 3 +- shortcuts/apps/git_credential.go | 115 +++++++--------- shortcuts/apps/git_credential_storage.go | 3 +- shortcuts/apps/git_credential_test.go | 96 +++++++++---- shortcuts/apps/gitcred/gitconfig.go | 8 +- shortcuts/apps/gitcred/helper.go | 35 ++--- shortcuts/apps/gitcred/lock.go | 4 +- shortcuts/apps/gitcred/url.go | 12 +- shortcuts/apps/html_publish_client.go | 36 ++--- shortcuts/apps/html_publish_client_test.go | 74 ++++++++-- shortcuts/apps/html_publish_tarball.go | 17 ++- shortcuts/apps/storage.go | 16 +-- .../apps/walk_html_publish_candidates.go | 12 +- shortcuts/common/runner.go | 10 +- .../references/lark-apps-db-execute.md | 2 +- 45 files changed, 851 insertions(+), 474 deletions(-) create mode 100644 shortcuts/apps/apps_errors.go create mode 100644 shortcuts/apps/apps_errors_test.go diff --git a/.golangci.yml b/.golangci.yml index b198d6ab..260e4b06 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/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/markdown/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/wiki/|internal/event/consume/|cmd/event/|events/|shortcuts/event/) + - 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/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/markdown/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/wiki/|internal/event/consume/|cmd/event/|events/|shortcuts/event/) 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/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/markdown/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/wiki/|shortcuts/common/mcp_client\.go|cmd/event/|events/|shortcuts/event/) + - path-except: (shortcuts/apps/|shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/markdown/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/wiki/|shortcuts/common/mcp_client\.go|cmd/event/|events/|shortcuts/event/) 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/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/markdown/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/wiki/|cmd/event/|events/|shortcuts/event/) + - path-except: (shortcuts/apps/|shortcuts/base/|shortcuts/calendar/|shortcuts/contact/|shortcuts/doc/|shortcuts/drive/|shortcuts/im/|shortcuts/mail/|shortcuts/markdown/|shortcuts/minutes/|shortcuts/okr/|shortcuts/sheets/|shortcuts/slides/|shortcuts/task/|shortcuts/vc/|shortcuts/whiteboard/|shortcuts/wiki/|cmd/event/|events/|shortcuts/event/) text: errs-no-legacy-helper linters: - forbidigo diff --git a/errs/subtypes.go b/errs/subtypes.go index 68a5dd37..913170d6 100644 --- a/errs/subtypes.go +++ b/errs/subtypes.go @@ -80,6 +80,7 @@ const ( SubtypeSDKError Subtype = "sdk_error" // lark SDK Do() returned an unexpected error SubtypeInvalidResponse Subtype = "invalid_response" // SDK response body not parsable as JSON SubtypeFileIO Subtype = "file_io" // local file I/O failure (mkdir / write / read) + SubtypeExternalTool Subtype = "external_tool" // an external tool the CLI shells out to (git, npx) failed at runtime; the tool output is in the message SubtypeStorage Subtype = "storage" // local persistence failure (e.g. config file save) // Generic untyped error lifted to InternalError uses SubtypeUnknown. ) diff --git a/lint/errscontract/rule_no_legacy_common_helper_call.go b/lint/errscontract/rule_no_legacy_common_helper_call.go index 723fc657..e6f43959 100644 --- a/lint/errscontract/rule_no_legacy_common_helper_call.go +++ b/lint/errscontract/rule_no_legacy_common_helper_call.go @@ -18,6 +18,7 @@ var migratedCommonHelperPaths = []string{ "cmd/event/", "events/", "internal/event/consume/", + "shortcuts/apps/", "shortcuts/base/", "shortcuts/calendar/", "shortcuts/contact/", diff --git a/lint/errscontract/rule_no_legacy_envelope_literal.go b/lint/errscontract/rule_no_legacy_envelope_literal.go index 61787452..57631ba8 100644 --- a/lint/errscontract/rule_no_legacy_envelope_literal.go +++ b/lint/errscontract/rule_no_legacy_envelope_literal.go @@ -19,6 +19,7 @@ var migratedEnvelopePaths = []string{ "cmd/event/", "events/", "internal/event/consume/", + "shortcuts/apps/", "shortcuts/base/", "shortcuts/calendar/", "shortcuts/contact/", diff --git a/shortcuts/apps/apps_access_scope_get.go b/shortcuts/apps/apps_access_scope_get.go index 390aee5d..4e8e0fcf 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" ) @@ -32,7 +31,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 }, diff --git a/shortcuts/apps/apps_access_scope_set.go b/shortcuts/apps/apps_access_scope_set.go index ad1b5595..99117cae 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" ) @@ -45,7 +44,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) }, @@ -90,36 +89,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 } @@ -127,18 +132,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 @@ -157,7 +162,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} @@ -166,7 +171,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_chat.go b/shortcuts/apps/apps_chat.go index a2932b36..bcad4536 100644 --- a/shortcuts/apps/apps_chat.go +++ b/shortcuts/apps/apps_chat.go @@ -9,7 +9,6 @@ import ( "io" "strings" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -43,14 +42,14 @@ var AppsChat = 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") } if strings.TrimSpace(rctx.Str("session-id")) == "" { - return output.ErrValidation("--session-id is required") + return appsValidationParamError("--session-id", "--session-id is required") } // Do not echo --message content in the error (spec §4 redaction). if strings.TrimSpace(rctx.Str("message")) == "" { - return output.ErrValidation("--message is required") + return appsValidationParamError("--message", "--message is required") } return nil }, diff --git a/shortcuts/apps/apps_create.go b/shortcuts/apps/apps_create.go index c96f877c..cbbcbf5b 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" ) @@ -36,7 +35,7 @@ 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") } return nil }, diff --git a/shortcuts/apps/apps_create_test.go b/shortcuts/apps/apps_create_test.go index 249c507d..ddc63671 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" @@ -47,6 +48,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_db_execute.go b/shortcuts/apps/apps_db_execute.go index 9daaf4d2..5af2cde1 100644 --- a/shortcuts/apps/apps_db_execute.go +++ b/shortcuts/apps/apps_db_execute.go @@ -31,8 +31,9 @@ import ( // - 多语句部分失败：`Statement K: ✗ []` + 末尾「前序语句已落地」提示 // // 失败语义：server 多语句失败仍返 code:0，把失败语句标成 ERROR 哨兵塞进 result。Execute 检测到哨兵 -// 后升级成 typed api_error（exit 非 0、detail 带 statement_index / completed / rolled_back）， -// 避免 agent 误判 ok:true 假成功。CLI 永远 DBA 模式（transactional=false），失败前的语句已 auto-commit +// 后按 partial failure 上报（exit 非 0）：stdout 输出 ok:false 数据，带 results / +// statement_index / error_code / error_message / rolled_back / note，避免 agent 误判 +// ok:true 假成功。CLI 永远 DBA 模式（transactional=false），失败前的语句已 auto-commit // 落地，故 rolled_back=false（真机 boe 实证）。 // // JSON envelope（成功路径）：CLI 把 server 返的 result 字符串解出来放进 `data.results` 数组。 @@ -68,19 +69,27 @@ var AppsDBExecute = common.Shortcut{ sql := strings.TrimSpace(rctx.Str("sql")) file := strings.TrimSpace(rctx.Str("file")) if sql != "" && file != "" { - return output.ErrValidation("--sql and --file are mutually exclusive") + return appsValidationError("--sql and --file are mutually exclusive"). + WithParams( + appsInvalidParam("--sql", "mutually exclusive with --file"), + appsInvalidParam("--file", "mutually exclusive with --sql"), + ) } if file != "" { data, err := cmdutil.ReadInputFile(rctx.FileIO(), file) if err != nil { - return output.ErrValidation("--file: %v", err) + return appsValidationParamError("--file", "--file: %v", err).WithCause(err) } // 归一化：把文件内容写回 --sql，下游（DryRun/Execute）统一从 sql 取。 rctx.Cmd.Flags().Set("sql", string(data)) sql = strings.TrimSpace(string(data)) } if sql == "" { - return output.ErrValidation("one of --sql or --file is required (use --sql - to read stdin)") + return appsValidationError("one of --sql or --file is required (use --sql - to read stdin)"). + WithParams( + appsInvalidParam("--sql", "one of --sql or --file is required"), + appsInvalidParam("--file", "one of --sql or --file is required"), + ) } return nil }, @@ -113,13 +122,15 @@ var AppsDBExecute = common.Shortcut{ data := map[string]interface{}{"results": stmts} // 多语句 / 单语句失败：server 仍返 code:0，把失败语句标成 ERROR 哨兵塞进 result。 - // 升级成 typed api_error（exit 非 0），别让 agent 误判 ok:true 假成功。 - // pretty 模式仍把逐条 ✓/✗ 摘要打到 stdout（人看），再返回 error（envelope→stderr）。 + // 已落地的前序语句 + 失败语句构成 partial failure：逐条结果作为 ok:false 数据 + // 留在 stdout（机器可读）+ 非零退出信号，别让 agent 误判 ok:true 假成功。 + // pretty 模式 stdout 只打逐条 ✓/✗ 摘要（不再叠一份 JSON envelope），仅返回退出信号。 if errIdx, errStmt, failed := findErrorSentinel(stmts); failed { if rctx.Format == "pretty" { renderSQLPretty(rctx.IO().Out, stmts) + return output.PartialFailure(output.ExitAPI) } - return sqlStatementError(stmts, errIdx, errStmt) + return rctx.OutPartialFailure(sqlStatementFailurePayload(stmts, errIdx, errStmt), nil) } rctx.OutFormat(data, nil, func(w io.Writer) { @@ -140,31 +151,28 @@ func findErrorSentinel(stmts []map[string]interface{}) (int, map[string]interfac return 0, nil, false } -// sqlStatementError 把 ERROR 哨兵升级成 typed api_error。 +// sqlStatementFailurePayload 把 ERROR 哨兵整理成 partial-failure 的 stdout 数据。 // // CLI 永远 DBA 模式（transactional=false），真机 boe 实证：失败语句之前的语句已逐条 auto-commit -// 落地，不存在外层事务回滚。因此 rolled_back=false、completed 列出已落地的前序语句，hint 提示用户 -// 别整批重跑（否则会重复写入）。 -func sqlStatementError(stmts []map[string]interface{}, errIdx int, errStmt map[string]interface{}) error { +// 落地，不存在外层事务回滚。因此 rolled_back=false、results 含全部逐条结果（ERROR 哨兵在 +// 失败位置），note 提示用户别整批重跑（否则会重复写入）。 +func sqlStatementFailurePayload(stmts []map[string]interface{}, errIdx int, errStmt map[string]interface{}) map[string]interface{} { code, msg := parseErrorSentinel(common.GetString(errStmt, "data")) stmtNo := errIdx + 1 // 1-based 给人看 - fullMsg := fmt.Sprintf("%s (at statement %d of %d)", msg, stmtNo, len(stmts)) - - apiErr := output.ErrAPI(code, fullMsg, map[string]interface{}{ - "statement_index": errIdx, - "completed": stmts[:errIdx], - "rolled_back": false, - }) - if apiErr.Detail != nil { - if errIdx > 0 { - apiErr.Detail.Hint = fmt.Sprintf( - "statements 1-%d were already applied (DBA mode auto-commits each statement); fix statement %d and re-run only the remaining statements.", - errIdx, stmtNo) - } else { - apiErr.Detail.Hint = "no statements were applied; fix the SQL and re-run." - } + note := "no statements were applied; fix the SQL and re-run." + if errIdx > 0 { + note = fmt.Sprintf( + "statements 1-%d were already applied (DBA mode auto-commits each statement); fix statement %d and re-run only the remaining statements.", + errIdx, stmtNo) + } + return map[string]interface{}{ + "results": stmts, + "statement_index": errIdx, + "error_code": code, + "error_message": fmt.Sprintf("%s (at statement %d of %d)", msg, stmtNo, len(stmts)), + "rolled_back": false, + "note": note, } - return apiErr } // parseErrorSentinel 解析 ERROR 哨兵的 data（`{code,message}` JSON），返回数值 code 与 message。 diff --git a/shortcuts/apps/apps_db_execute_test.go b/shortcuts/apps/apps_db_execute_test.go index 66e966ea..cb95d3f9 100644 --- a/shortcuts/apps/apps_db_execute_test.go +++ b/shortcuts/apps/apps_db_execute_test.go @@ -495,9 +495,9 @@ func TestAppsDBExecute_PrettyMultiStatementsPartialFailureWithErrorSentinel(t *t } } -// TestAppsDBExecute_MultiStatementFailureReturnsTypedError 钉死「多语句失败 → typed api_error」： -// json 默认不再打 ok:true 假成功，而是返回 *output.ExitError（type=api_error、非零 exit）， -// detail 带 statement_index / completed / rolled_back。rolled_back=false 因 CLI 永远 DBA 模式 +// TestAppsDBExecute_MultiStatementFailureReturnsTypedError 钉死「多语句失败 → partial failure」： +// 逐条结果 + statement_index / error_code / rolled_back / note 作为 ok:false 数据落 stdout， +// 退出信号是 PartialFailureError（非零 exit）。rolled_back=false 因 CLI 永远 DBA 模式 // （真机 boe 实证：失败前的语句已落地）。 func TestAppsDBExecute_MultiStatementFailureReturnsTypedError(t *testing.T) { factory, stdout, reg := newAppsExecuteFactory(t) @@ -518,45 +518,64 @@ func TestAppsDBExecute_MultiStatementFailureReturnsTypedError(t *testing.T) { []string{"+db-execute", "--yes", "--app-id", "app_x", "--sql", "x", "--as", "user"}, factory, stdout) if err == nil { - t.Fatalf("multi-statement failure must return a typed error; stdout:\n%s", stdout.String()) + t.Fatalf("multi-statement failure must return a partial-failure error; stdout:\n%s", stdout.String()) } // json 失败路径不得打成功 envelope。 if strings.Contains(stdout.String(), `"ok": true`) { t.Errorf("must not emit ok:true success envelope on failure; stdout:\n%s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("want *output.ExitError with detail, got %T: %v", err, err) + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("want *output.PartialFailureError, got %T: %v", err, err) } - if exitErr.Detail.Type != "api_error" { - t.Errorf("error.type = %q, want api_error", exitErr.Detail.Type) + if pfErr.Code != output.ExitAPI { + t.Errorf("exit = %d, want %d (ExitAPI)", pfErr.Code, output.ExitAPI) } - if exitErr.Detail.Code != 1300002 { - t.Errorf("error.code = %d, want 1300002", exitErr.Detail.Code) + payload := decodePartialFailureData(t, stdout.String()) + if got := payload["statement_index"]; got != float64(1) { + t.Errorf("statement_index = %v, want 1", got) } - if !strings.Contains(exitErr.Detail.Message, "(at statement 2 of 2)") { - t.Errorf("error.message missing statement locator: %q", exitErr.Detail.Message) + if got := payload["error_code"]; got != float64(1300002) { + t.Errorf("error_code = %v, want 1300002", got) } - if output.ExitCodeOf(err) != output.ExitAPI { - t.Errorf("exit = %d, want %d (ExitAPI)", output.ExitCodeOf(err), output.ExitAPI) + msg, _ := payload["error_message"].(string) + if !strings.Contains(msg, "(at statement 2 of 2)") { + t.Errorf("error_message missing statement locator: %q", msg) } - detail, ok := exitErr.Detail.Detail.(map[string]interface{}) - if !ok { - t.Fatalf("error.detail not a map: %T", exitErr.Detail.Detail) + if got := payload["rolled_back"]; got != false { + t.Errorf("rolled_back = %v, want false (DBA mode persists prior statements)", got) } - if detail["statement_index"] != 1 { - t.Errorf("statement_index = %v, want 1", detail["statement_index"]) + results, _ := payload["results"].([]interface{}) + if len(results) != 2 { + t.Errorf("results length = %d, want 2 (persisted statement + ERROR sentinel)", len(results)) } - if detail["rolled_back"] != false { - t.Errorf("rolled_back = %v, want false (DBA mode persists prior statements)", detail["rolled_back"]) - } - if completed, ok := detail["completed"].([]map[string]interface{}); !ok || len(completed) != 1 { - t.Errorf("completed = %v, want 1 persisted statement", detail["completed"]) + note, _ := payload["note"].(string) + if !strings.Contains(note, "already applied") { + t.Errorf("note should warn prior statements persisted, got %q", note) } } +// decodePartialFailureData 解析 stdout 上 ok:false 的 partial-failure envelope，返回 data 块。 +func decodePartialFailureData(t *testing.T, stdoutStr string) map[string]interface{} { + t.Helper() + var envelope struct { + OK bool `json:"ok"` + Data map[string]interface{} `json:"data"` + } + if err := json.Unmarshal([]byte(stdoutStr), &envelope); err != nil { + t.Fatalf("stdout is not a JSON envelope: %v\n%s", err, stdoutStr) + } + if envelope.OK { + t.Fatalf("envelope.ok = true, want false on partial failure") + } + if envelope.Data == nil { + t.Fatalf("envelope.data missing; stdout:\n%s", stdoutStr) + } + return envelope.Data +} + // TestAppsDBExecute_SingleErrorReturnsTypedError 单条语句失败（server 也返 code:0 + ERROR 哨兵） -// 同样升级成 typed error：statement_index=0、completed 空、message 标注 (at statement 1 of 1)。 +// 同样走 partial failure：statement_index=0、note 说明无语句落地、message 标注 (at statement 1 of 1)。 func TestAppsDBExecute_SingleErrorReturnsTypedError(t *testing.T) { factory, stdout, reg := newAppsExecuteFactory(t) reg.Register(&httpmock.Stub{ @@ -573,21 +592,23 @@ func TestAppsDBExecute_SingleErrorReturnsTypedError(t *testing.T) { []string{"+db-execute", "--yes", "--app-id", "app_x", "--sql", "x", "--as", "user"}, factory, stdout) if err == nil { - t.Fatalf("single ERROR sentinel must return a typed error; stdout:\n%s", stdout.String()) + t.Fatalf("single ERROR sentinel must return a partial-failure error; stdout:\n%s", stdout.String()) } - var exitErr *output.ExitError - if !errors.As(err, &exitErr) || exitErr.Detail == nil { - t.Fatalf("want *output.ExitError with detail, got %T: %v", err, err) + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("want *output.PartialFailureError, got %T: %v", err, err) } - if !strings.Contains(exitErr.Detail.Message, "(at statement 1 of 1)") { - t.Errorf("error.message missing locator: %q", exitErr.Detail.Message) + payload := decodePartialFailureData(t, stdout.String()) + msg, _ := payload["error_message"].(string) + if !strings.Contains(msg, "(at statement 1 of 1)") { + t.Errorf("error_message missing locator: %q", msg) } - detail, _ := exitErr.Detail.Detail.(map[string]interface{}) - if detail["statement_index"] != 0 { - t.Errorf("statement_index = %v, want 0", detail["statement_index"]) + if got := payload["statement_index"]; got != float64(0) { + t.Errorf("statement_index = %v, want 0", got) } - if completed, ok := detail["completed"].([]map[string]interface{}); !ok || len(completed) != 0 { - t.Errorf("completed = %v, want empty", detail["completed"]) + note, _ := payload["note"].(string) + if !strings.Contains(note, "no statements were applied") { + t.Errorf("note should say nothing was applied, got %q", note) } } @@ -795,3 +816,35 @@ func TestRenderSelectRowsAsTable_Branches(t *testing.T) { }) } } + +// TestAppsDBExecute_PrettyPartialFailureKeepsStdoutHumanOnly pins the pretty +// contract on a statement failure: stdout carries only the per-statement +// human summary (no JSON envelope stacked after it), and the command still +// exits non-zero via the partial-failure signal. +func TestAppsDBExecute_PrettyPartialFailureKeepsStdoutHumanOnly(t *testing.T) { + factory, stdout, reg := newAppsExecuteFactory(t) + reg.Register(&httpmock.Stub{ + Method: "POST", + URL: "/open-apis/spark/v1/apps/app_x/sql_commands", + Body: map[string]interface{}{ + "code": 0, + "data": map[string]interface{}{ + "result": `[{"sql_type":"ERROR","data":"{\"code\":\"k_dl_000002\",\"message\":\"syntax error\"}"}]`, + }, + }, + }) + err := runAppsShortcut(t, AppsDBExecute, + []string{"+db-execute", "--yes", "--app-id", "app_x", "--sql", "x", "--format", "pretty", "--as", "user"}, + factory, stdout) + var pfErr *output.PartialFailureError + if !errors.As(err, &pfErr) { + t.Fatalf("want *output.PartialFailureError, got %T: %v", err, err) + } + out := stdout.String() + if !strings.Contains(out, "✗") { + t.Fatalf("pretty summary missing failure marker; stdout:\n%s", out) + } + if strings.Contains(out, `"ok"`) { + t.Fatalf("pretty stdout must not stack a JSON envelope after the summary; stdout:\n%s", out) + } +} diff --git a/shortcuts/apps/apps_db_table_get.go b/shortcuts/apps/apps_db_table_get.go index 4041d414..eb1ca178 100644 --- a/shortcuts/apps/apps_db_table_get.go +++ b/shortcuts/apps/apps_db_table_get.go @@ -8,7 +8,6 @@ import ( "io" "strings" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -45,7 +44,7 @@ var AppsDBTableGet = common.Shortcut{ return err } if strings.TrimSpace(rctx.Str("table")) == "" { - return output.ErrValidation("--table is required") + return appsValidationParamError("--table", "--table is required") } return nil }, diff --git a/shortcuts/apps/apps_env_pull.go b/shortcuts/apps/apps_env_pull.go index 21c17afb..e2242f9a 100644 --- a/shortcuts/apps/apps_env_pull.go +++ b/shortcuts/apps/apps_env_pull.go @@ -47,11 +47,11 @@ var AppsEnvPull = common.Shortcut{ }, Validate: func(ctx context.Context, rctx *common.RuntimeContext) error { if strings.TrimSpace(rctx.Str("app-id")) == "" { - return &errs.ValidationError{Problem: errs.Problem{Category: errs.CategoryValidation, Subtype: errs.SubtypeInvalidArgument, Message: "--app-id is required"}, Param: "app-id"} + return appsValidationParamError("--app-id", "--app-id is required") } _, envFile, err := resolveEnvPullTarget(strings.TrimSpace(rctx.Str("project-path"))) if err != nil { - return &errs.ValidationError{Problem: errs.Problem{Category: errs.CategoryValidation, Subtype: errs.SubtypeInvalidArgument, Message: fmt.Sprintf("--project-path: %v", err)}, Param: "project-path", Cause: err} + return appsValidationParamError("--project-path", "--project-path: %v", err).WithCause(err) } if err := checkEnvPullTarget(envFile); err != nil { return err @@ -71,7 +71,7 @@ var AppsEnvPull = common.Shortcut{ appID := strings.TrimSpace(rctx.Str("app-id")) _, envFile, err := resolveEnvPullTarget(strings.TrimSpace(rctx.Str("project-path"))) if err != nil { - return &errs.ValidationError{Problem: errs.Problem{Category: errs.CategoryValidation, Subtype: errs.SubtypeInvalidArgument, Message: fmt.Sprintf("--project-path: %v", err)}, Param: "project-path", Cause: err} + return appsValidationParamError("--project-path", "--project-path: %v", err).WithCause(err) } if err := checkEnvPullTarget(envFile); err != nil { return err @@ -120,7 +120,7 @@ func resolveEnvPullTarget(projectPath string) (string, string, error) { if strings.TrimSpace(projectPath) == "" { cwd, err := os.Getwd() //nolint:forbidigo // shortcuts cannot import internal/vfs; cwd lookup is local-only and bounded. if err != nil { - return "", "", fmt.Errorf("cannot determine working directory: %w", err) + return "", "", errs.NewInternalError(errs.SubtypeUnknown, "cannot determine working directory: %v", err).WithCause(err) } projectPath = cwd } @@ -137,13 +137,13 @@ func checkEnvPullTarget(envFile string) error { if os.IsNotExist(err) { return nil } - return &errs.ValidationError{Problem: errs.Problem{Category: errs.CategoryValidation, Subtype: errs.SubtypeInvalidArgument, Message: fmt.Sprintf("cannot inspect %s: %v", envFile, err)}, Param: "project-path", Cause: err} + return appsValidationParamError("--project-path", "cannot inspect %s: %v", envFile, err).WithCause(err) } if info.Mode()&os.ModeSymlink != 0 { - return &errs.ValidationError{Problem: errs.Problem{Category: errs.CategoryValidation, Subtype: errs.SubtypeInvalidArgument, Message: fmt.Sprintf("target %s must be a regular file, not a symlink", envFile)}, Param: "project-path"} + return appsValidationParamError("--project-path", "target %s must be a regular file, not a symlink", envFile) } if !info.Mode().IsRegular() { - return &errs.ValidationError{Problem: errs.Problem{Category: errs.CategoryValidation, Subtype: errs.SubtypeInvalidArgument, Message: fmt.Sprintf("target %s must be a regular file", envFile)}, Param: "project-path"} + return appsValidationParamError("--project-path", "target %s must be a regular file", envFile) } return nil } @@ -156,7 +156,7 @@ func extractEnvPullVars(data map[string]interface{}) (map[string]string, envPull } } if raw == nil { - return nil, envPullDatabaseInfo{}, nil, &errs.ValidationError{Problem: errs.Problem{Category: errs.CategoryValidation, Subtype: errs.SubtypeInvalidResponse, Message: "response field env_vars must be an object or array of key/value entries"}} + return nil, envPullDatabaseInfo{}, nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "response field env_vars must be an object or array of key/value entries") } var skippedKeys []string @@ -203,7 +203,7 @@ func extractEnvPullVars(data map[string]interface{}) (map[string]string, envPull } return out, info, skippedKeys, nil default: - return nil, envPullDatabaseInfo{}, nil, &errs.ValidationError{Problem: errs.Problem{Category: errs.CategoryValidation, Subtype: errs.SubtypeInvalidResponse, Message: "response field env_vars must be an object or array of key/value entries"}} + return nil, envPullDatabaseInfo{}, nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "response field env_vars must be an object or array of key/value entries") } } diff --git a/shortcuts/apps/apps_env_pull_test.go b/shortcuts/apps/apps_env_pull_test.go index f80a80a4..1e9b3424 100644 --- a/shortcuts/apps/apps_env_pull_test.go +++ b/shortcuts/apps/apps_env_pull_test.go @@ -1079,3 +1079,28 @@ func TestEnsureEnvPullParentDir_MkdirError(t *testing.T) { t.Error("MkdirAll over a file component must surface an error") } } + +// TestExtractEnvPullVars_MissingEnvVarsIsInternalInvalidResponse pins that a +// response without a usable env_vars field classifies as +// internal/invalid_response — a broken upstream payload, not a flag problem +// the agent should retry with different arguments. +func TestExtractEnvPullVars_MissingEnvVarsIsInternalInvalidResponse(t *testing.T) { + for name, data := range map[string]map[string]interface{}{ + "missing": {}, + "wrong type": {"env_vars": "not-an-object"}, + } { + t.Run(name, func(t *testing.T) { + _, _, _, err := extractEnvPullVars(data) + if err == nil { + t.Fatalf("expected error for %s env_vars", name) + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed problem, got %T: %v", err, err) + } + if p.Category != errs.CategoryInternal || p.Subtype != errs.SubtypeInvalidResponse { + t.Fatalf("classification = %s/%s, want internal/invalid_response", p.Category, p.Subtype) + } + }) + } +} diff --git a/shortcuts/apps/apps_errors.go b/shortcuts/apps/apps_errors.go new file mode 100644 index 00000000..9d0653cb --- /dev/null +++ b/shortcuts/apps/apps_errors.go @@ -0,0 +1,104 @@ +// 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 appsFailedPreconditionError(format string, args ...any) *errs.ValidationError { + return errs.NewValidationError(errs.SubtypeFailedPrecondition, format, args...) +} + +// appsStorageError classifies a local credential/state storage failure +// (read, write, delete, list) as internal/storage. +func appsStorageError(err error, format string, args ...any) *errs.InternalError { + return errs.NewInternalError(errs.SubtypeStorage, format, args...).WithCause(err) +} + +// appsExternalToolError classifies a runtime failure of an external tool the +// CLI shells out to (git, npx) as internal/external_tool. The tool output is +// carried in the message; recovery guidance goes in the hint. +func appsExternalToolError(err error, format string, args ...any) *errs.InternalError { + return errs.NewInternalError(errs.SubtypeExternalTool, format, args...).WithCause(err) +} + +// appsSubprocessEnvelopeError classifies a malformed or failed envelope from a +// lark-cli subprocess (+git-credential-init / +env-pull) as internal/invalid_response. +func appsSubprocessEnvelopeError(format string, args ...any) *errs.InternalError { + return errs.NewInternalError(errs.SubtypeInvalidResponse, format, args...) +} + +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 00000000..ccab1d7c --- /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 6c6f58f9..18dd6d07 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" ) @@ -35,11 +35,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 @@ -150,9 +150,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 约束。 @@ -178,15 +178,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 @@ -196,24 +195,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 1d8e942f..788e1c06 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) + problem := requireAppsValidationProblem(t, err) + if !strings.Contains(problem.Message, "index.html") { + t.Fatalf("message missing 'index.html': %v", problem.Message) } - if exitErr.Detail.Type != "validation" { - t.Fatalf("error.type = %q, want validation", exitErr.Detail.Type) - } - 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) + problem := requireAppsValidationProblem(t, err) + if !strings.Contains(problem.Message, ".env") { + t.Fatalf("error message should list the offending file, got %q", problem.Message) } - 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) - } - 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_init.go b/shortcuts/apps/apps_init.go index a20b8184..1d7eb94a 100644 --- a/shortcuts/apps/apps_init.go +++ b/shortcuts/apps/apps_init.go @@ -14,8 +14,8 @@ import ( "strings" "unicode" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/charcheck" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -72,14 +72,14 @@ var AppsInit = common.Shortcut{ // exit-1 (root.go handleRootError case 4), bypassing the structured // envelope. The spec and the E2E assert exit-2 + a structured // {"ok":false,"error":{...}} envelope for missing --app-id, so the empty - // check lives in Validate (output.ErrValidation -> ExitValidation=2). + // check lives in Validate (typed validation error -> exit 2). {Name: "app-id", Desc: "Miaoda app ID"}, {Name: "dir", Desc: "clone target directory; absolute or relative path (default ./)"}, {Name: "template", Desc: "code-init template for an empty repo; optional — if omitted, derived from the app's tech stack"}, }, 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 }, @@ -152,14 +152,14 @@ func resolveTargetPath(rctx *common.RuntimeContext, appID string) (string, error // path is a log-injection vector); charcheck additionally rejects dangerous // Unicode (bidi overrides, zero-width) that IsControl does not. if strings.IndexFunc(raw, unicode.IsControl) >= 0 { - return "", output.ErrValidation("--dir must not contain control characters") + return "", appsValidationParamError("--dir", "--dir must not contain control characters") } if err := charcheck.RejectControlChars(raw, "--dir"); err != nil { - return "", output.ErrValidation("%v", err) + return "", appsValidationParamError("--dir", "%v", err).WithCause(err) } abs, err := filepath.Abs(raw) //nolint:forbidigo // shortcuts cannot import internal/vfs (depguard rule shortcuts-no-vfs); raw is control-char-validated above, and FileIO.ResolvePath cannot resolve a clone target (it rejects absolute paths). if err != nil { - return "", output.ErrValidation("--dir cannot be resolved: %v", err) + return "", appsValidationParamError("--dir", "--dir cannot be resolved: %v", err) } return abs, nil } @@ -173,20 +173,20 @@ func ensureEmptyDir(dir string) error { return nil } if err != nil { - return output.ErrValidation("--dir cannot be read: %v", err) + return appsValidationParamError("--dir", "--dir cannot be read: %v", err) } if info.Mode()&os.ModeSymlink != 0 { - return output.ErrValidation("--dir must not be a symlink: %q", dir) + return appsValidationParamError("--dir", "--dir must not be a symlink: %q", dir) } if !info.IsDir() { - return output.ErrValidation("--dir exists and is not a directory: %q", dir) + return appsValidationParamError("--dir", "--dir exists and is not a directory: %q", dir) } entries, err := os.ReadDir(dir) //nolint:forbidigo // shortcuts cannot import internal/vfs (depguard rule shortcuts-no-vfs); dir is the validated clone target, and FileIO has no ReadDir. if err != nil { - return output.ErrValidation("--dir cannot be read: %v", err) + return appsValidationParamError("--dir", "--dir cannot be read: %v", err) } if len(entries) > 0 { - return output.ErrValidation("target directory %q already exists and is not empty", dir) + return appsValidationParamError("--dir", "target directory %q already exists and is not empty", dir) } return nil } @@ -209,11 +209,11 @@ func ensureMetaAppID(dir, appID string) error { return nil } if err != nil { - return output.Errorf(output.ExitAPI, "meta_write", "read %s failed: %v", metaRelPath, err) + return appsFileIOError(err, "read %s failed: %v", metaRelPath, err) } var m map[string]interface{} if err := json.Unmarshal(b, &m); err != nil { - return output.Errorf(output.ExitAPI, "meta_write", "parse %s failed: %v", metaRelPath, err) + return appsFileIOError(err, "parse %s failed: %v", metaRelPath, err) } if cur, _ := m["app_id"].(string); strings.TrimSpace(cur) != "" { return nil @@ -224,10 +224,10 @@ func ensureMetaAppID(dir, appID string) error { m["app_id"] = appID out, err := json.MarshalIndent(m, "", " ") if err != nil { - return output.Errorf(output.ExitAPI, "meta_write", "marshal %s failed: %v", metaRelPath, err) + return appsFileIOError(err, "marshal %s failed: %v", metaRelPath, err) } if err := os.WriteFile(path, append(out, '\n'), 0o644); err != nil { //nolint:forbidigo // shortcuts cannot import internal/vfs (depguard rule shortcuts-no-vfs); path is under the validated clone dir, and FileIO.Save rejects absolute paths. - return output.Errorf(output.ExitAPI, "meta_write", "write %s failed: %v", metaRelPath, err) + return appsFileIOError(err, "write %s failed: %v", metaRelPath, err) } return nil } @@ -244,7 +244,7 @@ func hasSteeringSkills(dir string) bool { func isEmptyRepo(ctx context.Context, dir string) (bool, error) { stdout, stderr, err := initRunner.Run(ctx, dir, "git", "ls-files") if err != nil { - return false, output.Errorf(output.ExitAPI, "git_ls_files", "git ls-files failed: %s", gitErr(stderr, err)) + return false, appsExternalToolError(err, "git ls-files failed: %s", gitErr(stderr, err)) } for _, line := range strings.Split(strings.TrimSpace(stdout), "\n") { f := strings.TrimSpace(line) @@ -274,19 +274,19 @@ func runScaffold(ctx context.Context, dir, appID, template string) (string, erro // seed README.md — as empty. If other seed files (e.g. .gitignore) can // appear, extend isEmptyRepo's allow-list accordingly. if _, stderr, err := initRunner.Run(ctx, dir, "npx", "-y", "--prefer-online", miaodaCLIPkg, "app", "init", "--template", template, "--app-id", appID); err != nil { - return "", output.Errorf(output.ExitAPI, "npx_app_init", "npx app init failed: %s", gitErr(stderr, err)) + return "", appsExternalToolError(err, "npx app init failed: %s", gitErr(stderr, err)) } return scaffoldKindInit, nil } if _, stderr, err := initRunner.Run(ctx, dir, "npx", "-y", "--prefer-online", miaodaCLIPkg, "app", "sync"); err != nil { - return "", output.Errorf(output.ExitAPI, "npx_app_sync", "npx app sync failed: %s", gitErr(stderr, err)) + return "", appsExternalToolError(err, "npx app sync failed: %s", gitErr(stderr, err)) } if err := ensureMetaAppID(dir, appID); err != nil { return "", err } if !hasSteeringSkills(dir) { if _, stderr, err := initRunner.Run(ctx, dir, "npx", "-y", "--prefer-online", miaodaCLIPkg, "skills", "sync", "--local"); err != nil { - return "", output.Errorf(output.ExitAPI, "npx_skills_sync", "npx skills sync failed: %s", gitErr(stderr, err)) + return "", appsExternalToolError(err, "npx skills sync failed: %s", gitErr(stderr, err)) } } return scaffoldKindUpgrade, nil @@ -303,13 +303,13 @@ func parseRepoURLFromEnvelope(stdout string) (string, error) { } `json:"data"` } if err := json.Unmarshal([]byte(stdout), &env); err != nil { - return "", output.Errorf(output.ExitInternal, "credential_init", "could not parse +git-credential-init output as JSON: %v", err) + return "", appsSubprocessEnvelopeError("could not parse +git-credential-init output as JSON: %v", err) } if !env.OK { - return "", output.Errorf(output.ExitInternal, "credential_init", "+git-credential-init reported failure") + return "", appsSubprocessEnvelopeError("+git-credential-init reported failure") } if strings.TrimSpace(env.Data.RepositoryURL) == "" { - return "", output.Errorf(output.ExitInternal, "credential_init", "+git-credential-init returned no repository_url") + return "", appsSubprocessEnvelopeError("+git-credential-init returned no repository_url") } return env.Data.RepositoryURL, nil } @@ -324,13 +324,13 @@ func parseEnvFileFromEnvelope(stdout string) (string, error) { } `json:"data"` } if err := json.Unmarshal([]byte(stdout), &env); err != nil { - return "", output.Errorf(output.ExitInternal, "env_pull", "could not parse +env-pull output as JSON: %v", err) + return "", appsSubprocessEnvelopeError("could not parse +env-pull output as JSON: %v", err) } if !env.OK { - return "", output.Errorf(output.ExitInternal, "env_pull", "+env-pull reported failure") + return "", appsSubprocessEnvelopeError("+env-pull reported failure") } if strings.TrimSpace(env.Data.EnvFile) == "" { - return "", output.Errorf(output.ExitInternal, "env_pull", "+env-pull returned no env_file") + return "", appsSubprocessEnvelopeError("+env-pull returned no env_file") } return env.Data.EnvFile, nil } @@ -364,7 +364,9 @@ func validateRepoURLScheme(repoURL string) error { if strings.HasPrefix(repoURL, "http://") || strings.HasPrefix(repoURL, "https://") { return nil } - return output.Errorf(output.ExitValidation, "validation", + // The URL comes from the +git-credential-init subprocess response, not user + // input, so a non-http(s) scheme is a broken upstream contract. + return appsSubprocessEnvelopeError( "repository_url from +git-credential-init must be http(s); refusing %q", redactURLCredentials(repoURL)) } @@ -415,12 +417,12 @@ func appsInitExecute(ctx context.Context, rctx *common.RuntimeContext) error { } if _, err := exec.LookPath("git"); err != nil { - return output.ErrWithHint(output.ExitInternal, "dependency", - "git executable not found on PATH", "install git and ensure it is on your PATH") + return appsFailedPreconditionError("git executable not found on PATH"). + WithHint("install git and ensure it is on your PATH") } if _, err := exec.LookPath("npx"); err != nil { - return output.ErrWithHint(output.ExitInternal, "dependency", - "npx executable not found on PATH", "install Node.js (which provides npx) and ensure it is on your PATH") + return appsFailedPreconditionError("npx executable not found on PATH"). + WithHint("install Node.js (which provides npx) and ensure it is on your PATH") } if err := ensureEmptyDir(dir); err != nil { @@ -438,11 +440,11 @@ func appsInitExecute(ctx context.Context, rctx *common.RuntimeContext) error { initLogf(rctx, "Cloning into %s...", dir) if _, stderr, err := initRunner.Run(ctx, "", "git", "clone", "--", repoURL, dir); err != nil { - return output.Errorf(output.ExitAPI, "git_clone", "git clone failed: %s", gitErr(stderr, err)) + return appsExternalToolError(err, "git clone failed: %s", gitErr(stderr, err)) } initLogf(rctx, "Checking out %s...", defaultInitBranch) if _, stderr, err := initRunner.Run(ctx, dir, "git", "checkout", defaultInitBranch); err != nil { - return output.Errorf(output.ExitAPI, "git_checkout", "git checkout %s failed: %s", defaultInitBranch, gitErr(stderr, err)) + return appsExternalToolError(err, "git checkout %s failed: %s", defaultInitBranch, gitErr(stderr, err)) } initLogf(rctx, "Initializing app code (running miaoda-cli)...") @@ -536,7 +538,7 @@ func pullEnv(ctx context.Context, rctx *common.RuntimeContext, appID, dir string func issueCredentials(ctx context.Context, rctx *common.RuntimeContext, appID string) (string, error) { self, err := os.Executable() if err != nil { - return "", output.Errorf(output.ExitInternal, "internal", "cannot locate lark-cli executable: %v", err) + return "", errs.NewInternalError(errs.SubtypeUnknown, "cannot locate lark-cli executable: %v", err).WithCause(err) } args := []string{"apps", "+git-credential-init", "--app-id", appID, "--format", "json"} if as := strings.TrimSpace(rctx.Str("as")); as != "" { @@ -544,9 +546,9 @@ func issueCredentials(ctx context.Context, rctx *common.RuntimeContext, appID st } stdout, stderr, err := initRunner.Run(ctx, "", self, args...) if err != nil { - return "", output.ErrWithHint(output.ExitAPI, "credential_init", - fmt.Sprintf("apps +git-credential-init failed: %s", gitErr(stderr, err)), - "ensure apps +git-credential-init is available and you are logged in") + return "", appsExternalToolError(err, "apps +git-credential-init failed: %s", gitErr(stderr, err)). + WithHint("ensure apps +git-credential-init is available and you are logged in"). + WithCause(err) } return parseRepoURLFromEnvelope(stdout) } @@ -560,7 +562,7 @@ func issueCredentials(ctx context.Context, rctx *common.RuntimeContext, appID st func commitAndPushIfDirty(ctx context.Context, dir, scaffoldKind string) (committed, pushed bool, err error) { status, stderr, runErr := initRunner.Run(ctx, dir, "git", "status", "--porcelain") if runErr != nil { - return false, false, output.Errorf(output.ExitAPI, "git_status", "git status failed: %s", gitErr(stderr, runErr)) + return false, false, appsExternalToolError(runErr, "git status failed: %s", gitErr(stderr, runErr)) } if strings.TrimSpace(status) == "" { return false, false, nil @@ -595,7 +597,7 @@ func commitAndPushIfDirty(ctx context.Context, dir, scaffoldKind string) (commit if _, se, e := initRunner.Run(ctx, dir, "git", "push", "origin", defaultInitBranch); e != nil { return true, false, withAppsHint( - output.Errorf(output.ExitAPI, "git_push", "git push failed: %s", gitErr(se, e)), + appsExternalToolError(e, "git push failed: %s", gitErr(se, e)), "the push was rejected — the git output is in the message above; if it is a non-fast-forward (remote has new commits), sync the remote and retry; if it is an auth failure, make sure `lark-cli apps +git-credential-init` has succeeded") } return true, true, nil @@ -609,10 +611,10 @@ func commitAndPushIfDirty(ctx context.Context, dir, scaffoldKind string) (commit func stageAndCommit(ctx context.Context, dir, message string, pathspecs ...string) error { addArgs := append([]string{"add", "-A", "--"}, pathspecs...) if _, se, e := initRunner.Run(ctx, dir, "git", addArgs...); e != nil { - return output.Errorf(output.ExitAPI, "git_add", "git add failed: %s", gitErr(se, e)) + return appsExternalToolError(e, "git add failed: %s", gitErr(se, e)) } if _, se, e := initRunner.Run(ctx, dir, "git", "commit", "--no-verify", "-m", message); e != nil { - return output.Errorf(output.ExitAPI, "git_commit", "git commit failed: %s", gitErr(se, e)) + return appsExternalToolError(e, "git commit failed: %s", gitErr(se, e)) } return nil } diff --git a/shortcuts/apps/apps_init_test.go b/shortcuts/apps/apps_init_test.go index f8415d61..7eac1369 100644 --- a/shortcuts/apps/apps_init_test.go +++ b/shortcuts/apps/apps_init_test.go @@ -17,6 +17,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/shortcuts/common" @@ -1466,3 +1467,28 @@ func TestAppsInit_Description_IsAboutCode(t *testing.T) { t.Errorf("Description should mention app code: %q", AppsInit.Description) } } + +// TestRunScaffold_SubprocessFailureIsExternalTool pins the typed +// classification of an external-tool failure: a failing git subprocess +// surfaces as internal/external_tool with the cause preserved. +func TestRunScaffold_SubprocessFailureIsExternalTool(t *testing.T) { + cause := errors.New("exit status 128") + f := &fakeCommandRunner{results: map[string]fakeCallResult{ + "git ls-files": {stderr: "fatal: not a git repository", err: cause}, + }} + withFakeRunner(t, f) + _, err := runScaffold(context.Background(), t.TempDir(), "app_x", "nestjs-react-fullstack") + if err == nil { + t.Fatalf("expected error from failing git subprocess") + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed problem, got %T: %v", err, err) + } + if p.Category != errs.CategoryInternal || p.Subtype != errs.SubtypeExternalTool { + t.Fatalf("classification = %s/%s, want internal/external_tool", p.Category, p.Subtype) + } + if !errors.Is(err, cause) { + t.Fatalf("cause chain not preserved: %v", err) + } +} diff --git a/shortcuts/apps/apps_release_create.go b/shortcuts/apps/apps_release_create.go index 30654430..ada111eb 100644 --- a/shortcuts/apps/apps_release_create.go +++ b/shortcuts/apps/apps_release_create.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" ) @@ -33,7 +32,7 @@ var AppsReleaseCreate = 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 }, diff --git a/shortcuts/apps/apps_release_get.go b/shortcuts/apps/apps_release_get.go index 4e7ac914..3ed84053 100644 --- a/shortcuts/apps/apps_release_get.go +++ b/shortcuts/apps/apps_release_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" ) @@ -32,10 +31,10 @@ var AppsReleaseGet = 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") } if strings.TrimSpace(rctx.Str("release-id")) == "" { - return output.ErrValidation("--release-id is required") + return appsValidationParamError("--release-id", "--release-id is required") } return nil }, diff --git a/shortcuts/apps/apps_release_list.go b/shortcuts/apps/apps_release_list.go index 8cc53630..761c4272 100644 --- a/shortcuts/apps/apps_release_list.go +++ b/shortcuts/apps/apps_release_list.go @@ -35,7 +35,7 @@ var AppsReleaseList = 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 }, diff --git a/shortcuts/apps/apps_session_create.go b/shortcuts/apps/apps_session_create.go index 48eace6a..e5e93d45 100644 --- a/shortcuts/apps/apps_session_create.go +++ b/shortcuts/apps/apps_session_create.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" ) @@ -31,7 +30,7 @@ var AppsSessionCreate = 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 }, diff --git a/shortcuts/apps/apps_session_get.go b/shortcuts/apps/apps_session_get.go index 7f4a642d..b8c9bc8f 100644 --- a/shortcuts/apps/apps_session_get.go +++ b/shortcuts/apps/apps_session_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" ) @@ -33,10 +32,10 @@ var AppsSessionGet = 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") } if strings.TrimSpace(rctx.Str("session-id")) == "" { - return output.ErrValidation("--session-id is required") + return appsValidationParamError("--session-id", "--session-id is required") } return nil }, diff --git a/shortcuts/apps/apps_session_list.go b/shortcuts/apps/apps_session_list.go index 310baac8..072da206 100644 --- a/shortcuts/apps/apps_session_list.go +++ b/shortcuts/apps/apps_session_list.go @@ -32,7 +32,7 @@ var AppsSessionList = 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 }, diff --git a/shortcuts/apps/apps_session_stop.go b/shortcuts/apps/apps_session_stop.go index 449328a0..028480b6 100644 --- a/shortcuts/apps/apps_session_stop.go +++ b/shortcuts/apps/apps_session_stop.go @@ -9,7 +9,6 @@ import ( "io" "strings" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/common" ) @@ -35,13 +34,13 @@ var AppsSessionStop = 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") } if strings.TrimSpace(rctx.Str("session-id")) == "" { - return output.ErrValidation("--session-id is required") + return appsValidationParamError("--session-id", "--session-id is required") } if strings.TrimSpace(rctx.Str("turn-id")) == "" { - return output.ErrValidation("--turn-id is required") + return appsValidationParamError("--turn-id", "--turn-id is required") } return nil }, diff --git a/shortcuts/apps/apps_update.go b/shortcuts/apps/apps_update.go index 3b4e0e39..845d5e9f 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" ) @@ -34,11 +33,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 }, diff --git a/shortcuts/apps/common.go b/shortcuts/apps/common.go index de2e7cee..3b0c9e2b 100644 --- a/shortcuts/apps/common.go +++ b/shortcuts/apps/common.go @@ -4,11 +4,9 @@ package apps import ( - "errors" "strings" "github.com/larksuite/cli/errs" - "github.com/larksuite/cli/internal/output" ) // appsService 是 CLI 命令的 service 前缀（lark-cli apps ...）。 @@ -23,11 +21,11 @@ const apiBasePath = "/open-apis/spark/v1" // lark-apps SKILL.md ("app_id 获取"); the hint stays lean and does not repeat it. const appIDListHint = "verify --app-id is correct and you have access to the app; list your apps with `lark-cli apps +list`" -// withAppsHint attaches an actionable next-step hint to a failure returned by -// CallAPI, preserving its original classification (typed subtype/code/log_id or -// legacy detail). A hint already present on the error is kept (the upstream -// wording wins); only an empty hint is filled in. Mirrors -// drive.appendDriveExportRecoveryHint. err==nil passes through. +// withAppsHint attaches an actionable next-step hint to a typed failure, +// preserving its original classification (subtype/code/log_id). A hint already +// present on the error is kept (the upstream wording wins); only an empty hint +// is filled in. Mirrors drive.appendDriveExportRecoveryHint. err==nil and +// untyped errors pass through unchanged. func withAppsHint(err error, hint string) error { if err == nil { return nil @@ -39,14 +37,5 @@ func withAppsHint(err error, hint string) error { } return err } - // Legacy *output.ExitError fallback: fill the hint in place, preserving the - // original class / exit code rather than downgrading the error. - var exitErr *output.ExitError - if errors.As(err, &exitErr) && exitErr.Detail != nil { - if strings.TrimSpace(exitErr.Detail.Hint) == "" { - exitErr.Detail.Hint = hint - } - return err - } return err } diff --git a/shortcuts/apps/common_test.go b/shortcuts/apps/common_test.go index 092874e9..8060d1da 100644 --- a/shortcuts/apps/common_test.go +++ b/shortcuts/apps/common_test.go @@ -7,7 +7,7 @@ import ( "errors" "testing" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" ) func TestWithAppsHint(t *testing.T) { @@ -17,46 +17,40 @@ func TestWithAppsHint(t *testing.T) { } }) - t.Run("empty hint gets filled, code/type preserved", func(t *testing.T) { - in := &output.ExitError{Code: 1, Detail: &output.ErrDetail{Type: "api_error", Message: "boom"}} + t.Run("empty hint gets filled, classification preserved", func(t *testing.T) { + in := errs.NewAPIError(errs.SubtypeNotFound, "boom").WithCode(404) out := withAppsHint(in, "run +release-list") - var exitErr *output.ExitError - if !errors.As(out, &exitErr) { - t.Fatalf("returned error is not *output.ExitError: %T", out) + p, ok := errs.ProblemOf(out) + if !ok { + t.Fatalf("returned error is not typed: %T", out) } - if exitErr.Detail.Hint != "run +release-list" { - t.Errorf("Hint = %q, want %q", exitErr.Detail.Hint, "run +release-list") + if p.Hint != "run +release-list" { + t.Errorf("Hint = %q, want %q", p.Hint, "run +release-list") } - if exitErr.Code != 1 || exitErr.Detail.Type != "api_error" || exitErr.Detail.Message != "boom" { - t.Errorf("code/type/message mutated: code=%d type=%q msg=%q", exitErr.Code, exitErr.Detail.Type, exitErr.Detail.Message) + if p.Subtype != errs.SubtypeNotFound || p.Code != 404 || p.Message != "boom" { + t.Errorf("subtype/code/message mutated: subtype=%q code=%d msg=%q", p.Subtype, p.Code, p.Message) } }) t.Run("existing hint is preserved, not clobbered", func(t *testing.T) { - in := output.ErrWithHint(1, "api_error", "boom", "original hint") + in := errs.NewAPIError(errs.SubtypeUnknown, "boom").WithHint("original hint") out := withAppsHint(in, "new hint") - var exitErr *output.ExitError - if !errors.As(out, &exitErr) { - t.Fatalf("returned error is not *output.ExitError: %T", out) - } - if exitErr.Detail.Hint != "original hint" { - t.Errorf("Hint = %q, want preserved %q", exitErr.Detail.Hint, "original hint") + p, _ := errs.ProblemOf(out) + if p.Hint != "original hint" { + t.Errorf("Hint = %q, want preserved %q", p.Hint, "original hint") } }) t.Run("blank-whitespace hint is treated as empty and filled", func(t *testing.T) { - in := output.ErrWithHint(1, "api_error", "boom", " ") + in := errs.NewAPIError(errs.SubtypeUnknown, "boom").WithHint(" ") out := withAppsHint(in, "filled hint") - var exitErr *output.ExitError - if !errors.As(out, &exitErr) { - t.Fatalf("returned error is not *output.ExitError: %T", out) - } - if exitErr.Detail.Hint != "filled hint" { - t.Errorf("Hint = %q, want %q", exitErr.Detail.Hint, "filled hint") + p, _ := errs.ProblemOf(out) + if p.Hint != "filled hint" { + t.Errorf("Hint = %q, want %q", p.Hint, "filled hint") } }) - t.Run("unrecognized error type returned unchanged, no panic", func(t *testing.T) { + t.Run("untyped error returned unchanged, no panic", func(t *testing.T) { in := errors.New("plain") out := withAppsHint(in, "ignored") if out == nil || out.Error() != "plain" { diff --git a/shortcuts/apps/db_common.go b/shortcuts/apps/db_common.go index 9f039139..9223e46e 100644 --- a/shortcuts/apps/db_common.go +++ b/shortcuts/apps/db_common.go @@ -7,7 +7,6 @@ import ( "fmt" "strings" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" ) @@ -37,7 +36,7 @@ func appDbEnvCreatePath(appID string) string { func requireAppID(raw string) (string, error) { id := strings.TrimSpace(raw) if id == "" { - return "", output.ErrValidation("--app-id is required") + return "", appsValidationParamError("--app-id", "--app-id is required") } return id, nil } diff --git a/shortcuts/apps/git_credential.go b/shortcuts/apps/git_credential.go index 2e1789eb..94265d64 100644 --- a/shortcuts/apps/git_credential.go +++ b/shortcuts/apps/git_credential.go @@ -6,7 +6,6 @@ package apps import ( "context" "encoding/json" - "errors" "fmt" "io" "net/http" @@ -22,10 +21,11 @@ import ( "github.com/spf13/cobra" "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/client" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/errclass" "github.com/larksuite/cli/internal/keychain" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/shortcuts/apps/gitcred" "github.com/larksuite/cli/shortcuts/common" @@ -53,9 +53,12 @@ var AppsGitCredentialInit = 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 validate.ResourceName(strings.TrimSpace(rctx.Str("app-id")), "--app-id") + if err := validate.ResourceName(strings.TrimSpace(rctx.Str("app-id")), "--app-id"); err != nil { + return appsValidationParamError("--app-id", "%v", err).WithCause(err) + } + return nil }, DryRun: func(ctx context.Context, rctx *common.RuntimeContext) *common.DryRunAPI { appID := strings.TrimSpace(rctx.Str("app-id")) @@ -129,9 +132,12 @@ var AppsGitCredentialRemove = 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 validate.ResourceName(strings.TrimSpace(rctx.Str("app-id")), "--app-id") + if err := validate.ResourceName(strings.TrimSpace(rctx.Str("app-id")), "--app-id"); err != nil { + return appsValidationParamError("--app-id", "%v", err).WithCause(err) + } + return nil }, DryRun: func(ctx context.Context, rctx *common.RuntimeContext) *common.DryRunAPI { appID := strings.TrimSpace(rctx.Str("app-id")) @@ -268,7 +274,7 @@ func (i runtimeIssuer) Issue(ctx context.Context, appID string, profile gitcred. HttpMethod: http.MethodGet, ApiPath: issuePath(appID), }) - data, err := parseIssueCredentialData(resp, err) + data, err := parseIssueCredentialData(resp, err, i.rctx.APIClassifyContext()) if err != nil { return nil, err } @@ -285,7 +291,8 @@ func (i factoryIssuer) Issue(ctx context.Context, appID string, profile gitcred. return nil, err } if cfg.UserOpenId == "" { - return nil, output.ErrAuth("not logged in: run `lark-cli auth login --scope \"spark:app:read\"`") + return nil, errs.NewAuthenticationError(errs.SubtypeTokenMissing, "not logged in"). + WithHint("run `lark-cli auth login --scope \"spark:app:read\"`") } ac, err := i.f.NewAPIClientWithConfig(cfg) if err != nil { @@ -296,7 +303,11 @@ func (i factoryIssuer) Issue(ctx context.Context, appID string, profile gitcred. ApiPath: issuePath(appID), } resp, err := ac.DoSDKRequest(ctx, req, core.AsUser) - data, err := parseIssueCredentialData(resp, err) + data, err := parseIssueCredentialData(resp, err, errclass.ClassifyContext{ + Brand: string(cfg.Brand), + AppID: cfg.AppID, + Identity: string(core.AsUser), + }) if err != nil { return nil, err } @@ -414,13 +425,11 @@ func gitCredentialLocalError(action string, err error) error { if err == nil { return nil } + // Typed errors pass through unchanged; everything the apps domain and the + // shared runtime produce is typed, so there is no legacy envelope to forward. if _, ok := errs.UnwrapTypedError(err); ok { return err } - var exitErr *output.ExitError - if errors.As(err, &exitErr) { - return err - } return &errs.ConfigError{Problem: errs.Problem{ Category: errs.CategoryConfig, Subtype: errs.SubtypeInvalidConfig, @@ -448,64 +457,43 @@ func issuedFromData(appID string, data map[string]interface{}) (*gitcred.IssuedC issued.AppID = appID } if issued.GitHTTPURL == "" { - return nil, output.Errorf(output.ExitAPI, "api_error", "Issue Miaoda Git credential: response missing gitURL") + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "Issue Miaoda Git credential: response missing gitURL") } if issued.PAT == "" { - return nil, output.Errorf(output.ExitAPI, "api_error", "Issue Miaoda Git credential: response missing token") + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, "Issue Miaoda Git credential: response missing token") } return issued, nil } -func parseIssueCredentialData(resp *larkcore.ApiResp, err error) (map[string]any, error) { +// parseIssueCredentialData turns the git-credential issue response into the +// credential data map. A standard Lark envelope (top-level "code") and any +// HTTP error status route through the shared response classifier, so generic +// codes (missing scope, app not authorized) and 5xx statuses keep their +// canonical category/subtype/retryable classification. The endpoint's +// non-standard success shapes — direct git info or a BaseResp wrapper — are +// handled locally. +func parseIssueCredentialData(resp *larkcore.ApiResp, err error, cc errclass.ClassifyContext) (map[string]any, error) { if err != nil { - return nil, err + return nil, client.WrapDoAPIError(err) } detail := logIDDetail(resp) if resp == nil || len(resp.RawBody) == 0 { - return nil, &errs.InternalError{Problem: errs.Problem{ - Category: errs.CategoryInternal, - Subtype: errs.SubtypeUnknown, - Message: "Issue Miaoda Git credential: empty response body", - }} + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, + "Issue Miaoda Git credential: empty response body") } var result map[string]any - if jsonErr := json.Unmarshal(resp.RawBody, &result); jsonErr != nil { - return nil, &errs.InternalError{Problem: errs.Problem{ - Category: errs.CategoryInternal, - Subtype: errs.SubtypeUnknown, - Message: fmt.Sprintf("Issue Miaoda Git credential: unmarshal response: %s", jsonErr), - }, Cause: jsonErr} - } - if resp.StatusCode >= http.StatusBadRequest { - msg := firstString(result, "msg", "message") - if msg == "" { - msg = fmt.Sprintf("HTTP %d", resp.StatusCode) + jsonErr := json.Unmarshal(resp.RawBody, &result) + _, hasCode := result["code"] + if jsonErr != nil || hasCode || resp.StatusCode >= http.StatusBadRequest { + data, cerr := common.ClassifyAPIResponseWith(resp, cc) + if cerr != nil { + return nil, withAppsHint(cerr, gitCredentialIssueHint) } - return nil, &errs.APIError{Problem: errs.Problem{ - Category: errs.CategoryAPI, - Subtype: errs.SubtypeUnknown, - Code: resp.StatusCode, - Message: msg, - LogID: logIDString(resp), - Hint: gitCredentialIssueHint, - Retryable: resp.StatusCode >= http.StatusInternalServerError, - }} - } - if _, hasCode := result["code"]; hasCode { - code := firstInt64(result, "code") - if code != 0 { - return nil, &errs.APIError{Problem: errs.Problem{ - Category: errs.CategoryAPI, - Subtype: errs.SubtypeUnknown, - Code: int(code), - Message: firstString(result, "msg", "message"), - LogID: logIDString(resp), - Hint: gitCredentialIssueHint, - }} - } - if data, ok := result["data"].(map[string]any); ok { + if data != nil { result = data } + // data == nil: a code==0 envelope whose fields sit beside "code" instead + // of under "data" — keep the locally-unmarshalled top-level object. } else if err := checkGitInfoBaseResp(result, logIDString(resp)); err != nil { return nil, err } @@ -534,13 +522,11 @@ func checkGitInfoBaseResp(result map[string]any, logID string) error { if message == "" { message = "Git credential API returned non-zero BaseResp status" } - return &errs.APIError{Problem: errs.Problem{ - Category: errs.CategoryAPI, - Subtype: errs.SubtypeUnknown, - Code: int(code), - Message: "Issue Miaoda Git credential: " + message, - LogID: logID, - }} + baseErr := errs.NewAPIError(errs.SubtypeUnknown, "Issue Miaoda Git credential: %s", message).WithCode(int(code)) + if logID != "" { + baseErr = baseErr.WithLogID(logID) + } + return baseErr } return nil } @@ -578,6 +564,9 @@ func firstInt64(data map[string]interface{}, keys ...string) int64 { return int64(v) case float64: return int64(v) + case json.Number: + n, _ := v.Int64() + return n case string: n, _ := strconv.ParseInt(strings.TrimSpace(v), 10, 64) return n diff --git a/shortcuts/apps/git_credential_storage.go b/shortcuts/apps/git_credential_storage.go index 9b1c5dac..d76355c4 100644 --- a/shortcuts/apps/git_credential_storage.go +++ b/shortcuts/apps/git_credential_storage.go @@ -5,7 +5,6 @@ package apps import ( "errors" - "fmt" "net/url" "os" "path/filepath" @@ -35,7 +34,7 @@ func (gitCredentialAppStorage) ListAppIDs() ([]string, error) { if errors.Is(err, os.ErrNotExist) { return []string{}, nil } - return nil, fmt.Errorf("apps storage: read root: %w", err) + return nil, appsStorageError(err, "apps storage: read root: %v", err) } appIDs := make([]string, 0, len(entries)) for _, e := range entries { diff --git a/shortcuts/apps/git_credential_test.go b/shortcuts/apps/git_credential_test.go index 8aeab946..2ae5c062 100644 --- a/shortcuts/apps/git_credential_test.go +++ b/shortcuts/apps/git_credential_test.go @@ -25,8 +25,8 @@ import ( "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/errclass" "github.com/larksuite/cli/internal/httpmock" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/shortcuts/apps/gitcred" "github.com/larksuite/cli/shortcuts/common" ) @@ -213,7 +213,7 @@ func TestParseIssueCredentialDataAcceptsDirectBaseRespShape(t *testing.T) { "expiredTime":1780050600, "BaseResp":{"StatusCode":0,"StatusMessage":"ok"} }`), - }, nil) + }, nil, errclass.ClassifyContext{}) if err != nil { t.Fatalf("parseIssueCredentialData returned error: %v", err) } @@ -717,9 +717,9 @@ func TestGitCredentialLocalErrorWrapsOnlyPlainErrors(t *testing.T) { t.Fatalf("typed error was rewrapped: %#v", got) } - exitErr := output.ErrValidation("bad app") - if got := gitCredentialLocalError("action", exitErr); got != exitErr { - t.Fatalf("legacy output error was rewrapped: %#v", got) + validationErr := errs.NewValidationError(errs.SubtypeInvalidArgument, "bad app") + if got := gitCredentialLocalError("action", validationErr); got != error(validationErr) { + t.Fatalf("typed validation error was rewrapped: %#v", got) } if got := gitCredentialLocalError("action", nil); got != nil { @@ -925,43 +925,43 @@ func TestGitCredentialHelpersAndParsers(t *testing.T) { } func TestParseIssueCredentialDataErrors(t *testing.T) { - if _, err := parseIssueCredentialData(nil, errors.New("transport failed")); err == nil { + if _, err := parseIssueCredentialData(nil, errors.New("transport failed"), errclass.ClassifyContext{}); err == nil { t.Fatal("parseIssueCredentialData transport error returned nil") } - if _, err := parseIssueCredentialData(nil, nil); err == nil { + if _, err := parseIssueCredentialData(nil, nil, errclass.ClassifyContext{}); err == nil { t.Fatal("parseIssueCredentialData nil response returned nil") } - if _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusOK, RawBody: []byte("{bad json")}, nil); err == nil { + if _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusOK, RawBody: []byte("{bad json")}, nil, errclass.ClassifyContext{}); err == nil { t.Fatal("parseIssueCredentialData bad json returned nil") } header := http.Header{"X-Tt-Logid": []string{"log_x"}} - if _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusBadRequest, RawBody: []byte(`{"msg":"bad request"}`), Header: header}, nil); err == nil || !strings.Contains(err.Error(), "bad request") { + if _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusBadRequest, RawBody: []byte(`{"msg":"bad request"}`), Header: header}, nil, errclass.ClassifyContext{}); err == nil || !strings.Contains(err.Error(), "bad request") { t.Fatalf("HTTP error = %v", err) } - if _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusInternalServerError, RawBody: []byte(`{}`), Header: header}, nil); err == nil || !strings.Contains(err.Error(), "HTTP 500") { + if _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusInternalServerError, RawBody: []byte(`{}`), Header: header}, nil, errclass.ClassifyContext{}); err == nil || !strings.Contains(err.Error(), "HTTP 500") { t.Fatalf("HTTP fallback error = %v", err) } - if _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusOK, RawBody: []byte(`{"code":999,"msg":"failed"}`), Header: header}, nil); err == nil || !strings.Contains(err.Error(), "failed") { + if _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusOK, RawBody: []byte(`{"code":999,"msg":"failed"}`), Header: header}, nil, errclass.ClassifyContext{}); err == nil || !strings.Contains(err.Error(), "failed") { t.Fatalf("code error = %v", err) } - data, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusOK, RawBody: []byte(`{"code":0}`), Header: header}, nil) + data, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusOK, RawBody: []byte(`{"code":0}`), Header: header}, nil, errclass.ClassifyContext{}) if err != nil { t.Fatalf("code zero without data returned error: %v", err) } if data["log_id"] != "log_x" { t.Fatalf("log_id = %v", data["log_id"]) } - data, err = parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusOK, RawBody: []byte(`null`), Header: header}, nil) + data, err = parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusOK, RawBody: []byte(`null`), Header: header}, nil, errclass.ClassifyContext{}) if err != nil { t.Fatalf("null response with log id returned error: %v", err) } if data["log_id"] != "log_x" { t.Fatalf("null response log_id = %v", data["log_id"]) } - if _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusOK, RawBody: []byte(`{"BaseResp":{"StatusCode":7,"StatusMessage":"denied"}}`), Header: header}, nil); err == nil || !strings.Contains(err.Error(), "denied") { + if _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusOK, RawBody: []byte(`{"BaseResp":{"StatusCode":7,"StatusMessage":"denied"}}`), Header: header}, nil, errclass.ClassifyContext{}); err == nil || !strings.Contains(err.Error(), "denied") { t.Fatalf("BaseResp error = %v", err) } - if _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusOK, RawBody: []byte(`{"baseResp":{"statusCode":7}}`)}, nil); err == nil || !strings.Contains(err.Error(), "non-zero BaseResp") { + if _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusOK, RawBody: []byte(`{"baseResp":{"statusCode":7}}`)}, nil, errclass.ClassifyContext{}); err == nil || !strings.Contains(err.Error(), "non-zero BaseResp") { t.Fatalf("BaseResp fallback error = %v", err) } } @@ -970,7 +970,7 @@ func TestParseIssueCredentialDataErrors(t *testing.T) { // credential issuance failure is flagged retryable and carries the developer-access hint. func TestParseIssueCredentialData503IsRetryableWithHint(t *testing.T) { header := http.Header{"X-Tt-Logid": []string{"log_x"}} - _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusServiceUnavailable, RawBody: []byte(`{"msg":"upstream busy"}`), Header: header}, nil) + _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusServiceUnavailable, RawBody: []byte(`{"msg":"upstream busy"}`), Header: header}, nil, errclass.ClassifyContext{}) if err == nil { t.Fatal("expected 503 error, got nil") } @@ -990,7 +990,7 @@ func TestParseIssueCredentialData503IsRetryableWithHint(t *testing.T) { // non-zero business code (no HTTP status) carries the hint but is not retryable. func TestParseIssueCredentialDataBusinessCodeHasHintNotRetryable(t *testing.T) { header := http.Header{"X-Tt-Logid": []string{"log_x"}} - _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusOK, RawBody: []byte(`{"code":999,"msg":"no developer access"}`), Header: header}, nil) + _, err := parseIssueCredentialData(&larkcore.ApiResp{StatusCode: http.StatusOK, RawBody: []byte(`{"code":999,"msg":"no developer access"}`), Header: header}, nil, errclass.ClassifyContext{}) if err == nil { t.Fatal("expected business-code error, got nil") } @@ -1014,11 +1014,10 @@ func TestParseIssueCredentialDataBusinessCodeHasHintNotRetryable(t *testing.T) { // server msg and assert (a) Message equals that msg exactly, and (b) neither // Message nor Hint contains any token/secret-shaped string. // -// Note: server msg passthrough is the framework's responsibility; apps adds -// only a static hint. There is no msg redaction in this path (verbatim -// passthrough is the existing behavior), so this test does not assert a -// redaction that does not exist — it asserts that apps injects nothing -// sensitive of its own. +// Note: server msg passthrough is the shared classifier's responsibility; +// apps adds only a static hint. There is no msg redaction in this path, so +// this test does not assert a redaction that does not exist — it asserts +// that apps injects nothing sensitive of its own. func TestParseIssueCredentialDataMessageAddsNoExtraSecret(t *testing.T) { const serverMsg = "permission denied" header := http.Header{"X-Tt-Logid": []string{"log_x"}} @@ -1045,7 +1044,7 @@ func TestParseIssueCredentialDataMessageAddsNoExtraSecret(t *testing.T) { }, } { t.Run(tc.name, func(t *testing.T) { - _, err := parseIssueCredentialData(tc.resp, nil) + _, err := parseIssueCredentialData(tc.resp, nil, errclass.ClassifyContext{}) if err == nil { t.Fatal("expected an error, got nil") } @@ -1053,9 +1052,12 @@ func TestParseIssueCredentialDataMessageAddsNoExtraSecret(t *testing.T) { if !ok { t.Fatalf("expected typed errs.Problem, got %T: %v", err, err) } - // (a) The server msg is passed through verbatim. - if p.Message != serverMsg { - t.Fatalf("Message = %q, want server msg %q (verbatim passthrough)", p.Message, serverMsg) + // (a) The server msg survives into the message. The business-code + // path passes it through verbatim; the HTTP-status path reports + // "HTTP : " via the shared classifier, so assert + // containment rather than equality. + if !strings.Contains(p.Message, serverMsg) { + t.Fatalf("Message = %q, want it to contain server msg %q", p.Message, serverMsg) } // apps adds only the static hint — assert that exact static text, // proving apps injects no per-request secret into the hint either. @@ -1138,3 +1140,45 @@ exit 0 } t.Setenv("PATH", dir+string(os.PathListSeparator)+os.Getenv("PATH")) } + +// TestParseIssueCredentialData_SharedClassifierCoverage pins the canonical +// classifications the shared classifier provides on the credential-issue +// path: a generic missing-scope code becomes a typed permission error with +// the missing scopes extracted, and an HTTP 503 becomes a retryable +// network/server_error — neither collapses to api/unknown. +func TestParseIssueCredentialData_SharedClassifierCoverage(t *testing.T) { + header := http.Header{"X-Tt-Logid": []string{"log_x"}} + + t.Run("missing scope classifies as authorization with scopes", func(t *testing.T) { + body := `{"code":99991676,"msg":"token scope insufficient","error":{"permission_violations":[{"subject":"spark:app:read"}]}}` + _, err := parseIssueCredentialData(&larkcore.ApiResp{ + StatusCode: http.StatusOK, RawBody: []byte(body), Header: header, + }, nil, errclass.ClassifyContext{}) + var permErr *errs.PermissionError + if !errors.As(err, &permErr) { + t.Fatalf("want *errs.PermissionError, got %T: %v", err, err) + } + if permErr.Subtype != errs.SubtypeTokenScopeInsufficient { + t.Fatalf("subtype = %q, want %q", permErr.Subtype, errs.SubtypeTokenScopeInsufficient) + } + if len(permErr.MissingScopes) != 1 || permErr.MissingScopes[0] != "spark:app:read" { + t.Fatalf("MissingScopes = %v, want [spark:app:read]", permErr.MissingScopes) + } + }) + + t.Run("http 503 classifies as retryable network server_error", func(t *testing.T) { + _, err := parseIssueCredentialData(&larkcore.ApiResp{ + StatusCode: http.StatusServiceUnavailable, RawBody: []byte(`{"msg":"upstream busy"}`), Header: header, + }, nil, errclass.ClassifyContext{}) + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("want typed problem, got %T: %v", err, err) + } + if p.Category != errs.CategoryNetwork || p.Subtype != errs.SubtypeNetworkServer { + t.Fatalf("classification = %s/%s, want network/server_error", p.Category, p.Subtype) + } + if !p.Retryable { + t.Fatalf("retryable = false, want true for 5xx") + } + }) +} diff --git a/shortcuts/apps/gitcred/gitconfig.go b/shortcuts/apps/gitcred/gitconfig.go index c12fb492..28b18381 100644 --- a/shortcuts/apps/gitcred/gitconfig.go +++ b/shortcuts/apps/gitcred/gitconfig.go @@ -5,10 +5,10 @@ package gitcred import ( "context" - "fmt" "os/exec" "strings" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" ) @@ -38,7 +38,7 @@ func (g GlobalGitConfig) SetHelper(ctx context.Context, gitHTTPURL, appID string return err } if hadHelper && previousHelper != helper && !g.isManagedHelper(previousHelper) { - return fmt.Errorf("git credential helper already configured for %s; refusing to overwrite non-lark helper", normalizedURL) + return errs.NewValidationError(errs.SubtypeFailedPrecondition, "git credential helper already configured for %s; refusing to overwrite non-lark helper", normalizedURL) } if err := exec.CommandContext(ctx, "git", "config", "--global", helperKey, helper).Run(); err != nil { return err @@ -106,7 +106,7 @@ func gitConfigGet(ctx context.Context, key string) (string, bool, error) { if isGitConfigGetMissing(err) { return "", false, nil } - return "", false, fmt.Errorf("get %s: %w", key, err) + return "", false, errs.NewInternalError(errs.SubtypeExternalTool, "git config get %s failed: %v", key, err).WithCause(err) } func gitConfigUnset(ctx context.Context, key string) error { @@ -114,7 +114,7 @@ func gitConfigUnset(ctx context.Context, key string) error { if isGitConfigUnsetMissing(err) { return nil } - return fmt.Errorf("unset %s: %w", key, err) + return errs.NewInternalError(errs.SubtypeExternalTool, "git config unset %s failed: %v", key, err).WithCause(err) } return nil } diff --git a/shortcuts/apps/gitcred/helper.go b/shortcuts/apps/gitcred/helper.go index cf2dcf45..d6543482 100644 --- a/shortcuts/apps/gitcred/helper.go +++ b/shortcuts/apps/gitcred/helper.go @@ -11,7 +11,7 @@ import ( "strings" "time" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/validate" ) @@ -40,21 +40,21 @@ func NewManager(store *Store, secrets *SecretStore, gitConfig GitConfig, issuer func (m *Manager) Init(ctx context.Context, profile ProfileContext, appID string) (*InitResult, error) { appID = strings.TrimSpace(appID) if appID == "" { - return nil, output.ErrValidation("--app-id is required") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--app-id is required").WithParam("--app-id") } if err := validate.ResourceName(appID, "--app-id"); err != nil { - return nil, err + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "%v", err).WithParam("--app-id").WithCause(err) } if profile.UserOpenID == "" { - return nil, output.ErrAuth("not logged in: run `lark-cli auth login --scope \"spark:app:read\"`") + return nil, errs.NewAuthenticationError(errs.SubtypeTokenMissing, "not logged in").WithHint("run `lark-cli auth login --scope \"spark:app:read\"`") } unlockApp, err := lockApp(appID) if err != nil { - return nil, fmt.Errorf("acquire Git credential lock for %s: %w", appID, err) + return nil, errs.NewInternalError(errs.SubtypeStorage, "acquire Git credential lock for %s: %v", appID, err).WithCause(err) } defer unlockApp() if m.Issuer == nil { - return nil, output.Errorf(output.ExitAPI, "api_error", "git credential issuer is not configured") + return nil, errs.NewInternalError(errs.SubtypeUnknown, "git credential issuer is not configured") } issued, err := m.Issuer.Issue(ctx, appID, profile) if err != nil { @@ -125,14 +125,14 @@ func (m *Manager) Init(ctx context.Context, profile ProfileContext, appID string func (m *Manager) Remove(ctx context.Context, profile ProfileContext, appID string) (*RemoveResult, error) { appID = strings.TrimSpace(appID) if appID == "" { - return nil, output.ErrValidation("--app-id is required") + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "--app-id is required").WithParam("--app-id") } if err := validate.ResourceName(appID, "--app-id"); err != nil { - return nil, err + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, "%v", err).WithParam("--app-id").WithCause(err) } unlockApp, err := lockApp(appID) if err != nil { - return nil, fmt.Errorf("acquire Git credential lock for %s: %w", appID, err) + return nil, errs.NewInternalError(errs.SubtypeStorage, "acquire Git credential lock for %s: %v", appID, err).WithCause(err) } defer unlockApp() records, err := m.Store.FindByAppID(appID, ProfileContext{}) @@ -335,7 +335,7 @@ func (m *Manager) Erase(r io.Reader) error { } unlockApp, err := lockApp(record.AppID) if err != nil { - return fmt.Errorf("acquire Git credential lock for %s: %w", record.AppID, err) + return errs.NewInternalError(errs.SubtypeStorage, "acquire Git credential lock for %s: %v", record.AppID, err).WithCause(err) } defer unlockApp() record, err = m.Store.FindByURL(url) @@ -360,7 +360,8 @@ func (m *Manager) readConfirmed(url string, current ProfileContext) (CredentialR return CredentialRecord{}, "", false, err } if record.ProfileAppID != current.ProfileAppID || record.UserOpenID != current.UserOpenID { - return CredentialRecord{}, "", false, fmt.Errorf("current login does not match initialized credential; run `lark-cli apps +git-credential-init --app-id %s` with the current login or switch back to the original account", record.AppID) + return CredentialRecord{}, "", false, errs.NewValidationError(errs.SubtypeFailedPrecondition, "current login does not match initialized credential"). + WithHint(fmt.Sprintf("run `lark-cli apps +git-credential-init --app-id %s` with the current login or switch back to the original account", record.AppID)) } pat, err := m.Secrets.Get(record.PATRef) if err != nil { @@ -423,7 +424,7 @@ func ParseCredentialInput(r io.Reader) (CredentialInput, error) { func parseNormalizedForInput(raw string) (CredentialInput, error) { parts := strings.SplitN(raw, "://", 2) if len(parts) != 2 { - return CredentialInput{}, output.ErrValidation("invalid credential URL") + return CredentialInput{}, errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid credential URL") } hostPath := parts[1] idx := strings.Index(hostPath, "/") @@ -457,19 +458,19 @@ func defaultUsername(username string) string { func validateIssuedCredential(appID, normalizedURL string, issued *IssuedCredential, now int64) error { if issued == nil { - return output.Errorf(output.ExitAPI, "api_error", "Issue Miaoda Git credential: empty credential") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "Issue Miaoda Git credential: empty credential") } if issued.AppID != "" && issued.AppID != appID { - return output.Errorf(output.ExitAPI, "api_error", "Issue Miaoda Git credential: response app_id %q does not match requested app_id %q", issued.AppID, appID) + return errs.NewInternalError(errs.SubtypeInvalidResponse, "Issue Miaoda Git credential: response app_id %q does not match requested app_id %q", issued.AppID, appID) } if normalizedURL == "" { - return output.Errorf(output.ExitAPI, "api_error", "Issue Miaoda Git credential: response missing gitURL") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "Issue Miaoda Git credential: response missing gitURL") } if strings.TrimSpace(issued.PAT) == "" { - return output.Errorf(output.ExitAPI, "api_error", "Issue Miaoda Git credential: response missing token") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "Issue Miaoda Git credential: response missing token") } if issued.ExpiresAt <= now { - return output.Errorf(output.ExitAPI, "api_error", "Issue Miaoda Git credential: response expiredTime must be in the future") + return errs.NewInternalError(errs.SubtypeInvalidResponse, "Issue Miaoda Git credential: response expiredTime must be in the future") } return nil } diff --git a/shortcuts/apps/gitcred/lock.go b/shortcuts/apps/gitcred/lock.go index 3d6635a8..3cc0ae46 100644 --- a/shortcuts/apps/gitcred/lock.go +++ b/shortcuts/apps/gitcred/lock.go @@ -5,12 +5,12 @@ package gitcred import ( "errors" - "fmt" "path/filepath" "regexp" "sync" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/lockfile" "github.com/larksuite/cli/internal/vfs" //nolint:depguard // git credential locks live under CLI config dir and are not user file I/O. @@ -30,7 +30,7 @@ func lockURL(url string) func() { func lockApp(appID string) (func(), error) { dir := filepath.Join(core.GetConfigDir(), "locks") if err := vfs.MkdirAll(dir, 0700); err != nil { - return nil, fmt.Errorf("create Git credential lock dir: %w", err) + return nil, errs.NewInternalError(errs.SubtypeStorage, "create Git credential lock dir: %v", err).WithCause(err) } name := "apps_git_credential_" + safeLockNameChars.ReplaceAllString(appID, "_") + ".lock" lock := lockfile.New(filepath.Join(dir, filepath.Base(name))) diff --git a/shortcuts/apps/gitcred/url.go b/shortcuts/apps/gitcred/url.go index 1cab06ae..bb144799 100644 --- a/shortcuts/apps/gitcred/url.go +++ b/shortcuts/apps/gitcred/url.go @@ -12,17 +12,17 @@ import ( "path" "strings" - "github.com/larksuite/cli/internal/output" + "github.com/larksuite/cli/errs" ) func NormalizeGitHTTPURL(raw string) (string, error) { raw = strings.TrimSpace(raw) if raw == "" { - return "", output.ErrValidation("git_http_url is empty") + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "git_http_url is empty") } u, err := url.Parse(raw) if err != nil { - return "", output.ErrValidation("invalid git_http_url %q: %s", raw, err) + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "invalid git_http_url %q: %s", raw, err).WithCause(err) } return normalizeParsedURL(u) } @@ -31,7 +31,7 @@ func NormalizeCredentialInput(input CredentialInput) (string, error) { protocol := strings.TrimSpace(input.Protocol) host := strings.TrimSpace(input.Host) if protocol == "" || host == "" { - return "", output.ErrValidation("git credential input must include protocol and host") + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "git credential input must include protocol and host") } u := &url.URL{ Scheme: protocol, @@ -44,11 +44,11 @@ func NormalizeCredentialInput(input CredentialInput) (string, error) { func normalizeParsedURL(u *url.URL) (string, error) { scheme := strings.ToLower(strings.TrimSpace(u.Scheme)) if scheme != "http" && scheme != "https" { - return "", output.ErrValidation("git credential only supports http/https URLs") + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "git credential only supports http/https URLs") } host := normalizeHost(scheme, u.Host) if host == "" { - return "", output.ErrValidation("git_http_url host is empty") + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, "git_http_url host is empty") } cleanPath := cleanURLPath(u.EscapedPath()) normalized := (&url.URL{Scheme: scheme, Host: host, Path: cleanPath}).String() diff --git a/shortcuts/apps/html_publish_client.go b/shortcuts/apps/html_publish_client.go index c1b05b93..295873dd 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"` + data, err := api.runtime.ClassifyAPIResponse(apiResp) + if err != nil { + return nil, enrichHTMLPublishAPIError(err) } - if err := json.Unmarshal(raw, &envelope); err != nil { - return nil, fmt.Errorf("decode html-publish response: %w", err) + url, _ := data["url"].(string) + if url == "" { + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, + "html-publish response is missing the published app url") } - 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)) - } - 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 002d9dc6..998b4622 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) } } @@ -138,8 +180,18 @@ func TestBuildHTMLPublishFailureHint_NotFoundHintNoLongerMentionsList(t *testing } } -func TestParseHTMLPublishResponse_InvalidJSON(t *testing.T) { - if _, err := parseHTMLPublishResponse([]byte("{not json")); err == nil { - t.Error("malformed html-publish response must surface a decode error") +func TestAppsHTMLPublishAPI_MalformedResponseIsInvalidResponse(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", + RawBody: []byte("{not json"), + }) + + 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 b49d4eb7..b88909a9 100644 --- a/shortcuts/apps/html_publish_tarball.go +++ b/shortcuts/apps/html_publish_tarball.go @@ -9,10 +9,9 @@ import ( "compress/gzip" "crypto/sha256" "encoding/hex" - "errors" - "fmt" "io" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" ) @@ -26,7 +25,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 +44,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 +59,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 errs.NewInternalError(errs.SubtypeUnknown, "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 +75,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/storage.go b/shortcuts/apps/storage.go index 4e81dc09..fea308a2 100644 --- a/shortcuts/apps/storage.go +++ b/shortcuts/apps/storage.go @@ -5,11 +5,11 @@ package apps import ( "errors" - "fmt" "net/url" "os" "path/filepath" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/validate" "github.com/larksuite/cli/internal/vfs" //nolint:depguard // existing apps storage persists CLI config-dir state; it is not user file I/O. @@ -25,10 +25,10 @@ const storageRoot = "spark" // can traverse out of the storage directory. func checkSeg(name, what string) error { if err := validate.ResourceName(name, what); err != nil { - return fmt.Errorf("apps storage: %w", err) + return appsStorageError(err, "apps storage: %v", err) } if name == "." { - return fmt.Errorf("apps storage: %s must not be \".\"", what) + return errs.NewInternalError(errs.SubtypeStorage, "apps storage: %s must not be \".\"", what) } return nil } @@ -60,7 +60,7 @@ func Read(appID, key string) ([]byte, error) { if errors.Is(err, os.ErrNotExist) { return nil, nil } - return nil, fmt.Errorf("apps storage: read: %w", err) + return nil, appsStorageError(err, "apps storage: read: %v", err) } return data, nil } @@ -79,10 +79,10 @@ func Write(appID, key string, data []byte) error { return err } if err := vfs.MkdirAll(appDir(appID), 0700); err != nil { - return fmt.Errorf("apps storage: create dir: %w", err) + return appsStorageError(err, "apps storage: create dir: %v", err) } if err := validate.AtomicWrite(appKeyPath(appID, key), data, 0600); err != nil { - return fmt.Errorf("apps storage: write: %w", err) + return appsStorageError(err, "apps storage: write: %v", err) } return nil } @@ -96,7 +96,7 @@ func Delete(appID, key string) error { return err } if err := vfs.Remove(appKeyPath(appID, key)); err != nil && !errors.Is(err, os.ErrNotExist) { - return fmt.Errorf("apps storage: delete: %w", err) + return appsStorageError(err, "apps storage: delete: %v", err) } return nil } @@ -113,7 +113,7 @@ func List(appID string) ([]string, error) { if errors.Is(err, os.ErrNotExist) { return []string{}, nil } - return nil, fmt.Errorf("apps storage: read dir: %w", err) + return nil, appsStorageError(err, "apps storage: read dir: %v", err) } keys := make([]string, 0, len(entries)) for _, e := range entries { diff --git a/shortcuts/apps/walk_html_publish_candidates.go b/shortcuts/apps/walk_html_publish_candidates.go index d70257e9..4c762c5d 100644 --- a/shortcuts/apps/walk_html_publish_candidates.go +++ b/shortcuts/apps/walk_html_publish_candidates.go @@ -4,11 +4,11 @@ package apps import ( - "fmt" "io/fs" "path/filepath" "strings" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/extension/fileio" ) @@ -40,7 +40,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,7 +54,7 @@ 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) } // Skip a stray git repo: a directory named .git skips the whole subtree, // and a .git file (the gitdir pointer used by submodules/worktrees) is @@ -70,7 +70,7 @@ func walkHTMLPublishCandidates(fio fileio.FileIO, rootPath string) ([]htmlPublis } 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。 @@ -79,7 +79,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 @@ -87,7 +87,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 errs.NewInternalError(errs.SubtypeUnknown, "walker produced unsafe relative path %q for %s", relSlash, path) } out = append(out, htmlPublishCandidate{ RelPath: relSlash, diff --git a/shortcuts/common/runner.go b/shortcuts/common/runner.go index e3365d2f..d8be762c 100644 --- a/shortcuts/common/runner.go +++ b/shortcuts/common/runner.go @@ -298,6 +298,14 @@ func (ctx *RuntimeContext) CallAPITyped(method, url string, params map[string]in // carry fields a caller needs on failure (e.g. the file_token an overwrite // returned, for token-stability handling). func (ctx *RuntimeContext) ClassifyAPIResponse(resp *larkcore.ApiResp) (map[string]interface{}, error) { + return ClassifyAPIResponseWith(resp, ctx.APIClassifyContext()) +} + +// ClassifyAPIResponseWith is the RuntimeContext-free form of +// ClassifyAPIResponse for callers that drive the request outside a running +// shortcut (e.g. a cobra command holding only a factory) and supply their own +// classification context. +func ClassifyAPIResponseWith(resp *larkcore.ApiResp, cc errclass.ClassifyContext) (map[string]interface{}, error) { logID, _ := logIDFromHeader(resp)["log_id"].(string) result, parseErr := client.ParseJSONResponse(resp) @@ -321,7 +329,7 @@ func (ctx *RuntimeContext) ClassifyAPIResponse(resp *larkcore.ApiResp) (map[stri } } out, _ := resultMap["data"].(map[string]interface{}) - if apiErr := errclass.BuildAPIError(resultMap, ctx.APIClassifyContext()); apiErr != nil { + if apiErr := errclass.BuildAPIError(resultMap, cc); apiErr != nil { return out, apiErr } if resp.StatusCode >= 400 { diff --git a/skills/lark-apps/references/lark-apps-db-execute.md b/skills/lark-apps/references/lark-apps-db-execute.md index 1d3caf44..f7d78819 100644 --- a/skills/lark-apps/references/lark-apps-db-execute.md +++ b/skills/lark-apps/references/lark-apps-db-execute.md @@ -28,7 +28,7 @@ lark-cli apps +db-execute --app-id app_xxx --env dev --sql - --yes < /Users/.../ - 成功默认 JSON 读取 `data.results[]`；每个元素对应一条 SQL，常见字段有 `sql_type`、`data`、`record_count`、`affected_rows`。 - pretty 会按 SELECT/DML/DDL 自适应渲染；多语句会逐条显示 Statement 摘要。 -- 失败可能仍有前序语句已执行；看 `error.detail.statement_index`、`completed`、`rolled_back` 和 `hint` 决定从哪条继续。 +- 失败可能仍有前序语句已执行；此时 stdout 输出 `ok:false` 的 envelope（exit 非 0），从 `data` 读 `results[]`（全部逐条结果，失败语句 `sql_type` 为 `ERROR`）、`statement_index`、`error_code`、`error_message`、`rolled_back` 和 `note`，决定从哪条继续。 ## Agent 规则