fix: silence expected user-workflow failures from Sentry error alerts#526
Merged
Conversation
PR #463 bridged every logger.Error(...) call to Sentry, which flooded the dashboard with user-workflow failures (contract reverts, failed Tenderly simulations, on-chain UserOp reverts) that are expected outcomes, not infra bugs. This downgrades those sites to Warn and removes a set of leftover CRITICAL DEBUG traces. - vm_runner_contract_write.go: - Remove 5 leftover CRITICAL DEBUG log statements; the ReturnData nil path is expected when simulation reverts (tenderly_client.go explicitly nulls ReturnData in that case). - Downgrade method execution failed summary to Warn. - In the bundler-failed branch, keep Error for real infra/AA problems (AA21 prefund, bundler unreachable, etc.) but drop to Warn when the underlying error is success=false in UserOperationEvent (on-chain revert of the user target contract). - vm_runner_eth_transfer.go: same bundler Error/Warn branching. - tenderly_client.go: downgrade the two simulation-revert sites to Warn. - executor.go and engine.go: downgrade the task/simulation completed with failures summary lines to Warn; ExecutionStatus_FAILED on the persisted execution is the authoritative signal.
…al path
The on-chain-revert vs infra-failure classification added in the prior
commit was duplicated across contract-write and eth-transfer bundler
failure sites, and the withdrawal RPC path still unconditionally logged
Error for the same kinds of bundler failures. Pull the logic into a
single helper in pkg/erc4337/preset so all three call sites share it.
- New pkg/erc4337/preset/bundler_error.go:
- IsUserOpRevert(err) predicate
- LogBundlerError(lgr, err, msg, tags...) picks Warn for on-chain
reverts, Error for infra/AA failures
- Replaces the inline branches in vm_runner_contract_write.go and
vm_runner_eth_transfer.go
- Applies the same classification to the withdrawal UserOp failure site
in aggregator/rpc_server.go so user-level withdrawal reverts (e.g.
ERC20 transfer reverted) no longer page Sentry
- Tests cover nil, unrelated errors, AA-code infra errors, direct/wrapped
revert markers, and the nil-logger guard
Contributor
There was a problem hiding this comment.
Pull request overview
This PR reduces noisy Sentry alerts by downgrading expected user-workflow failures (simulated/on-chain reverts and per-task failure summaries) from Error to Warn, while keeping true infra/AA failures at Error via a new bundler error classifier in the ERC-4337 preset layer.
Changes:
- Add
preset.IsUserOpRevert+preset.LogBundlerErrorto classify on-chain UserOp reverts asWarnand keep infra/AA failures asError, with focused unit tests. - Replace several
logger.Error(...)calls withWarn(...)for expected simulation/workflow failure summaries and Tenderly revert outcomes. - Remove / downgrade noisy “CRITICAL DEBUG” logs around Tenderly return data handling.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/erc4337/preset/bundler_error.go | Introduces revert detection + severity-aware bundler error logging helper. |
| pkg/erc4337/preset/bundler_error_test.go | Adds unit tests validating classification and nil-logger safety. |
| core/taskengine/vm_runner_eth_transfer.go | Uses preset.LogBundlerError for ETH transfer UserOp failures to avoid paging on reverts. |
| core/taskengine/vm_runner_contract_write.go | Uses preset.LogBundlerError, removes noisy debug traces, and downgrades method-failure summary to Warn. |
| core/taskengine/tenderly_client.go | Downgrades simulation revert logs to Warn (expected workflow outcomes). |
| core/taskengine/executor.go | Downgrades task “completed with failures” summary from Error to Warn. |
| core/taskengine/engine.go | Downgrades workflow simulation failure summary from Error to Warn. |
| aggregator/rpc_server.go | Uses preset.LogBundlerError on withdrawal UserOp failure path for revert-vs-infra severity. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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
PR #463 bridged every
logger.Error(...)call to Sentry. That surfaced a pre-existing shape problem: manyError-level logs fire on expected user-workflow outcomes (simulated reverts, on-chain UserOp reverts, workflow-step failures), which were harmless before the bridge but now flood Sentry. The 2026-04-16 weekly review shows 14 issues all with the same culprit frame (pkg/logger func1) and all first-seen 2026-03-04 — the PR #463 deploy date.This PR:
Warnacross:tenderly_client.go— simulated revert (simulation.status=false, call-trace revert)vm_runner_contract_write.go— method-execution-failed summaryexecutor.go+engine.go— per-task/per-simulation completed-with-failures summariesCRITICAL DEBUGtraces invm_runner_contract_write.go— one of which (ReturnData is nil) was a top Sentry issue even thoughtenderly_client.goexplicitly nullsReturnDataon a reverted simulation (expected post-condition).preset.LogBundlerErrorto classify bundler errors:success=false in UserOperationEvent) go toWarn(the target contract reverted, not our bug).Errorand remain in Sentry.aggregator/rpc_server.go(previously unconditionallyError, now covered).Real infra failures (bundler down, AA21, storage writes, scheduler errors, etc.) keep paging via the untouched
logger.Errorsites.Test plan
go build ./pkg/logger/... ./pkg/erc4337/... ./core/taskengine/... ./aggregator/...cleango test ./pkg/logger/...passesgo test -run IsUserOpRevert^|LogBundlerError ./pkg/erc4337/preset/— 10 new subtests pass (nil, unrelated, AA21, AA25, direct marker, wrapped marker; Warn/Error dispatch; nil-logger guard)🤖 Generated with Claude Code