-
Notifications
You must be signed in to change notification settings - Fork 105
feat: add experimental retry for prepareUserOp errors in transaction processing #916
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…processing Introduced a new environment variable EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS to enable retrying on prepareUserOp errors instead of failing the transaction immediately. Updated transaction handling logic to support this feature.
WalkthroughIntroduces a new experimental boolean env flag to control retry behavior for PrepareUserOp errors and updates the send-transaction worker to conditionally retry on such errors by rethrowing to trigger job retry; otherwise it preserves the existing path returning an errored transaction. Logging positions in catch blocks were adjusted accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor J as Job Runner
participant W as SendTransactionWorker
participant P as PrepareUserOp
participant S as SignUserOp
participant B as BundleUserOp
J->>W: _sendUserOp(job)
W->>P: prepareUserOp()
alt PrepareUserOp succeeds
P-->>W: preparedUserOp
W->>S: signUserOp()
alt Sign succeeds
S-->>W: signedUserOp
W->>B: bundleUserOp()
alt Bundle succeeds
B-->>W: bundling result
W-->>J: success
else Bundle error
W->>W: log error
W-->>J: return errored transaction
end
else Sign error
W->>W: log error
W-->>J: return errored transaction
end
else PrepareUserOp error
W->>W: log error
alt env.EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS = true
note over W: Throw to trigger job retry
W-->>J: throw error (retry)
else env flag = false
W-->>J: return errored transaction
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/shared/utils/env.ts (2)
167-167: Typo prevents ACCOUNT_CACHE_SIZE from being read.ACCOUNT_CAHCE_SIZE is misspelled, so the parsed env won’t populate ACCOUNT_CACHE_SIZE. This can impact caching and memory behavior.
Apply:
- ACCOUNT_CACHE_SIZE: process.env.ACCOUNT_CAHCE_SIZE, + ACCOUNT_CACHE_SIZE: process.env.ACCOUNT_CACHE_SIZE,
107-107: Avoid precision loss: parse EXPERIMENTAL__MAX_GAS_PRICE_WEI as a string.Defining 10**18 as a number loses integer precision beyond 2^53-1 and then BigInt(...) receives a rounded value. Store as a string and BigInt it at use sites.
Use:
- EXPERIMENTAL__MAX_GAS_PRICE_WEI: z.coerce.number().default(10 ** 18), + EXPERIMENTAL__MAX_GAS_PRICE_WEI: z + .string() + .regex(/^\d+$/) + .default("1000000000000000000"),No code changes required at use sites since BigInt(env.EXPERIMENTAL__MAX_GAS_PRICE_WEI) accepts strings.
🧹 Nitpick comments (2)
src/shared/utils/env.ts (1)
118-119: Env flag name/comment mismatches actual behavior — clarify scope or rename.This flag is used to retry errors in prepare, sign, and bundle phases (see worker). Either:
- Rename to EXPERIMENTAL__RETRY_USEROP_ERRORS, or
- Keep the name and restrict usage to prepare-only, or
- Fast fix: update the comment to reflect all three phases.
Fast-fix diff (comment only):
- // Retry prepareUserOp errors instead of immediately failing the transaction + // Retry userOp errors (prepare/sign/bundle) instead of immediately failing the transaction EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS: boolEnvSchema(false),Also applies to: 174-175
src/worker/tasks/send-transaction-worker.ts (1)
382-382: Consider redacting sensitive fields in logs.
stringify(signedUserOp)may include signatures and calldata. Redact or log a digest to reduce risk and log volume.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/shared/utils/env.ts(2 hunks)src/worker/tasks/send-transaction-worker.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/worker/tasks/send-transaction-worker.ts (1)
src/shared/utils/env.ts (1)
env(23-185)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (2)
src/worker/tasks/send-transaction-worker.ts (2)
368-371: Flag name suggests prepare-only, but code retries sign/bundle as well. Confirm intent.If the broader behavior is intended, consider renaming the flag or updating the comment in env.ts (see suggestion there). If not intended, gate rethrow only in the prepare catch.
Also applies to: 403-406
846-873: SendTransactionQueue retry/backoff verified — attempts=5 with exponential backoff (base 1s).
Configured in src/worker/queues/send-transaction-queue.ts (overrides defaultJobOptions: attempts: 5, backoff: { type: "exponential", delay: 1_000 }).
| job.log(`Failed to populate transaction: ${errorMessage}`); | ||
|
|
||
| // If retry is enabled for prepareUserOp errors, throw to trigger job retry | ||
| if (env.EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS) { | ||
| throw error; | ||
| } | ||
|
|
||
| // Otherwise, return errored transaction as before |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Throw a wrapped Error and fix log message to match the step.
- Log says “populate transaction” but this is the prepare step.
- Throwing the raw error can be non-Error/lose context; wrap consistently.
- job.log(`Failed to populate transaction: ${errorMessage}`);
+ job.log(`Failed to prepare userop: ${errorMessage}`);
- // If retry is enabled for prepareUserOp errors, throw to trigger job retry
+ // If retry is enabled, throw to trigger job retry
if (env.EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS) {
- throw error;
+ throw wrapError(error, "Bundler");
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/worker/tasks/send-transaction-worker.ts around lines 298 to 305, the log
and thrown value are incorrect: change the job.log message to refer to the
"prepare" step (e.g., "Failed to prepare transaction" or "Failed to prepare
userOp") instead of "populate transaction", and when retry is enabled wrap the
original error in an Error object before throwing — include a clear contextual
message and attach the original error (use Error cause if available or include
the original error.message/stack) so retries receive a proper Error instance
with preserved context.
| // If retry is enabled for prepareUserOp errors, throw to trigger job retry | ||
| if (env.EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS) { | ||
| throw error; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Rethrow a wrapped Error for consistent typing and telemetry.
Mirror the RPC path and the prepare catch: wrap before throwing.
- if (env.EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS) {
- throw error;
- }
+ if (env.EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS) {
+ throw wrapError(error, "Bundler");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // If retry is enabled for prepareUserOp errors, throw to trigger job retry | |
| if (env.EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS) { | |
| throw error; | |
| } | |
| // If retry is enabled for prepareUserOp errors, throw to trigger job retry | |
| if (env.EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS) { | |
| throw wrapError(error, "Bundler"); | |
| } |
🤖 Prompt for AI Agents
In src/worker/tasks/send-transaction-worker.ts around lines 368-372, instead of
rethrowing the raw caught error, wrap it in a new Error with a consistent
message (matching the RPC/prepare catch pattern) and include the original error
as the cause (or append its stack/message) before throwing; this preserves
consistent typing and telemetry while retaining original error details for
debugging.
| if (env.EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS) { | ||
| throw error; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Same here: wrap before rethrowing.
Aligns error shape and ensures BullMQ receives a proper Error instance.
- if (env.EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS) {
- throw error;
- }
+ if (env.EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS) {
+ throw wrapError(error, "Bundler");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (env.EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS) { | |
| throw error; | |
| } | |
| if (env.EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS) { | |
| throw wrapError(error, "Bundler"); | |
| } | |
🤖 Prompt for AI Agents
In src/worker/tasks/send-transaction-worker.ts around lines 403 to 406, the code
rethrows a non-Error value directly which can break BullMQ error handling;
instead wrap the original thrown value in a proper Error instance before
rethrowing. Replace the direct throw error with throwing a new Error constructed
from the original (e.g., new Error(error instanceof Error ? error.message :
String(error))) and, if supported, attach the original as a cause or set
error.stack to preserve debugging info so BullMQ receives a proper Error object.
Introduced a new environment variable EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORS to enable retrying on prepareUserOp errors instead of failing the transaction immediately. Updated transaction handling logic to support this feature.
Changes
How this PR will be tested
Output
(Example: Screenshot/GIF for UI changes, cURL output for API changes)
PR-Codex overview
This PR introduces a new experimental feature that allows retrying
prepareUserOperrors instead of failing the transaction immediately. It adds a configuration option and updates the transaction handling logic to support this retry mechanism.Detailed summary
EXPERIMENTAL__RETRY_PREPARE_USEROP_ERRORStoenvconfiguration.send-transaction-worker.tsto retry onprepareUserOperrors if the new flag is enabled._sendUserOpfunction to handle retries for signing and bundling operations.Summary by CodeRabbit