🥅 server: skip sentry capture for expected liquidity errors#727
🥅 server: skip sentry capture for expected liquidity errors#727cruzdanilo wants to merge 1 commit intomainfrom
Conversation
🦋 Changeset detectedLatest commit: 8ae07a1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ Finishing touches🧪 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 |
Summary of ChangesHello @cruzdanilo, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the error reporting mechanism within the server's Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request successfully implements the logic to selectively skip Sentry error capture for expected liquidity errors, specifically InsufficientAccountLiquidity (status code 557). The changes ensure that only truly unhandled or unexpected contract errors are reported to Sentry, reducing noise from known and handled conditions. The accompanying tests validate this behavior, confirming that captureException is not called for InsufficientAccountLiquidity errors, but is correctly called for other contract errors like Replay or generic transaction reverts. This is a good improvement for error monitoring and clarity.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| vi.spyOn(traceClient, "traceCall").mockResolvedValue({ ...callFrame, output: "0xb5a78004" }); | ||
|
|
||
| await database.insert(cards).values([{ id: "replay", credentialId: "cred", lastFour: "2222", mode: 4 }]); | ||
|
|
||
| const response = await appClient.index.$post({ | ||
| ...authorization, | ||
| json: { | ||
| ...authorization.json, |
There was a problem hiding this comment.
Bug: The error handler in server/hooks/panda.ts uses "InsufficientAccountLiquidity" but the contract throws "InsufficientLiquidity", causing incorrect error handling for liquidity failures.
Severity: HIGH
Suggested Fix
In server/hooks/panda.ts at line 352, change the case "InsufficientAccountLiquidity" to "InsufficientLiquidity" to match the custom error name defined and thrown by the Solidity contract.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: server/test/hooks/panda.test.ts#L147-L154
Potential issue: The error handler in `server/hooks/panda.ts` at line 352 incorrectly
checks for the error name `"InsufficientAccountLiquidity"`. The corresponding Solidity
contract actually throws an error named `"InsufficientLiquidity"`. This mismatch causes
the `switch` statement to fail, leading to a fall-through case. As a result,
transactions failing due to insufficient liquidity will incorrectly return a generic
`PandaError` with status code 550 instead of the intended specific error with status
code 557. This also causes these expected errors to be unnecessarily captured by Sentry,
creating noise.
Did we get this right? 👍 / 👎 to inform future reviews.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #727 +/- ##
==========================================
+ Coverage 68.05% 68.07% +0.01%
==========================================
Files 206 206
Lines 6784 6785 +1
Branches 2117 2118 +1
==========================================
+ Hits 4617 4619 +2
Misses 1986 1986
+ Partials 181 180 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
closes #663