feat(contact): emit typed error envelopes across the contact domain#1287
Merged
Conversation
|
Lost in the diff? Review this PR in Change Stack to follow the change map from intent to exact ranges. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMigrate contact shortcuts to typed errs and classified API responses: expand lint migration scopes, add contact fanout error helpers and tests, switch CallAPI to CallAPITyped, refactor search validation/decoding, preserve fanout underlying errors, and update tests to assert typed problems. ChangesTyped Error Migration for Contact Domain
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
github-actions
Bot
added
domain/contact
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shortcuts/contact/contact_search_user_fanout.go (1)
222-349: 💤 Low valueConsider extracting repeated builder logic to reduce duplication.
All 8 switch cases share identical conditional-chaining logic (
WithCode,WithLogID,WithHint,WithRetryable). A small helper could cut this to ~30 lines while preserving behavior.♻️ Optional refactor sketch
// applyMetadata applies optional fields and returns an error interface. func applyMetadata(e interface { WithCode(int) interface{ WithLogID(string) interface{ WithHint(string, ...any) interface{ WithRetryable() error } } } // ... or define a shared TypedErrorBuilder interface in errs package }, first fanoutResult) error { if first.ErrCode != 0 { e = e.WithCode(first.ErrCode) } if first.ErrLog != "" { e = e.WithLogID(first.ErrLog) } if first.ErrHint != "" { e = e.WithHint("%s", first.ErrHint) } if first.ErrTry { return e.WithRetryable() } return e.(error) } func fanoutAllFailedError(first fanoutResult, msg string) error { subtype := first.ErrSub if subtype == "" { subtype = errs.SubtypeUnknown } var base interface{ ... } // typed error switch first.ErrCat { case errs.CategoryAuthentication: base = errs.NewAuthenticationError(subtype, "%s", msg) case errs.CategoryAuthorization: base = errs.NewPermissionError(subtype, "%s", msg) // ... other cases default: base = errs.NewAPIError(subtype, "%s", msg) } return applyMetadata(base, first) }This depends on whether the
errspackage exposes a common builder interface. If not, the current explicit approach is fine—just verbose.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shortcuts/contact/contact_search_user_fanout.go` around lines 222 - 349, The fanoutAllFailedError function repeats identical builder chaining across each error case; extract that duplication by creating a helper (e.g., applyMetadata) that accepts the error builder returned from errs.NewAuthenticationError / NewPermissionError / NewConfigError / NewNetworkError / NewValidationError / NewSecurityPolicyError / NewInternalError / NewAPIError and the fanoutResult, applies WithCode, WithLogID, WithHint and WithRetryable conditionally, and returns the final error; you can implement a small interface for the builder either locally or in the errs package to type the helper, then simplify fanoutAllFailedError to only choose the base error by ErrCat and call applyMetadata(base, first).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@shortcuts/contact/contact_search_user_fanout.go`:
- Around line 222-349: The fanoutAllFailedError function repeats identical
builder chaining across each error case; extract that duplication by creating a
helper (e.g., applyMetadata) that accepts the error builder returned from
errs.NewAuthenticationError / NewPermissionError / NewConfigError /
NewNetworkError / NewValidationError / NewSecurityPolicyError / NewInternalError
/ NewAPIError and the fanoutResult, applies WithCode, WithLogID, WithHint and
WithRetryable conditionally, and returns the final error; you can implement a
small interface for the builder either locally or in the errs package to type
the helper, then simplify fanoutAllFailedError to only choose the base error by
ErrCat and call applyMetadata(base, first).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 117150c8-65bb-492e-9a4b-dd6c7e1b7ab0
📒 Files selected for processing (7)
.golangci.ymllint/errscontract/rule_no_legacy_common_helper_call.golint/errscontract/rule_no_legacy_envelope_literal.goshortcuts/contact/contact_get_user.goshortcuts/contact/contact_search_user.goshortcuts/contact/contact_search_user_fanout.goshortcuts/contact/contact_search_user_test.go
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@20748e5f57bf652d109f53214ac3f3d1ceb5abf7🧩 Skill updatenpx skills add larksuite/cli#feat/errs-migrate-contact -y -g |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1287 +/- ##
==========================================
+ Coverage 71.47% 71.58% +0.10%
==========================================
Files 688 689 +1
Lines 65467 65521 +54
==========================================
+ Hits 46791 46901 +110
+ Misses 15031 14972 -59
- Partials 3645 3648 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
evandance
force-pushed
the
feat/errs-migrate-contact
branch
from
1bf7eef to
3dced30
Compare
github-actions
Bot
added
size/M
evandance
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/contact/contact_errors_test.go`:
- Around line 27-47: The test fails because contactFanoutAllFailedError
currently mutates the extracted typed problem's Message but returns an error
that does not preserve that updated Message; update contactFanoutAllFailedError
to extract the typed problem (using errs.ProblemOf), set p.Message to the new
decorated text, then reconstruct and return a new typed error that preserves all
original metadata (Category, Subtype, Code, LogID, Retryable, etc.) and includes
the updated Message so callers see the decorated message instead of the original
"rate limit". Ensure the returned value is a typed errs.APIError (or the same
concrete typed error used originally) rather than the unmodified original error.
In `@shortcuts/contact/contact_errors.go`:
- Around line 39-45: The code in contactFanoutAllFailedError mutates p.Message
returned by errs.ProblemOf(err) but ProblemOf returns a copy, so the mutation is
lost; replace this by reconstructing and returning a new typed error that
preserves the original error's metadata while setting Message to msg, e.g. use
errs.ProblemOf(err) to extract fields then use a type switch on the concrete
error type returned by err (APIError, NetworkError, etc.) to build the
corresponding typed error value with the same
category/subtype/code/log_id/retryable and the new message, and finally attach
WithHint(contactFanoutRetryHint) and WithCause(err) before returning; ensure
contactFanoutAllFailedError returns the reconstructed typed error so tests
expecting "all N queries failed" in p.Message pass.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: df69329e-0766-4be6-b680-ad7fe067cd99
📒 Files selected for processing (10)
.golangci.ymllint/errscontract/rule_no_legacy_common_helper_call.golint/errscontract/rule_no_legacy_envelope_literal.goshortcuts/contact/contact_errors.goshortcuts/contact/contact_errors_test.goshortcuts/contact/contact_get_user.goshortcuts/contact/contact_get_user_test.goshortcuts/contact/contact_search_user.goshortcuts/contact/contact_search_user_fanout.goshortcuts/contact/contact_search_user_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- .golangci.yml
- shortcuts/contact/contact_get_user.go
- shortcuts/contact/contact_search_user.go
evandance
force-pushed
the
feat/errs-migrate-contact
branch
2 times, most recently
from
0ded2f8 to
bfff947
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@shortcuts/contact/contact_search_user.go`:
- Around line 247-258: The function decodeSearchUserAPIData currently marshals a
nil/non-object data into "null" and silently decodes it to a zero-value
searchUserAPIData; fix this by explicitly rejecting missing/ nil data before
marshaling: in decodeSearchUserAPIData check if the input data is nil (or
otherwise clearly absent) and return contactInvalidResponseError("missing search
user response data") (preserving WithCause where appropriate) instead of
proceeding to json.Marshal, so malformed success bodies without a data object
are treated as an invalid response rather than an empty result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d63d92f3-3d26-4044-aca1-8c45fc8852ad
📒 Files selected for processing (10)
.golangci.ymllint/errscontract/rule_no_legacy_common_helper_call.golint/errscontract/rule_no_legacy_envelope_literal.goshortcuts/contact/contact_errors.goshortcuts/contact/contact_errors_test.goshortcuts/contact/contact_get_user.goshortcuts/contact/contact_get_user_test.goshortcuts/contact/contact_search_user.goshortcuts/contact/contact_search_user_fanout.goshortcuts/contact/contact_search_user_test.go
✅ Files skipped from review due to trivial changes (2)
- .golangci.yml
- lint/errscontract/rule_no_legacy_envelope_literal.go
🚧 Files skipped from review as they are similar to previous changes (2)
- shortcuts/contact/contact_get_user.go
- shortcuts/contact/contact_errors.go
evandance
force-pushed
the
feat/errs-migrate-contact
branch
from
bfff947 to
59dca34
Compare
evandance
changed the title
evandance
force-pushed
the
feat/errs-migrate-contact
branch
4 times, most recently
from
15b7e89 to
b74056b
Compare
github-actions
Bot
added
size/L
evandance
force-pushed
the
feat/errs-migrate-contact
branch
from
b74056b to
d8bd1df
Compare
evandance
force-pushed
the
feat/errs-migrate-contact
branch
from
d8bd1df to
20748e5
Compare
liangshuo-1
approved these changes
Jun 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
shortcuts/contactstill returned command-facing failures through legacyoutput.Err*, legacy common validation helpers, and legacy API-call envelopes. This migrates the contact domain to typederrs.*envelopes so callers get stabletype,subtype,param/params,code,hint,retryable, andlog_idmetadata across validation, API, network, invalid-response, and fanout all-failed flows.Changes
common.FlagErrorf,ResolveOpenIDs, andValidateUserIDto typed validation errors withparam/params.runtime.CallAPI/ manual response parsing toruntime.CallAPITypedorruntime.ClassifyAPIResponse.shortcuts/contact/contact_errors.gowith contact-local typed helpers for invalid responses and fanout error summary/all-failed decoration, matching the domain-helper pattern used by migrated drive/mail domains.+search-user --queriespartial-failure stdout shape while keeping the original typed error for the all-failed top-level return; typedcode,subtype,log_id,retryable, andhintare preserved viaerrs.ProblemOf.output.ExitErrordetails, and add coverage for+get-usertyped validation/API paths plus contact-local error helpers..golangci.ymlpath exceptions and the errscontract legacy-envelope / legacy-common-helper guards.Test Plan
gofmt -l .go test ./shortcuts/contact ./internal/errclassgo test -race ./shortcuts/contact ./internal/errclassgo test -cover ./shortcuts/contact— 89.8% package statement coveragego 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— 0 issuesgo mod tidyproduced nogo.mod/go.sumdiffRelated Issues
N/A
Summary by CodeRabbit
Bug Fixes
Refactor
New Features
Tests
Chores