diff --git a/.golangci.yml b/.golangci.yml index 175ee254a..220aa595d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -73,20 +73,20 @@ linters: - forbidigo # errs-typed-only enforced on paths already migrated to errs.NewXxxError. # Add a path when its migration is complete. - - path-except: (internal/auth/|internal/errcompat/|internal/errclass/|internal/client/|internal/cmdutil/factory\.go|cmd/auth/|cmd/config/|cmd/service/|shortcuts/common/mcp_client\.go|shortcuts/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/|shortcuts/im/) + - path-except: (internal/auth/|internal/errcompat/|internal/errclass/|internal/client/|internal/cmdutil/factory\.go|internal/event/consume/|cmd/auth/|cmd/config/|cmd/event/|cmd/service/|events/|shortcuts/common/mcp_client\.go|shortcuts/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/event/|shortcuts/im/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/) text: errs-typed-only linters: - forbidigo # errs-no-bare-wrap enforced on paths fully migrated to typed final # errors. Scoped separately from errs-typed-only because cmd/auth/, # cmd/config/ still have residual fmt.Errorf and must not be caught. - - path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/|shortcuts/im/|shortcuts/common/mcp_client\.go) + - path-except: (cmd/event/|events/|shortcuts/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/event/|shortcuts/im/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/|shortcuts/common/mcp_client\.go) text: errs-no-bare-wrap linters: - forbidigo # errs-no-legacy-helper enforced on domains whose shared validation/save # helpers have migrated to typed final errors. - - path-except: (shortcuts/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/|shortcuts/im/) + - path-except: (cmd/event/|events/|shortcuts/base/|shortcuts/calendar/|shortcuts/drive/|shortcuts/event/|shortcuts/im/|shortcuts/mail/|shortcuts/okr/|shortcuts/task/|shortcuts/whiteboard/) text: errs-no-legacy-helper linters: - forbidigo diff --git a/AGENTS.md b/AGENTS.md index 559b4baa3..69dc44c1b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -75,7 +75,31 @@ The one rule to internalize: **every error message you write will be parsed by a ### Structured errors in commands -`RunE` functions must return `output.Errorf` / `output.ErrWithHint` — never bare `fmt.Errorf`. AI agents parse stderr as JSON; bare errors break this contract. +Command-facing failures must be typed `errs.*` errors — never the legacy `output.Err*` helpers and never a final bare `fmt.Errorf`. AI agents parse the stderr envelope's `type` / `subtype` / `param` / `hint` fields to decide their next action; the full taxonomy lives in `errs/ERROR_CONTRACT.md`. + +Picking a constructor: + +| Failure | Constructor | +|---------|-------------| +| User flag/arg fails validation | `errs.NewValidationError(errs.SubtypeInvalidArgument, ...).WithParam("--flag")` | +| Valid request, wrong system state | `errs.NewValidationError(errs.SubtypeFailedPrecondition, ...).WithHint(...)` | +| Lark API returned `code != 0` | `runtime.CallAPITyped` (shortcuts) / `errclass.BuildAPIError` (raw responses) — never hand-build | +| Network / transport failure | `errs.NewNetworkError(errs.SubtypeNetworkTransport, ...)` | +| Local file I/O failure | `errs.NewInternalError(errs.SubtypeFileIO, ...)` — validate the path first (`validate.SafeInputPath` / `SafeOutputPath`) and use `vfs.*` | +| Unclassified lower-layer error as final | `errs.NewInternalError(errs.SubtypeUnknown, ...).WithCause(err)` | +| Lower layer already returned a typed error | pass it through unchanged — re-wrapping downgrades its classification | + +Signatures that are easy to guess wrong: + +- `runtime.CallAPITyped(method, url string, params map[string]interface{}, data interface{}) (map[string]interface{}, error)` — it performs the HTTP request itself and classifies `code != 0` into a typed error; just return the error it gives you. +- Typed pass-through check: `if _, ok := errs.ProblemOf(err); ok { return err }` — `ProblemOf` returns `(*errs.Problem, bool)`, not a nilable pointer. +- `.WithParam` exists only on `*errs.ValidationError`. `InternalError` / `NetworkError` have no param field — file or endpoint context goes in the message or `.WithHint(...)`. + +`forbidigo` + `lint/errscontract` reject the legacy `output.Err*` helpers, bare final `fmt.Errorf` / `errors.New`, and legacy envelope literals on migrated paths. Beyond what lint catches, three authoring conventions apply: + +- Preserve the underlying error with `.WithCause(err)` so `errors.Is` / `errors.Unwrap` keep working. +- `param` names only the user input that actually failed. Recovery guidance goes in `.WithHint(...)`; machine-readable recovery fields (`missing_scopes`, `log_id`) carry server/system ground truth only — never caller-side guesses. +- Error-path tests assert typed metadata via `errs.ProblemOf` (`category` / `subtype` / `param`) and cause preservation, not message substrings alone. ### stdout is data, stderr is everything else diff --git a/cmd/event/bus.go b/cmd/event/bus.go index 73d2958e7..61d2d3c0e 100644 --- a/cmd/event/bus.go +++ b/cmd/event/bus.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/event" @@ -38,7 +39,8 @@ func NewCmdBus(f *cmdutil.Factory) *cobra.Command { logger, err := bus.SetupBusLogger(eventsDir) if err != nil { - return err + return errs.NewInternalError(errs.SubtypeFileIO, + "set up bus logger: %s", err).WithCause(err) } tr := transport.New() @@ -58,7 +60,14 @@ func NewCmdBus(f *cmdutil.Factory) *cobra.Command { } }() - return b.Run(ctx) + if err := b.Run(ctx); err != nil { + if _, ok := errs.ProblemOf(err); ok { + return err + } + return errs.NewInternalError(errs.SubtypeUnknown, + "event bus daemon exited: %s", err).WithCause(err) + } + return nil }, } diff --git a/cmd/event/bus_test.go b/cmd/event/bus_test.go new file mode 100644 index 000000000..b974cb38a --- /dev/null +++ b/cmd/event/bus_test.go @@ -0,0 +1,45 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package event + +import ( + "os" + "path/filepath" + "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/core" +) + +// The hidden `event _bus` daemon command must exit with a typed file_io error +// when its log directory cannot be created (the error is only visible in the +// forked process's captured stderr / bus.log). +func TestBusCommandLoggerSetupFailureIsTypedFileIO(t *testing.T) { + dir := t.TempDir() + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", dir) + // Block the events/ root with a regular file so MkdirAll fails. + if err := os.WriteFile(filepath.Join(dir, "events"), []byte("x"), 0600); err != nil { + t.Fatal(err) + } + + f, _, _, _ := cmdutil.TestFactory(t, &core.CliConfig{ + AppID: "cli_bus_test", AppSecret: "secret", Brand: core.BrandFeishu, + }) + cmd := NewCmdBus(f) + cmd.SetArgs([]string{}) + + err := cmd.Execute() + if err == nil { + t.Fatal("expected logger setup error") + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed errs error, got %T: %v", err, err) + } + if p.Category != errs.CategoryInternal || p.Subtype != errs.SubtypeFileIO { + t.Errorf("problem = %s/%s, want %s/%s", p.Category, p.Subtype, + errs.CategoryInternal, errs.SubtypeFileIO) + } +} diff --git a/cmd/event/consume.go b/cmd/event/consume.go index 9fd4d234d..5060ccd1a 100644 --- a/cmd/event/consume.go +++ b/cmd/event/consume.go @@ -16,6 +16,7 @@ import ( "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/appmeta" "github.com/larksuite/cli/internal/auth" "github.com/larksuite/cli/internal/cmdutil" @@ -101,11 +102,10 @@ func runConsume(cmd *cobra.Command, f *cmdutil.Factory, eventKey string, o consu if o.jqExpr != "" { if err := output.ValidateJqExpression(o.jqExpr); err != nil { - return output.ErrWithHint( - output.ExitValidation, "validation", - err.Error(), - fmt.Sprintf("see `lark-cli event consume --help` EXAMPLES for common patterns, or `lark-cli event schema %s` for valid field paths", eventKey), - ) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", err). + WithParam("--jq"). + WithCause(err). + WithHint("see `lark-cli event consume --help` EXAMPLES for common patterns, or `lark-cli event schema %s` for valid field paths", eventKey) } } @@ -260,12 +260,12 @@ func preflightScopes(ctx context.Context, pf *preflightCtx) error { if len(missing) == 0 { return nil } - return output.ErrWithHint( - output.ExitAuth, "auth", - fmt.Sprintf("missing required scopes for EventKey %s (as %s): %s", - pf.eventKey, pf.identity, strings.Join(missing, ", ")), - scopeRemediationHint(pf.identity, missing, pf.appID, pf.brand), - ) + return errs.NewPermissionError(errs.SubtypeMissingScope, + "missing required scopes for EventKey %s (as %s): %s", + pf.eventKey, pf.identity, strings.Join(missing, ", ")). + WithIdentity(string(pf.identity)). + WithMissingScopes(missing...). + WithHint("%s", scopeRemediationHint(pf.identity, missing, pf.appID, pf.brand)) } // scopeRemediationHint returns an identity-appropriate fix for missing scopes. @@ -300,23 +300,27 @@ func preflightEventTypes(pf *preflightCtx) error { if len(missing) == 0 { return nil } - return output.ErrWithHint( - output.ExitValidation, "validation", - fmt.Sprintf("EventKey %s requires event types not subscribed in console: %s", - pf.keyDef.Key, strings.Join(missing, ", ")), - fmt.Sprintf("subscribe these events and publish a new app version at: %s", - consoleEventSubscriptionURL(pf.brand, pf.appID)), - ) + return errs.NewValidationError(errs.SubtypeFailedPrecondition, + "EventKey %s requires event types not subscribed in console: %s", + pf.keyDef.Key, strings.Join(missing, ", ")). + WithHint("subscribe these events and publish a new app version at: %s", + consoleEventSubscriptionURL(pf.brand, pf.appID)) } // sanitizeOutputDir rejects absolute/parent-escaping paths and ~ (SafeOutputPath treats it as a literal dir name). func sanitizeOutputDir(dir string) (string, error) { if strings.HasPrefix(dir, "~") { - return "", output.ErrValidation("%s; use a relative path like ./output instead", errOutputDirTilde) + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, + "%s; use a relative path like ./output instead", errOutputDirTilde). + WithParam("--output-dir"). + WithCause(errOutputDirTilde) } safe, err := validate.SafeOutputPath(dir) if err != nil { - return "", output.ErrValidation("%s %q: %s", errOutputDirUnsafe, dir, err) + return "", errs.NewValidationError(errs.SubtypeInvalidArgument, + "%s %q: %s", errOutputDirUnsafe, dir, err). + WithParam("--output-dir"). + WithCause(errOutputDirUnsafe) } return safe, nil } @@ -328,18 +332,21 @@ func resolveTenantToken(ctx context.Context, f *cmdutil.Factory, appID string) ( } result, err := f.Credential.ResolveToken(ctx, credential.NewTokenSpec(core.AsBot, appID)) if err != nil { - return "", output.ErrAuth("resolve tenant access token: %s", err) + if _, ok := errs.ProblemOf(err); ok { + return "", err + } + return "", errs.NewAuthenticationError(errs.SubtypeTokenMissing, + "resolve tenant access token: %s", err).WithCause(err) } if result == nil || result.Token == "" { - return "", output.ErrWithHint( - output.ExitAuth, "auth", - fmt.Sprintf("no tenant access token available for app %s", appID), - "Check that app_secret is configured (lark-cli config show) and try 'lark-cli auth login'.", - ) + return "", errs.NewAuthenticationError(errs.SubtypeTokenMissing, + "no tenant access token available for app %s", appID). + WithHint("Check that app_secret is configured (lark-cli config show) and try 'lark-cli auth login'.") } return result.Token, nil } +// Sentinels for errors.Is checks; call sites wrap them as typed ValidationError causes. var ( errInvalidParamFormat = errors.New("invalid --param format") errOutputDirTilde = errors.New("--output-dir does not support ~ expansion") @@ -351,7 +358,10 @@ func parseParams(raw []string) (map[string]string, error) { for _, kv := range raw { k, v, ok := strings.Cut(kv, "=") if !ok || k == "" { - return nil, output.ErrValidation("%s %q: expected key=value", errInvalidParamFormat, kv) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, + "%s %q: expected key=value", errInvalidParamFormat, kv). + WithParam("--param"). + WithCause(errInvalidParamFormat) } m[k] = v } diff --git a/cmd/event/consume_test.go b/cmd/event/consume_test.go index b484ceb6d..ff0906fad 100644 --- a/cmd/event/consume_test.go +++ b/cmd/event/consume_test.go @@ -4,9 +4,14 @@ package event import ( + "context" "errors" "strings" "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/credential" ) func TestParseParams(t *testing.T) { @@ -73,6 +78,7 @@ func TestParseParams(t *testing.T) { if tc.wantEcho != "" && !strings.Contains(err.Error(), tc.wantEcho) { t.Errorf("err %q should echo %q so user sees the bad input", err.Error(), tc.wantEcho) } + assertInvalidArgumentParam(t, err, "--param") return } if err != nil { @@ -90,6 +96,77 @@ func TestParseParams(t *testing.T) { } } +// emptyTokenResolver resolves to a result that carries no token. +type emptyTokenResolver struct{} + +func (emptyTokenResolver) ResolveToken(_ context.Context, _ credential.TokenSpec) (*credential.TokenResult, error) { + return &credential.TokenResult{}, nil +} + +// failingTokenResolver fails outright with an untyped error. +type failingTokenResolver struct{} + +func (failingTokenResolver) ResolveToken(_ context.Context, _ credential.TokenSpec) (*credential.TokenResult, error) { + return nil, errors.New("backend unavailable") +} + +func factoryWithResolver(r credential.DefaultTokenResolver) *cmdutil.Factory { + return &cmdutil.Factory{Credential: credential.NewCredentialProvider(nil, nil, r, nil)} +} + +func TestResolveTenantToken_EmptyTokenResult(t *testing.T) { + _, err := resolveTenantToken(context.Background(), factoryWithResolver(emptyTokenResolver{}), "cli_x") + if err == nil { + t.Fatal("expected error, got nil") + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed errs error, got %T: %v", err, err) + } + if p.Category != errs.CategoryAuthentication || p.Subtype != errs.SubtypeTokenMissing { + t.Errorf("problem = %s/%s, want %s/%s", p.Category, p.Subtype, + errs.CategoryAuthentication, errs.SubtypeTokenMissing) + } + var malformed *credential.MalformedTokenResultError + if !errors.As(err, &malformed) { + t.Error("empty-token failure should preserve the credential-layer cause") + } +} + +func TestResolveTenantToken_ResolverFailure(t *testing.T) { + _, err := resolveTenantToken(context.Background(), factoryWithResolver(failingTokenResolver{}), "cli_x") + if err == nil { + t.Fatal("expected error, got nil") + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed errs error, got %T: %v", err, err) + } + if p.Category != errs.CategoryAuthentication || p.Subtype != errs.SubtypeTokenMissing { + t.Errorf("problem = %s/%s, want %s/%s", p.Category, p.Subtype, + errs.CategoryAuthentication, errs.SubtypeTokenMissing) + } + if errors.Unwrap(err) == nil { + t.Error("resolver failure should preserve its cause") + } +} + +// assertInvalidArgumentParam verifies err is a typed validation error with +// subtype invalid_argument naming the given flag in its param field. +func assertInvalidArgumentParam(t *testing.T, err error, param string) { + t.Helper() + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %s, want %s", ve.Subtype, errs.SubtypeInvalidArgument) + } + if ve.Param != param { + t.Errorf("param = %q, want %q", ve.Param, param) + } +} + func TestSanitizeOutputDir(t *testing.T) { cases := []struct { name string @@ -130,6 +207,7 @@ func TestSanitizeOutputDir(t *testing.T) { if !errors.Is(err, tc.wantSentry) { t.Fatalf("want errors.Is(err, %v), got %q", tc.wantSentry, err.Error()) } + assertInvalidArgumentParam(t, err, "--output-dir") return } if err != nil { diff --git a/cmd/event/preflight_test.go b/cmd/event/preflight_test.go index 8ba2eb506..b0239f012 100644 --- a/cmd/event/preflight_test.go +++ b/cmd/event/preflight_test.go @@ -8,10 +8,10 @@ import ( "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/appmeta" "github.com/larksuite/cli/internal/core" eventlib "github.com/larksuite/cli/internal/event" - "github.com/larksuite/cli/internal/output" ) func newPreflightCtx(appID string, brand core.LarkBrand, identity core.Identity, keyDef *eventlib.KeyDefinition, appVer *appmeta.AppVersion) *preflightCtx { @@ -89,19 +89,17 @@ func TestPreflightEventTypes_MissingBlocks(t *testing.T) { if !strings.Contains(err.Error(), "mail.user_mailbox.event.message_read_v1") { t.Errorf("error should name the missing event type, got: %v", err) } - var exit *output.ExitError - if !errors.As(err, &exit) { - t.Fatalf("expected output.ExitError, got %T: %v", err, err) + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed errs error, got %T: %v", err, err) } - if exit.Code != output.ExitValidation { - t.Errorf("ExitCode = %d, want ExitValidation (%d)", exit.Code, output.ExitValidation) - } - if exit.Detail == nil { - t.Fatal("expected Detail with hint") + if p.Category != errs.CategoryValidation || p.Subtype != errs.SubtypeFailedPrecondition { + t.Errorf("problem = %s/%s, want %s/%s", p.Category, p.Subtype, + errs.CategoryValidation, errs.SubtypeFailedPrecondition) } wantURL := "https://open.feishu.cn/app/cli_XXXXXXXXXXXXXXXX/event" - if !strings.Contains(exit.Detail.Hint, wantURL) { - t.Errorf("hint missing subscription URL %q\ngot: %s", wantURL, exit.Detail.Hint) + if !strings.Contains(p.Hint, wantURL) { + t.Errorf("hint missing subscription URL %q\ngot: %s", wantURL, p.Hint) } } @@ -145,17 +143,19 @@ func TestPreflightScopes_Bot_MissingBlocks(t *testing.T) { if !strings.Contains(err.Error(), "im:message.group_at_msg") { t.Errorf("error should name missing scope, got: %v", err) } - var exit *output.ExitError - if !errors.As(err, &exit) { - t.Fatalf("expected output.ExitError, got %T: %v", err, err) + var permErr *errs.PermissionError + if !errors.As(err, &permErr) { + t.Fatalf("expected *errs.PermissionError, got %T: %v", err, err) } - if exit.Code != output.ExitAuth { - t.Errorf("ExitCode = %d, want ExitAuth (%d)", exit.Code, output.ExitAuth) + if permErr.Category != errs.CategoryAuthorization || permErr.Subtype != errs.SubtypeMissingScope { + t.Errorf("problem = %s/%s, want %s/%s", permErr.Category, permErr.Subtype, + errs.CategoryAuthorization, errs.SubtypeMissingScope) } - if exit.Detail == nil { - t.Fatal("expected Detail with hint, got nil Detail") + wantMissing := []string{"im:message.group_at_msg"} + if len(permErr.MissingScopes) != 1 || permErr.MissingScopes[0] != wantMissing[0] { + t.Errorf("MissingScopes = %v, want %v", permErr.MissingScopes, wantMissing) } - hint := exit.Detail.Hint + hint := permErr.Hint wantSubstrings := []string{ "https://open.feishu.cn/app/cli_x/auth?q=", "im:message.group_at_msg", diff --git a/cmd/event/runtime.go b/cmd/event/runtime.go index 92bc3b17c..836e2b644 100644 --- a/cmd/event/runtime.go +++ b/cmd/event/runtime.go @@ -6,8 +6,8 @@ package event import ( "context" "encoding/json" - "fmt" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/client" "github.com/larksuite/cli/internal/core" ) @@ -26,7 +26,11 @@ func (r *consumeRuntime) CallAPI(ctx context.Context, method, path string, body As: r.accessIdentity, }) if err != nil { - return nil, err + if _, ok := errs.ProblemOf(err); ok { + return nil, err + } + return nil, errs.NewNetworkError(errs.SubtypeNetworkTransport, + "api %s %s: %s", method, path, err).WithCause(err) } // Non-JSON HTTP errors (gateway text/plain 404 etc.) skip OAPI envelope parsing. ct := resp.Header.Get("Content-Type") @@ -36,11 +40,16 @@ func (r *consumeRuntime) CallAPI(ctx context.Context, method, path string, body if len(body) > maxBodyEcho { body = body[:maxBodyEcho] + "…(truncated)" } - return nil, fmt.Errorf("api %s %s returned %d: %s", method, path, resp.StatusCode, body) + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, + "api %s %s returned %d: %s", method, path, resp.StatusCode, body) } result, err := client.ParseJSONResponse(resp) if err != nil { - return nil, err + if _, ok := errs.ProblemOf(err); ok { + return nil, err + } + return nil, errs.NewInternalError(errs.SubtypeInvalidResponse, + "api %s %s: %s", method, path, err).WithCause(err) } if apiErr := r.client.CheckResponse(result, r.accessIdentity); apiErr != nil { return json.RawMessage(resp.RawBody), apiErr diff --git a/cmd/event/runtime_test.go b/cmd/event/runtime_test.go new file mode 100644 index 000000000..f40b0b397 --- /dev/null +++ b/cmd/event/runtime_test.go @@ -0,0 +1,143 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package event + +import ( + "context" + "errors" + "io" + "net/http" + "strings" + "testing" + + lark "github.com/larksuite/oapi-sdk-go/v3" + larkcore "github.com/larksuite/oapi-sdk-go/v3/core" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/client" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/credential" +) + +// staticTokenResolver always returns a fixed token without any HTTP calls. +type staticTokenResolver struct{} + +func (s *staticTokenResolver) ResolveToken(_ context.Context, _ credential.TokenSpec) (*credential.TokenResult, error) { + return &credential.TokenResult{Token: "test-token"}, nil +} + +// stubRoundTripper intercepts every outgoing request with a canned response. +type stubRoundTripper struct { + respond func(*http.Request) (*http.Response, error) +} + +func (s stubRoundTripper) RoundTrip(r *http.Request) (*http.Response, error) { return s.respond(r) } + +func newTestConsumeRuntime(rt http.RoundTripper) *consumeRuntime { + sdk := lark.NewClient("test-app", "test-secret", + lark.WithEnableTokenCache(false), + lark.WithLogLevel(larkcore.LogLevelError), + lark.WithHttpClient(&http.Client{Transport: rt}), + ) + return &consumeRuntime{ + client: &client.APIClient{ + SDK: sdk, + ErrOut: io.Discard, + Credential: credential.NewCredentialProvider(nil, nil, &staticTokenResolver{}, nil), + Config: &core.CliConfig{AppID: "test-app", AppSecret: "test-secret", Brand: core.BrandFeishu}, + }, + accessIdentity: core.AsBot, + } +} + +func stubResponse(status int, contentType, body string) func(*http.Request) (*http.Response, error) { + return func(r *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: status, + Header: http.Header{"Content-Type": []string{contentType}}, + Body: io.NopCloser(strings.NewReader(body)), + Request: r, + }, nil + } +} + +func requireCallAPIProblem(t *testing.T, err error, category errs.Category, subtype errs.Subtype) { + t.Helper() + if err == nil { + t.Fatal("expected error, got nil") + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed errs error, got %T: %v", err, err) + } + if p.Category != category || p.Subtype != subtype { + t.Fatalf("problem = %s/%s, want %s/%s", p.Category, p.Subtype, category, subtype) + } +} + +func TestConsumeRuntimeCallAPI_NonJSONHTTPError(t *testing.T) { + r := newTestConsumeRuntime(stubRoundTripper{respond: stubResponse(http.StatusNotFound, "text/plain", "gone")}) + _, err := r.CallAPI(context.Background(), "GET", "/open-apis/event/v1/connection", nil) + requireCallAPIProblem(t, err, errs.CategoryInternal, errs.SubtypeInvalidResponse) + if !strings.Contains(err.Error(), "returned 404") { + t.Errorf("error should echo the HTTP status, got: %v", err) + } +} + +func TestConsumeRuntimeCallAPI_NonJSONHTTPErrorTruncatesLongBody(t *testing.T) { + long := strings.Repeat("x", 300) + r := newTestConsumeRuntime(stubRoundTripper{respond: stubResponse(http.StatusBadGateway, "text/html", long)}) + _, err := r.CallAPI(context.Background(), "GET", "/open-apis/event/v1/connection", nil) + requireCallAPIProblem(t, err, errs.CategoryInternal, errs.SubtypeInvalidResponse) + if !strings.Contains(err.Error(), "…(truncated)") { + t.Errorf("long body should be truncated in the message, got: %v", err) + } +} + +func TestConsumeRuntimeCallAPI_UnparsableJSONBody(t *testing.T) { + r := newTestConsumeRuntime(stubRoundTripper{respond: stubResponse(http.StatusOK, "application/json", "{not json")}) + _, err := r.CallAPI(context.Background(), "GET", "/open-apis/event/v1/connection", nil) + requireCallAPIProblem(t, err, errs.CategoryInternal, errs.SubtypeInvalidResponse) +} + +func TestConsumeRuntimeCallAPI_TransportFailure(t *testing.T) { + r := newTestConsumeRuntime(stubRoundTripper{respond: func(*http.Request) (*http.Response, error) { + return nil, errors.New("connection refused") + }}) + _, err := r.CallAPI(context.Background(), "GET", "/open-apis/event/v1/connection", nil) + if err == nil { + t.Fatal("expected error, got nil") + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed errs error, got %T: %v", err, err) + } + if p.Category != errs.CategoryNetwork { + t.Fatalf("category = %s, want %s", p.Category, errs.CategoryNetwork) + } +} + +func TestConsumeRuntimeCallAPI_EnvelopeErrorIsTyped(t *testing.T) { + r := newTestConsumeRuntime(stubRoundTripper{respond: stubResponse(http.StatusOK, "application/json", + `{"code":99991663,"msg":"app not found"}`)}) + _, err := r.CallAPI(context.Background(), "GET", "/open-apis/event/v1/connection", nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if _, ok := errs.ProblemOf(err); !ok { + t.Fatalf("envelope error should be typed via BuildAPIError, got %T: %v", err, err) + } +} + +func TestConsumeRuntimeCallAPI_Success(t *testing.T) { + r := newTestConsumeRuntime(stubRoundTripper{respond: stubResponse(http.StatusOK, "application/json", + `{"code":0,"data":{"ok":true}}`)}) + raw, err := r.CallAPI(context.Background(), "GET", "/open-apis/event/v1/connection", nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(string(raw), `"code":0`) { + t.Errorf("raw body should pass through, got: %s", raw) + } +} diff --git a/cmd/event/schema.go b/cmd/event/schema.go index 830ce0566..74b56edfb 100644 --- a/cmd/event/schema.go +++ b/cmd/event/schema.go @@ -11,6 +11,7 @@ import ( "github.com/spf13/cobra" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" eventlib "github.com/larksuite/cli/internal/event" "github.com/larksuite/cli/internal/event/schemas" @@ -39,12 +40,14 @@ func resolveSchemaJSON(def *eventlib.KeyDefinition) (json.RawMessage, []string, if len(def.Schema.FieldOverrides) > 0 { var parsed map[string]interface{} if err := json.Unmarshal(base, &parsed); err != nil { - return nil, nil, err + return nil, nil, errs.NewInternalError(errs.SubtypeUnknown, + "parse base schema for field overrides: %s", err).WithCause(err) } orphans := schemas.ApplyFieldOverrides(parsed, def.Schema.FieldOverrides) out, err := json.Marshal(parsed) if err != nil { - return nil, nil, err + return nil, nil, errs.NewInternalError(errs.SubtypeUnknown, + "serialize schema with field overrides: %s", err).WithCause(err) } return out, orphans, nil } @@ -73,7 +76,7 @@ func renderSpec(s *eventlib.SchemaSpec) (json.RawMessage, error) { copy(buf, s.Raw) return buf, nil } - return nil, fmt.Errorf("schemaSpec has neither Type nor Raw") + return nil, errs.NewInternalError(errs.SubtypeUnknown, "schemaSpec has neither Type nor Raw") } func NewCmdSchema(f *cmdutil.Factory) *cobra.Command { @@ -165,7 +168,7 @@ func runSchema(f *cmdutil.Factory, key string, asJSON bool) error { resolved, _, err := resolveSchemaJSON(def) if err != nil { - return output.Errorf(output.ExitInternal, "internal", "resolve schema: %v", err) + return err } if resolved != nil { fmt.Fprintf(out, "\nOutput Schema:\n") diff --git a/cmd/event/schema_test.go b/cmd/event/schema_test.go index 3dc16a993..2fb67f0b7 100644 --- a/cmd/event/schema_test.go +++ b/cmd/event/schema_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/cmdutil" "github.com/larksuite/cli/internal/core" eventlib "github.com/larksuite/cli/internal/event" @@ -129,3 +130,38 @@ func TestResolveSchemaJSON_CustomWithOverlay(t *testing.T) { t.Errorf("overlay format = %v, want open_id", got) } } + +func TestRenderSpec_EmptySpecIsTypedInternalError(t *testing.T) { + _, err := renderSpec(&eventlib.SchemaSpec{}) + if err == nil { + t.Fatal("expected error for spec with neither Type nor Raw") + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed errs error, got %T: %v", err, err) + } + if p.Category != errs.CategoryInternal { + t.Errorf("category = %s, want %s", p.Category, errs.CategoryInternal) + } +} + +func TestResolveSchemaJSON_InvalidBaseWithOverridesIsTypedInternalError(t *testing.T) { + def := &eventlib.KeyDefinition{ + Key: "synthetic.invalid.base", + Schema: eventlib.SchemaDef{ + Custom: &eventlib.SchemaSpec{Raw: json.RawMessage("{not json")}, + FieldOverrides: map[string]schemas.FieldMeta{"x": {}}, + }, + } + _, _, err := resolveSchemaJSON(def) + if err == nil { + t.Fatal("expected error for unparsable base schema") + } + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("expected typed errs error, got %T: %v", err, err) + } + if p.Category != errs.CategoryInternal { + t.Errorf("category = %s, want %s", p.Category, errs.CategoryInternal) + } +} diff --git a/cmd/event/suggestions.go b/cmd/event/suggestions.go index 2bdaf5dff..a49979e73 100644 --- a/cmd/event/suggestions.go +++ b/cmd/event/suggestions.go @@ -8,8 +8,8 @@ import ( "sort" "strings" + "github.com/larksuite/cli/errs" eventlib "github.com/larksuite/cli/internal/event" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/suggest" ) @@ -64,9 +64,6 @@ func unknownEventKeyErr(key string) error { if guesses := suggestEventKeys(key); len(guesses) > 0 { msg += " — did you mean " + formatSuggestions(guesses) + "?" } - return output.ErrWithHint( - output.ExitValidation, "validation", - msg, - "Run 'lark-cli event list' to see available keys.", - ) + return errs.NewValidationError(errs.SubtypeInvalidArgument, "%s", msg). + WithHint("Run 'lark-cli event list' to see available keys.") } diff --git a/events/minutes/preconsume.go b/events/minutes/preconsume.go index 82c329c85..2429bc53b 100644 --- a/events/minutes/preconsume.go +++ b/events/minutes/preconsume.go @@ -5,9 +5,9 @@ package minutes import ( "context" - "fmt" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/event" ) @@ -16,7 +16,8 @@ const cleanupTimeout = 5 * time.Second func subscriptionPreConsume(eventType, subscribePath, unsubscribePath string) func(context.Context, event.APIClient, map[string]string) (func(), error) { return func(ctx context.Context, rt event.APIClient, _ map[string]string) (func(), error) { if rt == nil { - return nil, fmt.Errorf("runtime API client is required for pre-consume subscription") + return nil, errs.NewInternalError(errs.SubtypeUnknown, + "runtime API client is required for pre-consume subscription") } body := map[string]string{"event_type": eventType} diff --git a/events/vc/note_detail_retry_test.go b/events/vc/note_detail_retry_test.go new file mode 100644 index 000000000..b433c418b --- /dev/null +++ b/events/vc/note_detail_retry_test.go @@ -0,0 +1,35 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package vc + +import ( + "errors" + "testing" + + "github.com/larksuite/cli/errs" +) + +// isLarkCode must match the API code on typed errs.* errors — the consume +// runtime classifies OAPI failures via errclass.BuildAPIError, so the +// not-found retry in fillVCNoteGeneratedDetails depends on this reading +// Problem.Code rather than the legacy envelope shape. +func TestIsLarkCode_MatchesTypedAPIErrorCode(t *testing.T) { + typedNotFound := errs.NewAPIError(errs.SubtypeNotFound, "note not ready"). + WithCode(vcNoteDetailNotFoundCode) + if !isLarkCode(typedNotFound, vcNoteDetailNotFoundCode) { + t.Fatal("typed API error carrying the not-found code must match (retry path)") + } + if isLarkCode(typedNotFound, 99999) { + t.Error("a different expected code must not match") + } + + otherTyped := errs.NewAPIError(errs.SubtypeServerError, "boom").WithCode(500) + if isLarkCode(otherTyped, vcNoteDetailNotFoundCode) { + t.Error("typed error with another code must not match") + } + + if isLarkCode(errors.New("plain failure"), vcNoteDetailNotFoundCode) { + t.Error("untyped error must not match") + } +} diff --git a/events/vc/note_generated.go b/events/vc/note_generated.go index 8aeef4927..ac5e45760 100644 --- a/events/vc/note_generated.go +++ b/events/vc/note_generated.go @@ -6,12 +6,11 @@ package vc import ( "context" "encoding/json" - "errors" "fmt" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/event" - "github.com/larksuite/cli/internal/output" "github.com/larksuite/cli/internal/validate" ) @@ -148,9 +147,8 @@ func fillVCNoteGeneratedDetails(ctx context.Context, rt event.APIClient, out *VC } func isLarkCode(err error, code int) bool { - var exitErr *output.ExitError - if errors.As(err, &exitErr) && exitErr.Detail != nil { - return exitErr.Detail.Code == code + if p, ok := errs.ProblemOf(err); ok { + return p.Code == code } return false } diff --git a/events/vc/preconsume.go b/events/vc/preconsume.go index 9bd03d941..1fa78cbcf 100644 --- a/events/vc/preconsume.go +++ b/events/vc/preconsume.go @@ -5,9 +5,9 @@ package vc import ( "context" - "fmt" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/event" ) @@ -16,7 +16,8 @@ const cleanupTimeout = 5 * time.Second func subscriptionPreConsume(eventType, subscribePath, unsubscribePath string) func(context.Context, event.APIClient, map[string]string) (func(), error) { return func(ctx context.Context, rt event.APIClient, _ map[string]string) (func(), error) { if rt == nil { - return nil, fmt.Errorf("runtime API client is required for pre-consume subscription") + return nil, errs.NewInternalError(errs.SubtypeUnknown, + "runtime API client is required for pre-consume subscription") } body := map[string]string{"event_type": eventType} diff --git a/events/whiteboard/preconsume.go b/events/whiteboard/preconsume.go index 26f335dfb..6e60416ce 100644 --- a/events/whiteboard/preconsume.go +++ b/events/whiteboard/preconsume.go @@ -8,6 +8,7 @@ import ( "fmt" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/event" "github.com/larksuite/cli/internal/validate" ) @@ -24,11 +25,15 @@ const cleanupTimeout = 5 * time.Second func whiteboardSubscriptionPreConsume(eventType string) func(context.Context, event.APIClient, map[string]string) (func(), error) { return func(ctx context.Context, rt event.APIClient, params map[string]string) (func(), error) { if rt == nil { - return nil, fmt.Errorf("runtime API client is required for pre-consume subscription") + return nil, errs.NewInternalError(errs.SubtypeUnknown, + "runtime API client is required for pre-consume subscription") } whiteboardID := params["whiteboard_id"] if whiteboardID == "" { - return nil, fmt.Errorf("param whiteboard_id is required for %s", eventType) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, + "param whiteboard_id is required for %s", eventType). + WithParam("--param"). + WithHint("pass it as --param whiteboard_id=; run `lark-cli event schema %s` for details", eventType) } encoded := validate.EncodePathSegment(whiteboardID) subscribePath := fmt.Sprintf("/open-apis/board/v1/whiteboards/%s/subscribe", encoded) diff --git a/events/whiteboard/preconsume_test.go b/events/whiteboard/preconsume_test.go index 26340cf5b..7af457040 100644 --- a/events/whiteboard/preconsume_test.go +++ b/events/whiteboard/preconsume_test.go @@ -11,6 +11,7 @@ import ( "sync" "testing" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/event" ) @@ -58,6 +59,16 @@ func TestWhiteboardSubscriptionPreConsume_MissingWhiteboardID(t *testing.T) { if !strings.Contains(err.Error(), "whiteboard_id") { t.Fatalf("error should mention whiteboard_id, got: %v", err) } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument || ve.Param != "--param" { + t.Errorf("subtype/param = %s/%q, want %s/%q", ve.Subtype, ve.Param, errs.SubtypeInvalidArgument, "--param") + } + if ve.Hint == "" { + t.Error("missing whiteboard_id should carry a hint") + } } // TestWhiteboardSubscriptionPreConsume_NilRuntime verifies that PreConsume @@ -70,6 +81,9 @@ func TestWhiteboardSubscriptionPreConsume_NilRuntime(t *testing.T) { if err == nil { t.Fatalf("expected error when runtime client is nil") } + if p, ok := errs.ProblemOf(err); !ok || p.Category != errs.CategoryInternal { + t.Errorf("nil-runtime invariant should be a typed internal error, got %T: %v", err, err) + } } // TestWhiteboardSubscriptionPreConsume_SubscribeError verifies that a diff --git a/internal/event/consume/consume.go b/internal/event/consume/consume.go index 6c862705a..0f51de2ef 100644 --- a/internal/event/consume/consume.go +++ b/internal/event/consume/consume.go @@ -14,6 +14,7 @@ import ( "sync/atomic" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/event" "github.com/larksuite/cli/internal/event/transport" ) @@ -44,7 +45,9 @@ func Run(ctx context.Context, tr transport.IPC, appID, profileName, domain strin keyDef, ok := event.Lookup(opts.EventKey) if !ok { - return fmt.Errorf("unknown EventKey: %s\nRun 'lark-cli event list' to see available keys", opts.EventKey) + return errs.NewValidationError(errs.SubtypeInvalidArgument, + "unknown EventKey: %s", opts.EventKey). + WithHint("run `lark-cli event list` to see available keys") } if err := validateParams(keyDef, opts.Params); err != nil { @@ -80,7 +83,8 @@ func Run(ctx context.Context, tr transport.IPC, appID, profileName, domain strin ack, br, err := doHello(conn, opts.EventKey, []string{keyDef.EventType}) if err != nil { - return fmt.Errorf("handshake failed: %w", err) + return errs.NewInternalError(errs.SubtypeUnknown, + "event bus handshake failed: %s", err).WithCause(err) } var cleanup func() @@ -90,7 +94,11 @@ func Run(ctx context.Context, tr transport.IPC, appID, profileName, domain strin } cleanup, err = keyDef.PreConsume(ctx, opts.Runtime, opts.Params) if err != nil { - return fmt.Errorf("pre-consume failed: %w", err) + if _, ok := errs.ProblemOf(err); ok { + return err + } + return errs.NewInternalError(errs.SubtypeUnknown, + "pre-consume failed: %s", err).WithCause(err) } } @@ -152,8 +160,10 @@ func validateParams(def *event.KeyDefinition, params map[string]string) error { for _, p := range def.Params { if p.Required { if _, ok := params[p.Name]; !ok { - return fmt.Errorf("required param %q missing for EventKey %s. Run 'lark-cli event schema %s' for details", - p.Name, def.Key, def.Key) + return errs.NewValidationError(errs.SubtypeInvalidArgument, + "required param %q missing for EventKey %s", p.Name, def.Key). + WithParam("--param"). + WithHint("pass it as --param %s=; run `lark-cli event schema %s` for details", p.Name, def.Key) } } } @@ -169,11 +179,15 @@ func validateParams(def *event.KeyDefinition, params map[string]string) error { continue } if len(validNames) == 0 { - return fmt.Errorf("unknown param %q: EventKey %s accepts no params. Run 'lark-cli event schema %s' for details", - k, def.Key, def.Key) + return errs.NewValidationError(errs.SubtypeInvalidArgument, + "unknown param %q: EventKey %s accepts no params", k, def.Key). + WithParam("--param"). + WithHint("run `lark-cli event schema %s` for details", def.Key) } - return fmt.Errorf("unknown param %q for EventKey %s. valid params: %s. Run 'lark-cli event schema %s' for details", - k, def.Key, strings.Join(validNames, ", "), def.Key) + return errs.NewValidationError(errs.SubtypeInvalidArgument, + "unknown param %q for EventKey %s. valid params: %s", k, def.Key, strings.Join(validNames, ", ")). + WithParam("--param"). + WithHint("run `lark-cli event schema %s` for details", def.Key) } return nil } diff --git a/internal/event/consume/jq.go b/internal/event/consume/jq.go index 416b98cfa..e1a9f8617 100644 --- a/internal/event/consume/jq.go +++ b/internal/event/consume/jq.go @@ -8,17 +8,21 @@ import ( "fmt" "github.com/itchyny/gojq" + + "github.com/larksuite/cli/errs" ) // CompileJQ compiles once for hot-path reuse; exported so callers can preflight before side effects. func CompileJQ(expr string) (*gojq.Code, error) { query, err := gojq.Parse(expr) if err != nil { - return nil, fmt.Errorf("invalid jq expression: %w", err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, + "invalid jq expression: %s", err).WithParam("--jq").WithCause(err) } code, err := gojq.Compile(query) if err != nil { - return nil, fmt.Errorf("jq compile error: %w", err) + return nil, errs.NewValidationError(errs.SubtypeInvalidArgument, + "jq compile error: %s", err).WithParam("--jq").WithCause(err) } return code, nil } diff --git a/internal/event/consume/loop_jq_test.go b/internal/event/consume/loop_jq_test.go index 97c607955..ff1891c1b 100644 --- a/internal/event/consume/loop_jq_test.go +++ b/internal/event/consume/loop_jq_test.go @@ -5,10 +5,13 @@ package consume import ( "encoding/json" + "errors" "fmt" "strings" "sync" "testing" + + "github.com/larksuite/cli/errs" ) func TestCompileJQReportsErrorEarly(t *testing.T) { @@ -20,6 +23,16 @@ func TestCompileJQReportsErrorEarly(t *testing.T) { if !strings.Contains(msg, "compile") && !strings.Contains(msg, "parse") && !strings.Contains(msg, "invalid") { t.Errorf("error should mention compile/parse/invalid, got: %v", err) } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument || ve.Param != "--jq" { + t.Errorf("subtype/param = %s/%q, want %s/%q", ve.Subtype, ve.Param, errs.SubtypeInvalidArgument, "--jq") + } + if errors.Unwrap(err) == nil { + t.Error("compile error should preserve its cause") + } } func TestCompileJQReturnsUsableCode(t *testing.T) { diff --git a/internal/event/consume/sink.go b/internal/event/consume/sink.go index 670b3e3fd..8cb4e3ea0 100644 --- a/internal/event/consume/sink.go +++ b/internal/event/consume/sink.go @@ -13,6 +13,7 @@ import ( "sync/atomic" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/vfs" ) @@ -23,7 +24,8 @@ type Sink interface { func newSink(opts Options) (Sink, error) { if opts.OutputDir != "" { if err := vfs.MkdirAll(opts.OutputDir, 0755); err != nil { - return nil, fmt.Errorf("create output dir: %w", err) + return nil, errs.NewInternalError(errs.SubtypeFileIO, + "create output dir: %s", err).WithCause(err) } // PID disambiguates filenames across processes sharing a Dir. return &DirSink{Dir: opts.OutputDir, pid: os.Getpid()}, nil diff --git a/internal/event/consume/startup.go b/internal/event/consume/startup.go index d8c5f6c4d..a42c119ea 100644 --- a/internal/event/consume/startup.go +++ b/internal/event/consume/startup.go @@ -16,6 +16,7 @@ import ( "path/filepath" "time" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/event" "github.com/larksuite/cli/internal/event/protocol" @@ -51,10 +52,9 @@ func EnsureBus(ctx context.Context, tr transport.IPC, appID, profileName, domain } else { fmt.Fprintf(errOut, "[event] remote connection check: online_instance_cnt=%d\n", count) if count > 0 { - return nil, fmt.Errorf("another event bus is already connected to this app "+ - "(%d active connection(s) detected via API).\n"+ - "Only one bus should run globally to avoid duplicate event delivery.\n"+ - "Use 'lark-cli event status' to check, or 'lark-cli event stop' on the other machine first", count) + return nil, errs.NewValidationError(errs.SubtypeFailedPrecondition, + "another event bus is already connected to this app (%d active connection(s) detected via API); only one bus should run globally to avoid duplicate event delivery", count). + WithHint("use `lark-cli event status` to check, or `lark-cli event stop` on the other machine first") } } } else { @@ -65,8 +65,10 @@ func EnsureBus(ctx context.Context, tr transport.IPC, appID, profileName, domain pid, forkErr := forkBus(tr, appID, profileName, domain) if forkErr != nil && !errors.Is(forkErr, lockfile.ErrHeld) { eventsRoot := filepath.Join(core.GetConfigDir(), "events") - return nil, fmt.Errorf("failed to start event bus daemon: %w\n"+ - "Check: disk space, permissions on %s, and 'lark-cli doctor'", forkErr, eventsRoot) + return nil, errs.NewInternalError(errs.SubtypeUnknown, + "failed to start event bus daemon: %s", forkErr). + WithCause(forkErr). + WithHint("check disk space, permissions on %s, and `lark-cli doctor`", eventsRoot) } if pid > 0 { announceForkedBus(errOut, pid) @@ -88,7 +90,9 @@ func EnsureBus(ctx context.Context, tr transport.IPC, appID, profileName, domain fmt.Fprintln(errOut, "[event] event bus exited unexpectedly.") fmt.Fprintln(errOut, "[event] please check app credentials (lark-cli config show) and retry.") fmt.Fprintf(errOut, "[event] logs: %s\n", logPath) - return nil, fmt.Errorf("failed to connect to event bus within %v (app=%s)", dialTimeout, appID) + return nil, errs.NewInternalError(errs.SubtypeUnknown, + "failed to connect to event bus within %v (app=%s)", dialTimeout, appID). + WithHint("check app credentials (`lark-cli config show`) and retry; bus logs: %s", logPath) } // probeAndDialBus distinguishes a healthy bus from a mid-shutdown listener via StatusQuery first. diff --git a/internal/event/consume/startup_guard_test.go b/internal/event/consume/startup_guard_test.go new file mode 100644 index 000000000..ca181ce35 --- /dev/null +++ b/internal/event/consume/startup_guard_test.go @@ -0,0 +1,98 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package consume + +import ( + "context" + "encoding/json" + "errors" + "io" + "net" + "strings" + "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/event" +) + +// failDialTransport refuses every dial so EnsureBus falls through to the +// remote-connection check without a local bus. +type failDialTransport struct{} + +func (failDialTransport) Listen(string) (net.Listener, error) { return nil, errors.New("no listen") } +func (failDialTransport) Dial(string) (net.Conn, error) { return nil, errors.New("refused") } +func (failDialTransport) Address(string) string { return "guard-test-addr" } +func (failDialTransport) Cleanup(string) {} + +// remoteBusyAPIClient reports active remote WebSocket connections. +type remoteBusyAPIClient struct{ count int } + +func (c remoteBusyAPIClient) CallAPI(context.Context, string, string, interface{}) (json.RawMessage, error) { + return json.RawMessage(`{"code":0,"msg":"ok","data":{"online_instance_cnt":` + + string(rune('0'+c.count)) + `}}`), nil +} + +func TestEnsureBus_RemoteBusAlreadyConnectedIsFailedPrecondition(t *testing.T) { + conn, err := EnsureBus(context.Background(), failDialTransport{}, + "cli_guard_test", "", "", remoteBusyAPIClient{count: 2}, io.Discard) + if conn != nil { + t.Fatal("expected nil conn when a remote bus is already connected") + } + if err == nil { + t.Fatal("expected single-bus guard error") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeFailedPrecondition { + t.Errorf("subtype = %s, want %s", ve.Subtype, errs.SubtypeFailedPrecondition) + } + if !strings.Contains(ve.Hint, "event stop") { + t.Errorf("hint should point at `event stop`, got: %q", ve.Hint) + } +} + +func TestRun_UnknownEventKeyIsTypedValidation(t *testing.T) { + err := Run(context.Background(), failDialTransport{}, "cli_x", "", "", Options{ + EventKey: "bogus.run.key", + ErrOut: io.Discard, + }) + if err == nil { + t.Fatal("expected unknown EventKey error") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument { + t.Errorf("subtype = %s, want %s", ve.Subtype, errs.SubtypeInvalidArgument) + } + if !strings.Contains(ve.Hint, "event list") { + t.Errorf("hint should point at `event list`, got: %q", ve.Hint) + } +} + +func TestRun_InvalidJQFailsBeforeAnySideEffect(t *testing.T) { + event.RegisterKey(event.KeyDefinition{ + Key: "consume.runtest.jq", + EventType: "consume.runtest.jq_v1", + Schema: event.SchemaDef{Custom: &event.SchemaSpec{Raw: json.RawMessage(`{}`)}}, + }) + err := Run(context.Background(), failDialTransport{}, "cli_x", "", "", Options{ + EventKey: "consume.runtest.jq", + JQExpr: "[invalid{{{", + ErrOut: io.Discard, + }) + if err == nil { + t.Fatal("expected jq validation error") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Param != "--jq" { + t.Errorf("param = %q, want %q", ve.Param, "--jq") + } +} diff --git a/internal/event/consume/validate_params_test.go b/internal/event/consume/validate_params_test.go new file mode 100644 index 000000000..188ffdcb7 --- /dev/null +++ b/internal/event/consume/validate_params_test.go @@ -0,0 +1,64 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package consume + +import ( + "errors" + "testing" + + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/event" +) + +func requireParamValidationError(t *testing.T, err error) { + t.Helper() + if err == nil { + t.Fatal("expected validation error, got nil") + } + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("expected *errs.ValidationError, got %T: %v", err, err) + } + if ve.Subtype != errs.SubtypeInvalidArgument || ve.Param != "--param" { + t.Errorf("subtype/param = %s/%q, want %s/%q", ve.Subtype, ve.Param, errs.SubtypeInvalidArgument, "--param") + } + if ve.Hint == "" { + t.Error("param validation error should hint at `lark-cli event schema`") + } +} + +func TestValidateParams_RequiredMissing(t *testing.T) { + def := &event.KeyDefinition{ + Key: "x.test", + Params: []event.ParamDef{{Name: "chat_id", Required: true}}, + } + requireParamValidationError(t, validateParams(def, map[string]string{})) +} + +func TestValidateParams_UnknownParam(t *testing.T) { + def := &event.KeyDefinition{ + Key: "x.test", + Params: []event.ParamDef{{Name: "chat_id"}}, + } + requireParamValidationError(t, validateParams(def, map[string]string{"nope": "1"})) +} + +func TestValidateParams_UnknownParamNoParamsAccepted(t *testing.T) { + def := &event.KeyDefinition{Key: "x.test"} + requireParamValidationError(t, validateParams(def, map[string]string{"nope": "1"})) +} + +func TestValidateParams_DefaultAppliedAndValidPasses(t *testing.T) { + def := &event.KeyDefinition{ + Key: "x.test", + Params: []event.ParamDef{{Name: "mode", Required: true, Default: "all"}}, + } + params := map[string]string{} + if err := validateParams(def, params); err != nil { + t.Fatalf("default should satisfy required param, got: %v", err) + } + if params["mode"] != "all" { + t.Errorf("default not applied, params=%v", params) + } +} diff --git a/lint/errscontract/rule_no_legacy_common_helper_call.go b/lint/errscontract/rule_no_legacy_common_helper_call.go index a6f05127e..41b43fa35 100644 --- a/lint/errscontract/rule_no_legacy_common_helper_call.go +++ b/lint/errscontract/rule_no_legacy_common_helper_call.go @@ -15,9 +15,13 @@ import ( // legacy validation/save helpers are forbidden; callers must use the typed // common replacements or construct an errs.* typed error directly. var migratedCommonHelperPaths = []string{ + "cmd/event/", + "events/", + "internal/event/consume/", "shortcuts/base/", "shortcuts/calendar/", "shortcuts/drive/", + "shortcuts/event/", "shortcuts/mail/", "shortcuts/okr/", "shortcuts/task/", diff --git a/lint/errscontract/rule_no_legacy_envelope_literal.go b/lint/errscontract/rule_no_legacy_envelope_literal.go index 37cada331..0025e0a0a 100644 --- a/lint/errscontract/rule_no_legacy_envelope_literal.go +++ b/lint/errscontract/rule_no_legacy_envelope_literal.go @@ -16,9 +16,13 @@ import ( // call sites must return a typed errs.* error instead. Future domains opt in by // appending their path prefix here. var migratedEnvelopePaths = []string{ + "cmd/event/", + "events/", + "internal/event/consume/", "shortcuts/base/", "shortcuts/calendar/", "shortcuts/drive/", + "shortcuts/event/", "shortcuts/mail/", "shortcuts/okr/", "shortcuts/task/", diff --git a/lint/errscontract/rule_no_legacy_runtime_api_call.go b/lint/errscontract/rule_no_legacy_runtime_api_call.go index e78ae9f1f..4a7d05e84 100644 --- a/lint/errscontract/rule_no_legacy_runtime_api_call.go +++ b/lint/errscontract/rule_no_legacy_runtime_api_call.go @@ -27,6 +27,11 @@ import ( // is not matched. runtime.DoAPI / runtime.RawAPI are intentionally not listed: // they return the raw response for the caller to classify and do not emit a // legacy envelope themselves. +// +// Files that do not import shortcuts/common are skipped: the legacy helpers +// are methods on common.RuntimeContext, so a same-named method on another +// receiver (for example the event domain's APIClient interface, whose +// implementation classifies into typed errs.* errors) is not a legacy call. func CheckNoLegacyRuntimeAPICall(path, src string) []Violation { if !isMigratedEnvelopePath(path) || strings.HasSuffix(path, "_test.go") { return nil @@ -36,6 +41,9 @@ func CheckNoLegacyRuntimeAPICall(path, src string) []Violation { if err != nil { return nil } + if !importsPath(file, commonImportPath) { + return nil + } var out []Violation ast.Inspect(file, func(n ast.Node) bool { call, ok := n.(*ast.CallExpr) @@ -71,3 +79,16 @@ func matchLegacyRuntimeAPIMethod(name string) (string, bool) { } return "", false } + +// importsPath reports whether the file imports the given package path. +func importsPath(file *ast.File, importPath string) bool { + for _, imp := range file.Imports { + if imp.Path == nil { + continue + } + if strings.Trim(imp.Path.Value, "`\"") == importPath { + return true + } + } + return false +} diff --git a/lint/errscontract/rules_test.go b/lint/errscontract/rules_test.go index 2e222ab3b..28b8513dd 100644 --- a/lint/errscontract/rules_test.go +++ b/lint/errscontract/rules_test.go @@ -813,6 +813,8 @@ func boom() error { func TestCheckNoLegacyRuntimeAPICall_RejectsCallAPIOnDrivePath(t *testing.T) { src := `package drive +import "github.com/larksuite/cli/shortcuts/common" + func boom(runtime *common.RuntimeContext) error { _, err := runtime.CallAPI("POST", "/x", nil, nil) return err @@ -833,6 +835,8 @@ func boom(runtime *common.RuntimeContext) error { func TestCheckNoLegacyRuntimeAPICall_RejectsCallAPIOnTaskPath(t *testing.T) { src := `package task +import "github.com/larksuite/cli/shortcuts/common" + func boom(runtime *common.RuntimeContext) error { _, err := runtime.CallAPI("POST", "/x", nil, nil) return err @@ -853,6 +857,8 @@ func boom(runtime *common.RuntimeContext) error { func TestCheckNoLegacyRuntimeAPICall_RejectsDoAPIJSONWithLogIDOnDrivePath(t *testing.T) { src := `package drive +import "github.com/larksuite/cli/shortcuts/common" + func boom(runtime *common.RuntimeContext) error { _, err := runtime.DoAPIJSONWithLogID("POST", "/x", nil, nil) return err @@ -1076,3 +1082,23 @@ func boom() error { t.Fatalf("expected 1 violation for function-value reference, got %d: %+v", len(v), v) } } + +func TestCheckNoLegacyRuntimeAPICall_SkipsNonCommonReceiver(t *testing.T) { + // The event domain's APIClient interface has a same-named CallAPI method + // whose implementation classifies into typed errs.* errors; without the + // shortcuts/common import the call cannot be the legacy RuntimeContext + // helper and must not fire. + src := `package vc + +import "github.com/larksuite/cli/internal/event" + +func boom(rt event.APIClient) error { + _, err := rt.CallAPI(nil, "POST", "/x", nil) + return err +} +` + v := CheckNoLegacyRuntimeAPICall("events/vc/preconsume.go", src) + if len(v) != 0 { + t.Errorf("non-common CallAPI receiver must not fire, got: %+v", v) + } +} diff --git a/shortcuts/event/errors.go b/shortcuts/event/errors.go new file mode 100644 index 000000000..4c582e8c9 --- /dev/null +++ b/shortcuts/event/errors.go @@ -0,0 +1,32 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package event + +import "github.com/larksuite/cli/errs" + +func eventValidationError(format string, args ...any) *errs.ValidationError { + return errs.NewValidationError(errs.SubtypeInvalidArgument, format, args...) +} + +func eventValidationParamError(param, format string, args ...any) *errs.ValidationError { + return eventValidationError(format, args...).WithParam(param) +} + +// eventValidationParamErrorWithCause appends ": " to the formatted +// message and preserves err as the unwrap cause. +func eventValidationParamErrorWithCause(err error, param, format string, args ...any) *errs.ValidationError { + return eventValidationParamError(param, format+": %s", append(args, err)...).WithCause(err) +} + +// eventFileIOError appends ": " to the formatted message and preserves +// err as the unwrap cause. +func eventFileIOError(err error, format string, args ...any) *errs.InternalError { + return errs.NewInternalError(errs.SubtypeFileIO, format+": %s", append(args, err)...).WithCause(err) +} + +// eventNetworkError appends ": " to the formatted message and preserves +// err as the unwrap cause. +func eventNetworkError(err error, format string, args ...any) *errs.NetworkError { + return errs.NewNetworkError(errs.SubtypeNetworkTransport, format+": %s", append(args, err)...).WithCause(err) +} diff --git a/shortcuts/event/pipeline.go b/shortcuts/event/pipeline.go index 34f957bfc..0b9b4caf5 100644 --- a/shortcuts/event/pipeline.go +++ b/shortcuts/event/pipeline.go @@ -63,13 +63,13 @@ func NewEventPipeline( func (p *EventPipeline) EnsureDirs() error { if p.config.OutputDir != "" { if err := vfs.MkdirAll(p.config.OutputDir, 0700); err != nil { - return fmt.Errorf("create output dir: %w", err) + return eventFileIOError(err, "create output dir") } } if p.config.Router != nil { for _, route := range p.config.Router.routes { if err := vfs.MkdirAll(route.dir, 0700); err != nil { - return fmt.Errorf("create route dir %s: %w", route.dir, err) + return eventFileIOError(err, "create route dir %s", route.dir) } } } diff --git a/shortcuts/event/processor_test.go b/shortcuts/event/processor_test.go index 464cda887..7505d7902 100644 --- a/shortcuts/event/processor_test.go +++ b/shortcuts/event/processor_test.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "io" "os" "path/filepath" @@ -15,7 +16,13 @@ import ( "testing" "time" + "github.com/larksuite/cli/errs" + "github.com/larksuite/cli/internal/cmdutil" + "github.com/larksuite/cli/internal/core" + "github.com/larksuite/cli/internal/lockfile" + "github.com/larksuite/cli/shortcuts/common" larkevent "github.com/larksuite/oapi-sdk-go/v3/event" + "github.com/spf13/cobra" ) // chdirTemp changes cwd to a fresh temp dir for the test duration. @@ -44,6 +51,87 @@ func makeRawEvent(eventType string, eventJSON string) *RawEvent { } } +func requireProblem(t *testing.T, err error, category errs.Category, subtype errs.Subtype, param string) { + t.Helper() + p, ok := errs.ProblemOf(err) + if !ok { + t.Fatalf("ProblemOf(%T) = false, error: %v", err, err) + } + if p.Category != category || p.Subtype != subtype { + t.Fatalf("problem = %s/%s, want %s/%s", p.Category, p.Subtype, category, subtype) + } + if param != "" { + var ve *errs.ValidationError + if !errors.As(err, &ve) { + t.Fatalf("error %T is not *errs.ValidationError", err) + } + if ve.Param != param { + t.Fatalf("Param = %q, want %q", ve.Param, param) + } + } +} + +func TestEventTypedErrorHelpers(t *testing.T) { + cause := errors.New("cause") + + validation := eventValidationError("bad input") + requireProblem(t, validation, errs.CategoryValidation, errs.SubtypeInvalidArgument, "") + + paramErr := eventValidationParamErrorWithCause(cause, "--flag", "bad %s value", "flag") + requireProblem(t, paramErr, errs.CategoryValidation, errs.SubtypeInvalidArgument, "--flag") + if got := paramErr.Error(); got != "bad flag value: cause" { + t.Fatalf("message = %q, want %q", got, "bad flag value: cause") + } + if !errors.Is(paramErr, cause) { + t.Fatal("validation error should preserve its cause") + } + + fileErr := eventFileIOError(cause, "write failed") + requireProblem(t, fileErr, errs.CategoryInternal, errs.SubtypeFileIO, "") + if got := fileErr.Error(); got != "write failed: cause" { + t.Fatalf("message = %q, want %q", got, "write failed: cause") + } + if !errors.Is(fileErr, cause) { + t.Fatal("file_io error should preserve its cause") + } + + networkErr := eventNetworkError(cause, "websocket failed") + requireProblem(t, networkErr, errs.CategoryNetwork, errs.SubtypeNetworkTransport, "") + if got := networkErr.Error(); got != "websocket failed: cause" { + t.Fatalf("message = %q, want %q", got, "websocket failed: cause") + } + if !errors.Is(networkErr, cause) { + t.Fatal("network error should preserve its cause") + } +} + +func newSubscribeTestRuntime(t *testing.T) *common.RuntimeContext { + t.Helper() + + var out, errOut bytes.Buffer + cmd := &cobra.Command{Use: "+subscribe"} + cmd.Flags().String("event-types", "", "") + cmd.Flags().String("filter", "", "") + cmd.Flags().Bool("json", false, "") + cmd.Flags().Bool("compact", false, "") + cmd.Flags().String("output-dir", "", "") + cmd.Flags().Bool("quiet", false, "") + cmd.Flags().StringArray("route", nil, "") + cmd.Flags().Bool("force", false, "") + + return &common.RuntimeContext{ + Cmd: cmd, + Config: &core.CliConfig{ + AppID: "cli_event_test", + AppSecret: "secret", + Brand: core.BrandFeishu, + }, + Factory: &cmdutil.Factory{ + IOStreams: cmdutil.NewIOStreams(strings.NewReader(""), &out, &errOut), + }, + } +} + // --- Registry --- func TestRegistryLookup(t *testing.T) { @@ -63,9 +151,11 @@ func TestRegistryDuplicateReturnsError(t *testing.T) { if err := r.Register(&ImMessageProcessor{}); err != nil { t.Fatalf("first register should succeed: %v", err) } - if err := r.Register(&ImMessageProcessor{}); err == nil { + err := r.Register(&ImMessageProcessor{}) + if err == nil { t.Error("expected error on duplicate registration") } + requireProblem(t, err, errs.CategoryInternal, errs.SubtypeUnknown, "") } // --- Filters --- @@ -106,6 +196,54 @@ func TestRegexFilter_Invalid(t *testing.T) { } } +func TestEventSubscribeExecuteRejectsUnsafeOutputDir(t *testing.T) { + rt := newSubscribeTestRuntime(t) + if err := rt.Cmd.Flags().Set("output-dir", "/tmp/events"); err != nil { + t.Fatal(err) + } + err := EventSubscribe.Execute(context.Background(), rt) + if err == nil { + t.Fatal("expected unsafe output-dir error") + } + requireProblem(t, err, errs.CategoryValidation, errs.SubtypeInvalidArgument, "--output-dir") + if errors.Unwrap(err) == nil { + t.Fatal("unsafe output-dir error should preserve its cause") + } +} + +func TestEventSubscribeExecuteRejectsInvalidFilter(t *testing.T) { + rt := newSubscribeTestRuntime(t) + if err := rt.Cmd.Flags().Set("force", "true"); err != nil { + t.Fatal(err) + } + if err := rt.Cmd.Flags().Set("filter", "[invalid"); err != nil { + t.Fatal(err) + } + err := EventSubscribe.Execute(context.Background(), rt) + if err == nil { + t.Fatal("expected invalid filter error") + } + requireProblem(t, err, errs.CategoryValidation, errs.SubtypeInvalidArgument, "--filter") + if errors.Unwrap(err) == nil { + t.Fatal("invalid filter error should preserve its cause") + } +} + +func TestEventSubscribeExecuteRejectsInvalidRoute(t *testing.T) { + rt := newSubscribeTestRuntime(t) + if err := rt.Cmd.Flags().Set("force", "true"); err != nil { + t.Fatal(err) + } + if err := rt.Cmd.Flags().Set("route", "no-equals-sign"); err != nil { + t.Fatal(err) + } + err := EventSubscribe.Execute(context.Background(), rt) + if err == nil { + t.Fatal("expected invalid route error") + } + requireProblem(t, err, errs.CategoryValidation, errs.SubtypeInvalidArgument, "--route") +} + func TestFilterChain(t *testing.T) { etf := NewEventTypeFilter("im.message.receive_v1, drive.file.edit_v1") rf, _ := NewRegexFilter("im\\..*") @@ -339,6 +477,106 @@ func TestPipeline_OutputDir(t *testing.T) { } } +func TestEventSubscribeExecuteRejectsHeldLock(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + lock, err := lockfile.ForSubscribe("cli_event_test") + if err != nil { + t.Fatal(err) + } + if err := lock.TryLock(); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = lock.Unlock() }) + + rt := newSubscribeTestRuntime(t) + execErr := EventSubscribe.Execute(context.Background(), rt) + if execErr == nil { + t.Fatal("expected lock-held error") + } + requireProblem(t, execErr, errs.CategoryValidation, errs.SubtypeFailedPrecondition, "") + if !errors.Is(execErr, lockfile.ErrHeld) { + t.Error("lock-held error should preserve lockfile.ErrHeld for errors.Is") + } + p, _ := errs.ProblemOf(execErr) + if p.Hint == "" { + t.Error("lock-held error should carry a recovery hint") + } + var ve *errs.ValidationError + if errors.As(execErr, &ve) && ve.Param != "" { + t.Errorf("lock contention names no offending flag; param = %q, want empty", ve.Param) + } +} + +func TestEventSubscribeDryRunEchoesFlags(t *testing.T) { + rt := newSubscribeTestRuntime(t) + for flag, value := range map[string]string{ + "event-types": "im.message.receive_v1", + "filter": "^im\\.", + "output-dir": "events_out", + } { + if err := rt.Cmd.Flags().Set(flag, value); err != nil { + t.Fatal(err) + } + } + if err := rt.Cmd.Flags().Set("route", "^im\\.message=dir:./messages"); err != nil { + t.Fatal(err) + } + + d := EventSubscribe.DryRun(context.Background(), rt) + if d == nil { + t.Fatal("DryRun returned nil") + } + payload, err := json.Marshal(d) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{ + `"command":"event +subscribe"`, + `"app_id":"cli_event_test"`, + `"event_types":"im.message.receive_v1"`, + `"output_dir":"events_out"`, + } { + if !strings.Contains(string(payload), want) { + t.Errorf("dry-run payload missing %s\ngot: %s", want, payload) + } + } +} + +func TestPipeline_EnsureDirsRouteDirFileIOError(t *testing.T) { + chdirTemp(t) + if err := os.WriteFile("blocked", []byte("x"), 0600); err != nil { + t.Fatal(err) + } + router, err := ParseRoutes([]string{`^im\.=dir:./blocked/child`}) + if err != nil { + t.Fatalf("ParseRoutes: %v", err) + } + p := NewEventPipeline(DefaultRegistry(), NewFilterChain(), + PipelineConfig{Mode: TransformCompact, Router: router}, io.Discard, io.Discard) + err = p.EnsureDirs() + if err == nil { + t.Fatal("expected file_io error for route dir blocked by a file") + } + requireProblem(t, err, errs.CategoryInternal, errs.SubtypeFileIO, "") +} + +func TestPipeline_EnsureDirsFileIOError(t *testing.T) { + path := filepath.Join(t.TempDir(), "not-a-dir") + if err := os.WriteFile(path, []byte("x"), 0600); err != nil { + t.Fatal(err) + } + p := NewEventPipeline(DefaultRegistry(), NewFilterChain(), + PipelineConfig{Mode: TransformCompact, OutputDir: filepath.Join(path, "child")}, io.Discard, io.Discard) + err := p.EnsureDirs() + if err == nil { + t.Fatal("expected file_io error") + } + requireProblem(t, err, errs.CategoryInternal, errs.SubtypeFileIO, "") + if errors.Unwrap(err) == nil { + t.Fatal("file_io error should preserve its cause") + } +} + // --- Pipeline: JsonFlag --- func TestPipeline_JsonFlag(t *testing.T) { @@ -608,6 +846,7 @@ func TestParseRoutes_MissingEquals(t *testing.T) { if err == nil { t.Error("expected error for missing =") } + requireProblem(t, err, errs.CategoryValidation, errs.SubtypeInvalidArgument, "--route") } func TestParseRoutes_InvalidRegex(t *testing.T) { @@ -615,6 +854,10 @@ func TestParseRoutes_InvalidRegex(t *testing.T) { if err == nil { t.Error("expected error for invalid regex") } + requireProblem(t, err, errs.CategoryValidation, errs.SubtypeInvalidArgument, "--route") + if errors.Unwrap(err) == nil { + t.Fatal("invalid regex error should preserve its cause") + } } func TestParseRoutes_MissingPrefix(t *testing.T) { @@ -622,6 +865,7 @@ func TestParseRoutes_MissingPrefix(t *testing.T) { if err == nil { t.Error("expected error for missing dir: prefix") } + requireProblem(t, err, errs.CategoryValidation, errs.SubtypeInvalidArgument, "--route") if !strings.Contains(err.Error(), "dir:") { t.Errorf("error should mention dir: prefix, got: %v", err) } @@ -632,6 +876,7 @@ func TestParseRoutes_EmptyPath(t *testing.T) { if err == nil { t.Error("expected error for empty path") } + requireProblem(t, err, errs.CategoryValidation, errs.SubtypeInvalidArgument, "--route") } func TestParseRoutes_RejectsAbsolutePath(t *testing.T) { @@ -639,6 +884,7 @@ func TestParseRoutes_RejectsAbsolutePath(t *testing.T) { if err == nil { t.Error("expected error for absolute path in route") } + requireProblem(t, err, errs.CategoryValidation, errs.SubtypeInvalidArgument, "--route") } func TestParseRoutes_RejectsTraversal(t *testing.T) { @@ -646,6 +892,7 @@ func TestParseRoutes_RejectsTraversal(t *testing.T) { if err == nil { t.Error("expected error for path traversal in route") } + requireProblem(t, err, errs.CategoryValidation, errs.SubtypeInvalidArgument, "--route") } func TestParseRoutes_PathSafety(t *testing.T) { diff --git a/shortcuts/event/registry.go b/shortcuts/event/registry.go index e51ef4f45..d7d627030 100644 --- a/shortcuts/event/registry.go +++ b/shortcuts/event/registry.go @@ -3,7 +3,7 @@ package event -import "fmt" +import "github.com/larksuite/cli/errs" // ProcessorRegistry manages event_type → EventProcessor mappings. type ProcessorRegistry struct { @@ -23,7 +23,7 @@ func NewProcessorRegistry(fallback EventProcessor) *ProcessorRegistry { func (r *ProcessorRegistry) Register(p EventProcessor) error { et := p.EventType() if _, exists := r.processors[et]; exists { - return fmt.Errorf("duplicate event processor for: %s", et) + return errs.NewInternalError(errs.SubtypeUnknown, "duplicate event processor for: %s", et) } r.processors[et] = p return nil diff --git a/shortcuts/event/router.go b/shortcuts/event/router.go index 079916475..f1949ef26 100644 --- a/shortcuts/event/router.go +++ b/shortcuts/event/router.go @@ -4,7 +4,6 @@ package event import ( - "fmt" "regexp" "strings" @@ -34,27 +33,27 @@ func ParseRoutes(specs []string) (*EventRouter, error) { for _, spec := range specs { parts := strings.SplitN(spec, "=", 2) if len(parts) != 2 { - return nil, fmt.Errorf("invalid route %q: expected format regex=dir:./path", spec) + return nil, eventValidationParamError("--route", "invalid --route %q: expected format regex=dir:./path", spec) } pattern := parts[0] target := parts[1] re, err := regexp.Compile(pattern) if err != nil { - return nil, fmt.Errorf("invalid regex in route %q: %w", spec, err) + return nil, eventValidationParamErrorWithCause(err, "--route", "invalid regex in --route %q", spec) } if !strings.HasPrefix(target, "dir:") { - return nil, fmt.Errorf("invalid route target %q: must start with \"dir:\" prefix (format: regex=dir:./path)", target) + return nil, eventValidationParamError("--route", "invalid --route target %q: must start with \"dir:\" prefix (format: regex=dir:./path)", target) } dir := strings.TrimPrefix(target, "dir:") if dir == "" { - return nil, fmt.Errorf("invalid route %q: directory path is empty", spec) + return nil, eventValidationParamError("--route", "invalid --route %q: directory path is empty", spec) } safeDir, err := validate.SafeOutputPath(dir) if err != nil { - return nil, fmt.Errorf("invalid route %q: %w", spec, err) + return nil, eventValidationParamErrorWithCause(err, "--route", "invalid --route %q", spec) } routes = append(routes, Route{pattern: re, dir: safeDir}) diff --git a/shortcuts/event/subscribe.go b/shortcuts/event/subscribe.go index 0878594af..941f5a700 100644 --- a/shortcuts/event/subscribe.go +++ b/shortcuts/event/subscribe.go @@ -6,6 +6,7 @@ package event import ( "context" "encoding/json" + "errors" "fmt" "io" "os" @@ -13,6 +14,7 @@ import ( "strings" "syscall" + "github.com/larksuite/cli/errs" "github.com/larksuite/cli/internal/core" "github.com/larksuite/cli/internal/lockfile" "github.com/larksuite/cli/internal/output" @@ -144,7 +146,7 @@ var EventSubscribe = common.Shortcut{ if outputDir != "" { safePath, err := validate.SafeOutputPath(outputDir) if err != nil { - return output.ErrValidation("unsafe output path: %s", err) + return eventValidationParamErrorWithCause(err, "--output-dir", "unsafe --output-dir") } outputDir = safePath } @@ -162,15 +164,18 @@ var EventSubscribe = common.Shortcut{ if !forceFlag { lock, err := lockfile.ForSubscribe(runtime.Config.AppID) if err != nil { - return fmt.Errorf("failed to create lock: %w", err) + return eventFileIOError(err, "failed to create event subscriber lock") } if err := lock.TryLock(); err != nil { - return output.ErrValidation( - "another event +subscribe instance is already running for app %s\n"+ - " Only one subscriber per app is allowed to prevent competing consumers.\n"+ - " Use --force to bypass this check.", - runtime.Config.AppID, - ) + if errors.Is(err, lockfile.ErrHeld) { + return errs.NewValidationError(errs.SubtypeFailedPrecondition, + "another event +subscribe instance is already running for app %s\n"+ + " Only one subscriber per app is allowed to prevent competing consumers.\n"+ + " Use --force to bypass this check.", + runtime.Config.AppID, + ).WithHint("stop the existing subscriber for this app, or rerun with --force if you accept split event delivery").WithCause(err) + } + return eventFileIOError(err, "failed to acquire event subscriber lock") } defer lock.Unlock() } @@ -179,7 +184,7 @@ var EventSubscribe = common.Shortcut{ eventTypeFilter := NewEventTypeFilter(eventTypesStr) regexFilter, err := NewRegexFilter(filterStr) if err != nil { - return output.ErrValidation("invalid --filter regex: %s", filterStr) + return eventValidationParamErrorWithCause(err, "--filter", "invalid --filter regex %q", filterStr) } var filterList []EventFilter if eventTypeFilter != nil { @@ -193,7 +198,7 @@ var EventSubscribe = common.Shortcut{ // --- Parse route --- router, err := ParseRoutes(routeSpecs) if err != nil { - return output.ErrValidation("invalid --route: %v", err) + return err } // --- Build pipeline --- @@ -292,7 +297,7 @@ var EventSubscribe = common.Shortcut{ return nil } if err != nil { - return output.ErrNetwork("WebSocket connection failed: %v", err) + return eventNetworkError(err, "WebSocket connection failed") } return nil } diff --git a/skills/lark-event/SKILL.md b/skills/lark-event/SKILL.md index 1d64e5953..e73d2a5d9 100644 --- a/skills/lark-event/SKILL.md +++ b/skills/lark-event/SKILL.md @@ -83,7 +83,11 @@ On exit, the last stderr line is `[event] exited — received N event(s) in Xs ( | 0 | `reason: limit` | `--max-events` reached | | 0 | `reason: timeout` | `--timeout` reached | | 0 | `reason: signal` | Ctrl+C / SIGTERM / stdin EOF | -| non-0 | `Error: ...` (no `exited` line) | Startup / runtime failure (permissions, network, params, config) | +| 2 | JSON error envelope on stderr (no `exited` line) | Validation failure (unknown EventKey, bad `--param` / `--jq`, another bus already connected) | +| 3 | JSON error envelope on stderr | Auth failure (missing token, missing scopes) | +| 4 / 5 | JSON error envelope on stderr | Network / internal failure (bus startup, handshake, file I/O) | + +Startup and runtime failures emit a structured JSON envelope on stderr: `{"ok":false,"error":{"type","subtype","param","message","hint",...}}` (the envelope may also carry top-level `identity` / `_notice` siblings). Parse `error.type` / `error.subtype` to branch (e.g. `missing_scope` carries a `missing_scopes` list), `error.param` to find the offending flag, and `error.hint` for the recovery action — do not regex-match message text. Orchestrators should treat `reason: limit/timeout/signal` (all exit 0) as "business completion" and non-zero as "failure". diff --git a/tests/cli_e2e/event/event_consume_error_test.go b/tests/cli_e2e/event/event_consume_error_test.go new file mode 100644 index 000000000..6cb022750 --- /dev/null +++ b/tests/cli_e2e/event/event_consume_error_test.go @@ -0,0 +1,41 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package event + +import ( + "context" + "testing" + "time" + + clie2e "github.com/larksuite/cli/tests/cli_e2e" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" +) + +// TestEventConsumeUnknownKeyRegression locks the typed error envelope emitted +// on stderr when `event consume` rejects an unknown EventKey. The lookup fails +// before any daemon fork or network access, so the test needs no credentials. +func TestEventConsumeUnknownKeyRegression(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + t.Setenv("LARKSUITE_CLI_APP_ID", "app") + t.Setenv("LARKSUITE_CLI_APP_SECRET", "secret") + t.Setenv("LARKSUITE_CLI_BRAND", "feishu") + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{"event", "consume", "bogus.key"}, + DefaultAs: "bot", + }) + require.NoError(t, err) + result.AssertExitCode(t, 2) + + errJSON := gjson.Get(result.Stderr, "error") + require.True(t, errJSON.Exists(), "stderr missing 'error' JSON envelope\nstderr:\n%s", result.Stderr) + require.Equal(t, "validation", errJSON.Get("type").String(), "stderr:\n%s", result.Stderr) + require.Equal(t, "invalid_argument", errJSON.Get("subtype").String(), "stderr:\n%s", result.Stderr) + require.Contains(t, errJSON.Get("message").String(), "unknown EventKey: bogus.key", "stderr:\n%s", result.Stderr) + require.Contains(t, errJSON.Get("hint").String(), "event list", "stderr:\n%s", result.Stderr) +} diff --git a/tests/cli_e2e/event/event_subscribe_dryrun_test.go b/tests/cli_e2e/event/event_subscribe_dryrun_test.go new file mode 100644 index 000000000..d1bd94a86 --- /dev/null +++ b/tests/cli_e2e/event/event_subscribe_dryrun_test.go @@ -0,0 +1,46 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package event + +import ( + "context" + "testing" + "time" + + clie2e "github.com/larksuite/cli/tests/cli_e2e" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" +) + +func TestEventSubscribeDryRun(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + t.Setenv("LARKSUITE_CLI_APP_ID", "app") + t.Setenv("LARKSUITE_CLI_APP_SECRET", "secret") + t.Setenv("LARKSUITE_CLI_BRAND", "feishu") + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "event", "+subscribe", + "--event-types", "im.message.receive_v1,contact.user.created_v3", + "--filter", "^im\\.", + "--output-dir", "events_out", + "--route", "^im\\.message=dir:./messages", + "--dry-run", + }, + DefaultAs: "bot", + }) + require.NoError(t, err) + result.AssertExitCode(t, 0) + + out := result.Stdout + require.Equal(t, "event +subscribe", gjson.Get(out, "command").String(), "stdout:\n%s", out) + require.Equal(t, "app", gjson.Get(out, "app_id").String(), "stdout:\n%s", out) + require.Equal(t, "im.message.receive_v1,contact.user.created_v3", gjson.Get(out, "event_types").String(), "stdout:\n%s", out) + require.Equal(t, "^im\\.", gjson.Get(out, "filter").String(), "stdout:\n%s", out) + require.Equal(t, "events_out", gjson.Get(out, "output_dir").String(), "stdout:\n%s", out) + require.Equal(t, "^im\\.message=dir:./messages", gjson.Get(out, "route").String(), "stdout:\n%s", out) +} diff --git a/tests/cli_e2e/event/event_subscribe_error_test.go b/tests/cli_e2e/event/event_subscribe_error_test.go new file mode 100644 index 000000000..20feadaba --- /dev/null +++ b/tests/cli_e2e/event/event_subscribe_error_test.go @@ -0,0 +1,47 @@ +// Copyright (c) 2026 Lark Technologies Pte. Ltd. +// SPDX-License-Identifier: MIT + +package event + +import ( + "context" + "testing" + "time" + + clie2e "github.com/larksuite/cli/tests/cli_e2e" + "github.com/stretchr/testify/require" + "github.com/tidwall/gjson" +) + +// TestEventSubscribeInvalidRouteRegression locks the typed error envelope +// emitted on stderr when +subscribe route parsing rejects user input. Route +// validation fails before any WebSocket connection is opened, so the test +// needs no credentials or network. +func TestEventSubscribeInvalidRouteRegression(t *testing.T) { + t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) + t.Setenv("LARKSUITE_CLI_APP_ID", "app") + t.Setenv("LARKSUITE_CLI_APP_SECRET", "secret") + t.Setenv("LARKSUITE_CLI_BRAND", "feishu") + + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + t.Cleanup(cancel) + + result, err := clie2e.RunCmd(ctx, clie2e.Request{ + Args: []string{ + "event", "+subscribe", + "--force", + "--route", "no-equals-sign", + }, + DefaultAs: "bot", + }) + require.NoError(t, err) + result.AssertExitCode(t, 2) + + errJSON := gjson.Get(result.Stderr, "error") + require.True(t, errJSON.Exists(), "stderr missing 'error' JSON envelope\nstderr:\n%s", result.Stderr) + require.Equal(t, "validation", errJSON.Get("type").String(), "stderr:\n%s", result.Stderr) + require.Equal(t, "invalid_argument", errJSON.Get("subtype").String(), "stderr:\n%s", result.Stderr) + require.Equal(t, "--route", errJSON.Get("param").String(), "stderr:\n%s", result.Stderr) + require.Equal(t, `invalid --route "no-equals-sign": expected format regex=dir:./path`, + errJSON.Get("message").String(), "stderr:\n%s", result.Stderr) +}