Skip to content

fix(event): migrate shortcut errors to typed envelopes#1289

Open
evandance wants to merge 1 commit into
mainfrom
feat/errs-migrate-event
Open

fix(event): migrate shortcut errors to typed envelopes#1289
evandance wants to merge 1 commit into
mainfrom
feat/errs-migrate-event

Conversation

@evandance
Copy link
Copy Markdown
Collaborator

@evandance evandance commented Jun 5, 2026

Summary

Migrate the event shortcut domain to typed errs.* envelopes for validation, local file I/O, lock contention, registry, route parsing, and WebSocket transport failures. This keeps event command errors structured, actionable, and machine-readable for CLI consumers.

Changes

  • Add event-domain typed error helpers and replace legacy output.Err* / final fmt.Errorf paths in shortcuts/event.
  • Preserve stable recovery fields such as param, subtype, and wrapped causes for --route, --filter, --output-dir, subscriber locks, directory creation, and WebSocket failures.
  • Add shortcuts/event/ to the typed error lint and errscontract migrated-path guards.
  • Add unit assertions for typed problem metadata and a dry-run e2e for event +subscribe.

Test Plan

  • Unit tests pass: make unit-test
  • Manual local verification confirms the lark-cli event +subscribe --dry-run flow works as expected: go test ./tests/cli_e2e/event
  • go vet ./...
  • go run -C lint . ..
  • go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=origin/main

Related Issues

  • None

Summary by CodeRabbit

  • Tests

    • Added end-to-end test coverage for event subscription with dry-run functionality, validating output structure and command success.
  • Chores

    • Enhanced error handling consistency across event processing by standardizing error types throughout the codebase, improving maintainability and reliability.

@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label Jun 5, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 5, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cf8d4802-242b-4697-a711-c938de0c06e5

📥 Commits

Reviewing files that changed from the base of the PR and between f3949f0 and 2cf4055.

📒 Files selected for processing (10)
  • .golangci.yml
  • lint/errscontract/rule_no_legacy_common_helper_call.go
  • lint/errscontract/rule_no_legacy_envelope_literal.go
  • shortcuts/event/errors.go
  • shortcuts/event/pipeline.go
  • shortcuts/event/processor_test.go
  • shortcuts/event/registry.go
  • shortcuts/event/router.go
  • shortcuts/event/subscribe.go
  • tests/cli_e2e/event/event_subscribe_dryrun_test.go

📝 Walkthrough

Walkthrough

This PR standardizes error handling in the shortcuts/event/ package to use the project's typed errs library. Lint rules are configured to enforce typed errors in the package, a new error helper module is introduced with event-specific wrappers, and all error paths across registry, pipeline, router, and subscribe modules are refactored to use these helpers instead of generic fmt.Errorf and output error constructors.

Changes

Event error handling standardization

Layer / File(s) Summary
Lint rule configuration for shortcuts/event/
.golangci.yml, lint/errscontract/rule_no_legacy_common_helper_call.go, lint/errscontract/rule_no_legacy_envelope_literal.go
The errs-typed-only, errs-no-bare-wrap, and errs-no-legacy-helper forbidigo rules are extended to include shortcuts/event/ in their exempted/migrated path lists to enforce typed error patterns in that package.
Event error helper module
shortcuts/event/errors.go
New module defines eventValidationError, eventValidationErrorWithCause, eventFileIOError, and eventNetworkError helper functions that wrap the errs library for event-specific error scenarios.
Registry duplicate-registration error
shortcuts/event/registry.go
ProcessorRegistry.Register switches from fmt.Errorf to errs.NewInternalError(errs.SubtypeUnknown) for duplicate registration, with errs import added.
Pipeline directory creation error handling
shortcuts/event/pipeline.go
EventPipeline.EnsureDirs now returns eventFileIOError instead of fmt.Errorf for output directory and per-route directory creation failures.
Route parsing error handling
shortcuts/event/router.go
ParseRoutes validation failures (invalid format, regex compilation, dir: prefix, path safety) now return eventValidationError/eventValidationErrorWithCause with cause preservation; fmt import removed.
Event subscription error handling
shortcuts/event/subscribe.go
Subscribe command refactors all error paths: output directory validation, lock acquisition (with `--force` guidance), filter regex, route parsing, and WebSocket operations now use event-specific error helpers with structured error types.
Test support and validation updates
shortcuts/event/processor_test.go, tests/cli_e2e/event/event_subscribe_dryrun_test.go
Test suite adds requireProblem helper to assert structured errs.ProblemOf results; registry, pipeline, and route parsing tests updated to validate error types and causes; new event subscribe dry-run e2e test added.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

size/L

Suggested reviewers

  • liuxinyanglxy

Poem

🐰 Errors now typed, no more generic fray,
The event package finds a structured way,
With helpers wrapped and causes chained tight,
From registry to routes, all errors shine bright!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(event): migrate shortcut errors to typed envelopes' clearly and concisely summarizes the main change—migrating event domain errors to typed structured error envelopes.
Description check ✅ Passed The description comprehensively covers all required template sections with clear summary, detailed changes, completed test plan with verified checkmarks, and proper issue references.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/errs-migrate-event

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 5, 2026

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@2cf4055a7df1fe4ea4846c4ff419feb4da77b39c

🧩 Skill update

npx skills add larksuite/cli#feat/errs-migrate-event -y -g

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 5, 2026

Codecov Report

❌ Patch coverage is 50.00000% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.33%. Comparing base (f3949f0) to head (2cf4055).

Files with missing lines Patch % Lines
shortcuts/event/subscribe.go 0.00% 13 Missing ⚠️
shortcuts/event/errors.go 81.81% 2 Missing ⚠️
shortcuts/event/pipeline.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1289   +/-   ##
=======================================
  Coverage   70.33%   70.33%           
=======================================
  Files         672      673    +1     
  Lines       65322    65335   +13     
=======================================
+ Hits        45941    45952   +11     
- Misses      15728    15731    +3     
+ Partials     3653     3652    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant