feat(client): pass isSuccess to ClientRetryPlugin onRetry returned callback#389
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request updates the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RetryPlugin
participant OnRetryFn
Client->>RetryPlugin: Initiate operation with retry logic
RetryPlugin->>OnRetryFn: Call onRetry(options)
Note right of OnRetryFn: Returns a callback function
loop For each retry attempt
RetryPlugin->>RetryPlugin: Execute retry attempt
alt Retry fails
OnRetryFn-->>RetryPlugin: Invoke callback(false)
else Retry succeeds
OnRetryFn-->>RetryPlugin: Invoke callback(true)
end
end
RetryPlugin->>Client: Return final result
Possibly related PRs
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
More templates
@orpc/arktype
@orpc/client
@orpc/openapi
@orpc/openapi-client
@orpc/contract
@orpc/react
@orpc/react-query
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/svelte-query
@orpc/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/client/src/plugins/retry.ts (2)
39-39: Consider usingundefinedinstead ofvoidin union type.The type signature uses
voidin a union type, which static analysis tools flag as potentially confusing.- onRetry?: (options: ClientRetryPluginAttemptOptions<ClientRetryPluginContext>) => void | ((isSuccess: boolean) => void) + onRetry?: (options: ClientRetryPluginAttemptOptions<ClientRetryPluginContext>) => undefined | ((isSuccess: boolean) => void)🧰 Tools
🪛 Biome (1.9.4)
[error] 39-39: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
80-80: Consider usingundefinedinstead ofvoidin union type.Similar to the interface declaration, using
voidin a union type here may cause confusion.- let callback: void | ((isSuccess: boolean) => void) + let callback: undefined | ((isSuccess: boolean) => void)🧰 Tools
🪛 Biome (1.9.4)
[error] 80-80: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/content/docs/plugins/client-retry.md(1 hunks)packages/client/src/plugins/retry.test.ts(2 hunks)packages/client/src/plugins/retry.ts(4 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/client/src/plugins/retry.ts
[error] 39-39: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 80-80: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
🔇 Additional comments (8)
apps/content/docs/plugins/client-retry.md (1)
57-63: LGTM: Clear and accurate documentation update.The documentation has been updated to reflect the new
onRetrycallback pattern, which now returns a function that accepts anisSuccessparameter. This clearly shows users how to implement logic that executes after a retry attempt completes.packages/client/src/plugins/retry.test.ts (3)
125-134: LGTM: Good test implementation approach.This change improves the test by simulating multiple retry attempts with eventual success on the 4th attempt, which better tests the real-world behavior of the retry mechanism.
139-139: Correctly updated expectation to match new behavior.The test expectation has been properly updated to expect a successful resolution with the "success" value instead of a rejection, aligning with the modified handler implementation.
176-178: LGTM: Properly testing callback arguments.The test now correctly verifies that the
cleancallback receives the appropriate boolean values indicating the success status of each retry attempt:
falsefor the first two attempts (failed retries)truefor the third attempt (successful retry)packages/client/src/plugins/retry.ts (4)
83-85: LGTM: Good variable renaming for clarity.Renaming from
currenttocurrentErrorand adjusting the function signature improves code readability by better reflecting the variable's purpose.
89-92: LGTM: Consistent error handling.Error handling has been correctly updated to use
currentErrorconsistently throughout the code.
110-110: Renamed variable improves semantic clarity.The variable rename from
unsubscribetocallbackbetter reflects its purpose since it's now being used to inform about retry success status.
131-132: LGTM: Properly implements the isSuccess callback.This implementation correctly:
- Passes the success status (
!currentError) to the callback- Clears the callback after use to prevent duplicate calls
This aligns with the interface change and documentation.
Summary by CodeRabbit
New Features
Documentation