feat(wiki): add exponential backoff retry for +node-create lock contention#1076
Merged
Merged
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:
📝 WalkthroughWalkthroughAdds interruptible exponential-backoff retries for wiki node creation on lock-contention (error 131009), routes stderr for retry logs, classifies contention errors, wraps exhausted retry errors retaining original metadata, and adds unit and dry-run e2e tests covering retry and validation cases. ChangesWiki Node Creation Retry Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
🚥 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 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 |
github-actions
Bot
added
domain/ccm
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
shortcuts/wiki/wiki_node_create.go (1)
375-386: ⚡ Quick winPreserve
ErrandRawwhen rewrappingExitError.Lines 375-386 rebuild the envelope but drop the original
ErrandRawfields, which can strip the causal chain or change downstream enrichment behavior. Copy those through when appending the retry hint.Suggested change
return &output.ExitError{ Code: exitErr.Code, + Err: exitErr.Err, + Raw: exitErr.Raw, Detail: &output.ErrDetail{ Type: exitErr.Detail.Type, Code: exitErr.Detail.Code, Message: exitErr.Detail.Message, Hint: hint,🤖 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/wiki/wiki_node_create.go` around lines 375 - 386, The current rewrap of an existing exitErr into a new &output.ExitError recreates Code and Detail but drops the original causal fields Err and Raw; update the return to copy exitErr.Err and exitErr.Raw into the new &output.ExitError so the causal chain and raw payload are preserved while still appending the retry hint into Detail (i.e., keep Code, Detail.*, plus Err and Raw from the original exitErr).shortcuts/wiki/wiki_node_create_test.go (1)
785-899: ⚡ Quick winAdd one test for the canceled-context branch.
The new suite doesn't exercise the
ctx.Done()path inshortcuts/wiki/wiki_node_create.goLines 319-323, so the "abort during backoff and returnctx.Err()" behavior can still regress unnoticed. A small cancellation test would close that gap.🤖 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/wiki/wiki_node_create_test.go` around lines 785 - 899, Add a test that exercises the ctx.Done() branch in runWikiNodeCreate by creating a fakeWikiNodeCreateClient that initially returns a lock-contention error (use output.ErrAPI(output.LarkErrWikiLockContention,...)), run runWikiNodeCreate with a context.WithCancel in a goroutine, cancel the context while the function is in its backoff/retry loop, and assert the call returned ctx.Err() (or context.Canceled), that no additional creates were invoked after cancellation, and that stderr/log contains the expected abort/retry message; reference the runWikiNodeCreate function and fakeWikiNodeCreateClient to locate where to add this test.
🤖 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/wiki/wiki_node_create_test.go`:
- Around line 785-899: Add a test that exercises the ctx.Done() branch in
runWikiNodeCreate by creating a fakeWikiNodeCreateClient that initially returns
a lock-contention error (use
output.ErrAPI(output.LarkErrWikiLockContention,...)), run runWikiNodeCreate with
a context.WithCancel in a goroutine, cancel the context while the function is in
its backoff/retry loop, and assert the call returned ctx.Err() (or
context.Canceled), that no additional creates were invoked after cancellation,
and that stderr/log contains the expected abort/retry message; reference the
runWikiNodeCreate function and fakeWikiNodeCreateClient to locate where to add
this test.
In `@shortcuts/wiki/wiki_node_create.go`:
- Around line 375-386: The current rewrap of an existing exitErr into a new
&output.ExitError recreates Code and Detail but drops the original causal fields
Err and Raw; update the return to copy exitErr.Err and exitErr.Raw into the new
&output.ExitError so the causal chain and raw payload are preserved while still
appending the retry hint into Detail (i.e., keep Code, Detail.*, plus Err and
Raw from the original exitErr).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a4da265-dce4-4171-a0e1-135578a73c87
📒 Files selected for processing (2)
shortcuts/wiki/wiki_node_create.goshortcuts/wiki/wiki_node_create_test.go
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1076 +/- ##
==========================================
+ Coverage 67.83% 67.85% +0.01%
==========================================
Files 592 592
Lines 55327 55373 +46
==========================================
+ Hits 37532 37574 +42
- Misses 14683 14685 +2
- Partials 3112 3114 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@b1da213623d84e19406ed37c5ed824173668219f🧩 Skill updatenpx skills add larksuite/cli#fix/wiki-node-create-lock-contention-retry -y -g |
fangshuyu-768
force-pushed
the
fix/wiki-node-create-lock-contention-retry
branch
from
1752a34 to
b9fbbac
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 `@shortcuts/wiki/wiki_node_create_test.go`:
- Around line 831-869: The test TestRunWikiNodeCreateRetriesExhausted asserts a
3-attempt (2-retry) behavior but runWikiNodeCreate should perform initial
attempt + 4 retries for lock contention; update the fake client and assertions:
set client.createErrs to five lockErr entries, assert len(client.createInvoked)
== 5, and update the hint expectation to look for "failed after 4 retries" while
still verifying output.LarkErrWikiLockContention and that the original "lock
contention" hint is preserved; reference runWikiNodeCreate,
fakeWikiNodeCreateClient.createErrs and createInvoked, and
output.LarkErrWikiLockContention when making the changes.
In `@shortcuts/wiki/wiki_node_create.go`:
- Around line 374-385: When reconstructing the ExitError (the returned
&output.ExitError) you're dropping the original error chain and raw payload;
update the constructor to copy exitErr.Err and exitErr.Raw into the new
ExitError so errors.Unwrap() and any enrichment using Raw still work—i.e.,
include the Err and Raw fields from the original exitErr when building the
returned *output.ExitError.
🪄 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: 506c6da4-cb58-448c-b2f8-7c68b8016058
📒 Files selected for processing (2)
shortcuts/wiki/wiki_node_create.goshortcuts/wiki/wiki_node_create_test.go
fangshuyu-768
force-pushed
the
fix/wiki-node-create-lock-contention-retry
branch
from
b9fbbac to
4886efe
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
shortcuts/wiki/wiki_node_create_test.go (1)
840-865:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign exhausted-retry assertions with the configured retry budget.
This case still validates 3 total attempts / 2 retries, so it won’t catch under-retrying against the intended initial attempt + 4 retries behavior.
🔧 Suggested test fix
- createErrs: []error{lockErr, lockErr, lockErr}, // all 3 attempts fail + createErrs: []error{lockErr, lockErr, lockErr, lockErr, lockErr}, // all 5 attempts fail @@ - if len(client.createInvoked) != 3 { - t.Fatalf("create invoked %d times, want 3", len(client.createInvoked)) + if len(client.createInvoked) != 5 { + t.Fatalf("create invoked %d times, want 5", len(client.createInvoked)) } @@ - if !strings.Contains(exitErr.Detail.Hint, "failed after 2 retries") { + if !strings.Contains(exitErr.Detail.Hint, "failed after 4 retries") { t.Fatalf("hint = %q, want retry exhaustion message", exitErr.Detail.Hint) }🤖 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/wiki/wiki_node_create_test.go` around lines 840 - 865, Test currently asserts 3 total attempts (2 retries) but the retry budget should be initial attempt + 4 retries; update the test setup and assertions accordingly: make client.createErrs contain 5 errors (e.g., lockErr repeated 5 times) so runWikiNodeCreate will be invoked 5 times, change the check that inspects len(client.createInvoked) to expect 5, and update the expected retry-exhaustion hint asserted against exitErr.Detail.Hint to reference "failed after 4 retries" (keep references to runWikiNodeCreate, wikiNodeCreateSpec, client.createInvoked, lockErr, output.ExitError, and output.LarkErrWikiLockContention to locate changes).
🧹 Nitpick comments (1)
tests/cli_e2e/wiki/wiki_node_create_dryrun_test.go (1)
199-204: ⚡ Quick winMake error-message extraction resilient to non-JSON stderr/stdout formats.
validateWikiErrorMessagecurrently returns""whenerror.messageis absent in both streams. Add a fallback tor.Stdout + r.Stderrso validation-reject assertions remain stable even if envelope formatting changes.Suggested patch
func validateWikiErrorMessage(r *clie2e.Result) string { if msg := gjson.Get(r.Stdout, "error.message").String(); msg != "" { return msg } - return gjson.Get(r.Stderr, "error.message").String() + if msg := gjson.Get(r.Stderr, "error.message").String(); msg != "" { + return msg + } + return r.Stdout + r.Stderr }Based on learnings: in
tests/cli_e2e/*_dryrun_test.go, Validate-stage failures should be asserted fromresult.Stdout + result.Stderrwith non-zero exit.🤖 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 `@tests/cli_e2e/wiki/wiki_node_create_dryrun_test.go` around lines 199 - 204, The validateWikiErrorMessage helper currently only returns JSON-parsed "error.message" from r.Stdout or r.Stderr and can return an empty string if neither stream contains that field; change it to fall back to the raw concatenated output when parsing yields empty, i.e., after attempting gjson.Get on r.Stdout and r.Stderr in validateWikiErrorMessage, if both results are empty return r.Stdout + r.Stderr so tests assert against the combined raw output (refer to function validateWikiErrorMessage and the r.Stdout / r.Stderr variables).
🤖 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_node_create.go`:
- Around line 30-38: The retry budget is too low — update the constants so
wikiNodeCreateMaxRetries allows 4 retry attempts (so total attempts = initial +
4 retries) and keep wikiNodeCreateRetryBaseDelay at 250*time.Millisecond so
exponential backoff yields 250ms, 500ms, 1s, 2s; locate and update every
occurrence of wikiNodeCreateMaxRetries and wikiNodeCreateRetryBaseDelay
(including the duplicate declaration around the other occurrence) so the retry
loop and any backoff logic use the new maxRetries value.
---
Duplicate comments:
In `@shortcuts/wiki/wiki_node_create_test.go`:
- Around line 840-865: Test currently asserts 3 total attempts (2 retries) but
the retry budget should be initial attempt + 4 retries; update the test setup
and assertions accordingly: make client.createErrs contain 5 errors (e.g.,
lockErr repeated 5 times) so runWikiNodeCreate will be invoked 5 times, change
the check that inspects len(client.createInvoked) to expect 5, and update the
expected retry-exhaustion hint asserted against exitErr.Detail.Hint to reference
"failed after 4 retries" (keep references to runWikiNodeCreate,
wikiNodeCreateSpec, client.createInvoked, lockErr, output.ExitError, and
output.LarkErrWikiLockContention to locate changes).
---
Nitpick comments:
In `@tests/cli_e2e/wiki/wiki_node_create_dryrun_test.go`:
- Around line 199-204: The validateWikiErrorMessage helper currently only
returns JSON-parsed "error.message" from r.Stdout or r.Stderr and can return an
empty string if neither stream contains that field; change it to fall back to
the raw concatenated output when parsing yields empty, i.e., after attempting
gjson.Get on r.Stdout and r.Stderr in validateWikiErrorMessage, if both results
are empty return r.Stdout + r.Stderr so tests assert against the combined raw
output (refer to function validateWikiErrorMessage and the r.Stdout / r.Stderr
variables).
🪄 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: 54db8c0b-2985-4ac9-9921-fd10512209bd
📒 Files selected for processing (3)
shortcuts/wiki/wiki_node_create.goshortcuts/wiki/wiki_node_create_test.gotests/cli_e2e/wiki/wiki_node_create_dryrun_test.go
fangshuyu-768
force-pushed
the
fix/wiki-node-create-lock-contention-retry
branch
from
efac5cb to
7005935
Compare
…ntion (#1012) When creating wiki nodes under the same parent concurrently, the API returns error code 131009 (lock contention) ~5-15% of the time. This adds automatic retry with exponential backoff (250ms, 500ms; max 2 retries) so callers no longer need to implement retry logic themselves. - Retry loop in runWikiNodeCreate: only retries on code 131009, respects context cancellation, prints progress to stderr - wrapWikiNodeCreateRetryError preserves Err/Raw/Detail.Code in ExitError - 6 unit tests covering retry success, exhaustion, non-contention error, single-retry success, context cancellation, no-retry on success - 8 dry-run E2E tests for wiki +node-create request shape and validation
fangshuyu-768
force-pushed
the
fix/wiki-node-create-lock-contention-retry
branch
from
7005935 to
b1da213
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
shortcuts/wiki/wiki_node_create.go (1)
30-38:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIncrease lock-contention retries to match the expected backoff contract.
wikiNodeCreateMaxRetriesis still2(250ms, 500ms), which under-delivers the expected lock-contention retry sequence (250ms, 500ms, 1s, 2s) for this flow and can cause avoidable failures under burst creates.Suggested patch
const ( // wikiNodeCreateMaxRetries is the maximum number of retry attempts after // the initial request when the API returns lock contention (code 131009). - wikiNodeCreateMaxRetries = 2 + wikiNodeCreateMaxRetries = 4 // wikiNodeCreateRetryBaseDelay is the initial backoff delay for lock - // contention retries. Subsequent retries double the delay (250ms, 500ms). + // contention retries. Subsequent retries double the delay + // (250ms, 500ms, 1s, 2s). wikiNodeCreateRetryBaseDelay = 250 * time.Millisecond )🤖 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/wiki/wiki_node_create.go` around lines 30 - 38, The lock-contention retry count is too low: update the constant wikiNodeCreateMaxRetries from 2 to 4 so the backoff sequence using wikiNodeCreateRetryBaseDelay (250ms) yields 250ms, 500ms, 1s, 2s retries; keep wikiNodeCreateRetryBaseDelay as-is and update the associated comment to reflect the new 4 retry attempts and the expanded delay sequence to ensure burst creates get the expected backoff behavior.
🧹 Nitpick comments (1)
shortcuts/wiki/wiki_node_create_test.go (1)
834-868: ⚡ Quick winAssert
ExitError.ErrandExitError.Rawpreservation in the exhaustion test.On Line 856-868 the test only validates
Detail.Code/Hint, so a regression in wrappedErr/Rawcould slip through.Suggested test hardening
func TestRunWikiNodeCreateRetriesExhausted(t *testing.T) { t.Parallel() - lockErr := output.ErrAPI(output.LarkErrWikiLockContention, "lock contention", nil) + cause := errors.New("upstream lock contention") + lockErr := &output.ExitError{ + Code: 1, + Detail: &output.ErrDetail{ + Type: "api", + Code: output.LarkErrWikiLockContention, + Message: "lock contention", + Hint: "lock contention", + }, + Err: cause, + Raw: true, + } @@ if !strings.Contains(exitErr.Detail.Hint, "lock contention") { t.Fatalf("hint = %q, want original classification hint preserved", exitErr.Detail.Hint) } + if !exitErr.Raw { + t.Fatalf("Raw = %v, want true", exitErr.Raw) + } + if !errors.Is(exitErr.Err, cause) { + t.Fatalf("Err = %v, want wrapped cause %v", exitErr.Err, cause) + } }🤖 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/wiki/wiki_node_create_test.go` around lines 834 - 868, The test only checks exitErr.Detail fields; also assert that the original lockErr from createErrs is preserved in the returned ExitError by verifying exitErr.Err and exitErr.Raw reference or wrap the original lockErr from the test (the lockErr used to populate client.createErrs). Update the test around the runWikiNodeCreate call to assert errors.Is(exitErr.Err, lockErr) (or compare equality if Raw is expected) and that exitErr.Raw == lockErr (or errors.Is(exitErr.Raw, lockErr)) so both Err and Raw are validated alongside Detail.Code and Detail.Hint; locate symbols runWikiNodeCreate, fakeWikiNodeCreateClient, client.createErrs, lockErr and ExitError/exitErr to add these assertions.
🤖 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.
Duplicate comments:
In `@shortcuts/wiki/wiki_node_create.go`:
- Around line 30-38: The lock-contention retry count is too low: update the
constant wikiNodeCreateMaxRetries from 2 to 4 so the backoff sequence using
wikiNodeCreateRetryBaseDelay (250ms) yields 250ms, 500ms, 1s, 2s retries; keep
wikiNodeCreateRetryBaseDelay as-is and update the associated comment to reflect
the new 4 retry attempts and the expanded delay sequence to ensure burst creates
get the expected backoff behavior.
---
Nitpick comments:
In `@shortcuts/wiki/wiki_node_create_test.go`:
- Around line 834-868: The test only checks exitErr.Detail fields; also assert
that the original lockErr from createErrs is preserved in the returned ExitError
by verifying exitErr.Err and exitErr.Raw reference or wrap the original lockErr
from the test (the lockErr used to populate client.createErrs). Update the test
around the runWikiNodeCreate call to assert errors.Is(exitErr.Err, lockErr) (or
compare equality if Raw is expected) and that exitErr.Raw == lockErr (or
errors.Is(exitErr.Raw, lockErr)) so both Err and Raw are validated alongside
Detail.Code and Detail.Hint; locate symbols runWikiNodeCreate,
fakeWikiNodeCreateClient, client.createErrs, lockErr and ExitError/exitErr to
add these assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7da8d7a3-e27a-494d-b8b0-e3aa15aabe55
📒 Files selected for processing (3)
shortcuts/wiki/wiki_node_create.goshortcuts/wiki/wiki_node_create_test.gotests/cli_e2e/wiki/wiki_node_create_dryrun_test.go
wittam-01
approved these changes
May 25, 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
Closes #1012
When creating wiki nodes under the same parent concurrently, the API returns error code 131009 (lock contention) ~5-15% of the time. This adds automatic retry with exponential backoff (250ms, 500ms; max 2 retries) so callers no longer need to implement retry logic themselves.
Changes
shortcuts/wiki/wiki_node_create.go: retry loop on code 131009 with exponential backoff, context cancellation support, retry-exhaustion hint preservingDetail.Code/Err/Rawshortcuts/wiki/wiki_node_create_test.go: 6 retry unit teststests/cli_e2e/wiki/wiki_node_create_dryrun_test.go: dry-run E2E tests forwiki +node-createTest plan
Summary by CodeRabbit
New Features
Tests