fix(wiki): bound node retries and rate-limit recovery - #2292
Conversation
|
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:
📝 WalkthroughWalkthroughWiki node copy now retries lock-contention errors with bounded exponential backoff and preserves typed error details. Wiki node lookup now adds rate-limit recovery guidance. Tests and references document both behaviors. ChangesWiki recovery handling
Estimated code review effort: 3 (Moderate) | ~25 minutes Mergeability Score: 🔵 Low · up to The PR adds bounded retries for lock contention while leaving other errors unchanged, so no merge-blocking runtime risk is identified. Before merging, owners should clarify that the limit is three total attempts and document reducing bulk-lookup concurrency after rate-limit responses. Sequence Diagram(s)sequenceDiagram
participant RunWikiNodeCopy
participant runWikiNodeCopyWithRetry
participant WikiAPI
participant Context
RunWikiNodeCopy->>runWikiNodeCopyWithRetry: prepared API path and request body
runWikiNodeCopyWithRetry->>WikiAPI: copy wiki node
WikiAPI-->>runWikiNodeCopyWithRetry: lock-contention error
runWikiNodeCopyWithRetry->>Context: check cancellation during backoff
runWikiNodeCopyWithRetry->>WikiAPI: retry copy request
WikiAPI-->>runWikiNodeCopyWithRetry: success or final wrapped error
runWikiNodeCopyWithRetry-->>RunWikiNodeCopy: copied node data or preserved error contract
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
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/wiki/wiki_list_copy_test.go`:
- Around line 627-646: Extend TestRunWikiNodeCopyDoesNotRetryOtherErrors to
inspect errs.ProblemOf(err) and assert the returned error has
errs.CategoryAuthorization and errs.SubtypePermissionDenied metadata, while
retaining the existing single-call and cause-preservation assertions.
In `@shortcuts/wiki/wiki_node_copy.go`:
- Around line 125-129: Update the cancellation branch in the backoff select to
return a *errs.NetworkError, using SubtypeNetworkTimeout for deadline expiry and
SubtypeNetworkTransport for other cancellations, while preserving ctx.Err() via
WithCause. Add a backoff-cancellation test that verifies the error metadata and
cause and confirms no additional API call occurs.
🪄 Autofix
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 Plus
Run ID: 6ec0eb3d-339f-4405-a249-e6461c2ef910
📒 Files selected for processing (3)
shortcuts/wiki/wiki_list_copy_test.goshortcuts/wiki/wiki_node_copy.goskills/lark-wiki/references/lark-wiki-node-copy.md
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@668b17a84e0f917d745b45fc462dbc0fb25d43fb🧩 Skill updatenpx skills add larksuite/cli#fix/wiki-node-copy-lock-retry -y -g |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2292 +/- ##
==========================================
- Coverage 76.45% 74.67% -1.78%
==========================================
Files 1025 1025
Lines 113720 137489 +23769
==========================================
+ Hits 86939 102675 +15736
- Misses 20109 28140 +8031
- Partials 6672 6674 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
e17f8e1 to
71da58c
Compare
f5c7d3a to
72802fa
Compare
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 `@skills/lark-wiki/references/lark-wiki-node-get.md`:
- Around line 68-73: Update the rate_limit guidance in the bulk lookup retry
instructions to require callers to reduce concurrency after receiving a 99991400
response, in addition to honoring bounded backoff. Preserve the existing
retry-attempt limit and prohibition on identity switching or reauthorization,
and make the concurrency reduction explicit for bulk operations.
- Line 72: Update the retry guidance in the documented operation to state
explicitly that there are three total attempts: one initial request followed by
two retries, with retry delays of 250 ms and 500 ms; avoid wording that could
imply three retries after the initial request.
🪄 Autofix
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 Plus
Run ID: 39547476-0552-4899-bcb6-ed8effc6b771
📒 Files selected for processing (2)
shortcuts/wiki/wiki_node_get.goskills/lark-wiki/references/lark-wiki-node-get.md
🚧 Files skipped from review as they are similar to previous changes (1)
- shortcuts/wiki/wiki_node_get.go
72802fa to
3b4dec0
Compare
3b4dec0 to
b30ef1a
Compare
b30ef1a to
a55de79
Compare
a55de79 to
668b17a
Compare
fangshuyu-768
left a comment
There was a problem hiding this comment.
Re-reviewed the latest head. The simplified retry guidance preserves the bounded retry contract, and the focused verification passes.
Summary
wiki +node-copyonly when the Wiki API returns lock-contention code131009wiki +node-getrecovery guidance for99991400: honorretry_after_secondswhen present, otherwise use exponential backoff with jitter, and stop after 3 total attemptsSafety
wiki +node-copyretries only the explicit131009write-lock conflict. Network failures and unrelated API errors are not retried, avoiding ambiguous replay of this non-idempotent operation.wiki +node-getdoes not retry inside the CLI. It remainsretryable=trueand gives callers a bounded recovery policy: one initial request plus at most two delayed retries, then stop and try again later.Testing
go test ./shortcuts/wiki/...node scripts/skill-format-check/index.jsgit diff --check