🥅 server: handle misc panda errors from sentry#736
Conversation
🦋 Changeset detectedLatest commit: 5fe25ec 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 |
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 significantly enhances the server's ability to process Panda card tokenization webhooks. It introduces specific handling for card notification events, particularly those indicating provisioning declines, by updating the payload validation and user identification logic. The changes ensure that the system can correctly interpret and respond to these critical events, improving the overall reliability and functionality of the Panda integration. 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
|
WalkthroughAdds handling for missing Panda users in the card GET endpoint (returns 403 when panda user lookup yields 404), introduces a Card payload variant and notification handling in the Panda webhook flow with credential-based user resolution, and adds tests for both API and webhook behaviors. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(220,220,255,0.5)
participant Webhook as Webhook Receiver
participant Handler as Panda Hook Handler
participant Cred as Credentials Store
participant Panda as Panda Service (getUser)
end
Webhook->>Handler: POST Card notification (resource/action/body)
Handler->>Handler: derive pandaId from Card.notification or Transaction
Handler->>Cred: query credential provider:"panda", providerId:pandaId
alt credential found
Handler->>Panda: getUser(pandaId)
alt user found
Panda->>Handler: user
Handler->>Handler: process with user
else user 404
Panda->>Handler: 404
Handler->>Handler: treat as missing user (no user)
end
else no credential
Cred-->>Handler: no credential
Handler->>Handler: skip user resolution
end
Handler->>Webhook: respond 200 { code: "ok" } or handle error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
Sentry Issue: SERVER-H7 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #736 +/- ##
==========================================
+ Coverage 67.21% 67.28% +0.07%
==========================================
Files 206 206
Lines 6902 6908 +6
Branches 2143 2147 +4
==========================================
+ Hits 4639 4648 +9
+ Misses 2081 2077 -4
- Partials 182 183 +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:
|
cruzdanilo
force-pushed
the
panda
branch
2 times, most recently
from
7131ed6 to
65a2c96
Compare
![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
This pull request successfully adds handling for the Panda card tokenization notification webhook. The changes include updating the payload validation schema to include the new notification action for the card resource, and adjusting the logic to correctly extract the user ID from various webhook payloads. New tests have been added to cover the new functionality, including cases with known, unknown, and null user IDs. My review includes one suggestion to improve the readability and maintainability of the logic that extracts the user ID.
cruzdanilo
changed the title
|
Sentry Issue: SERVER-FF |
|
@cursor review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@server/hooks/panda.ts`:
- Around line 222-234: The where clause in database.query.credentials.findFirst
uses a redundant and(eq(credentials.pandaId, pandaId)) wrapper; replace it with
the single predicate eq(credentials.pandaId, pandaId) directly (update the call
in the same block that computes pandaId and calls
database.query.credentials.findFirst) so the where option passes
eq(credentials.pandaId, pandaId) instead of and(...).
f48236b to
4f15be1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@server/test/hooks/panda.test.ts`:
- Around line 1259-1278: Spy on Sentry.setUser in the "returns ok with known
user" test, perform the request via appClient.index.$post as you already do,
then assert the spy was called with the credential id (e.g., { id: "cred" }) to
verify Sentry context enrichment; use Sentry.setUser spy/ mock and restore/reset
it after the test so other tests remain unaffected.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@server/test/hooks/panda.test.ts`:
- Around line 1281-1321: Add negative assertions to both "returns ok with null
userId" and "returns ok with unknown userId" tests to confirm setUser was not
called: after the existing response assertions in the it blocks (the ones that
expect status 200 and JSON { code: "ok" }), add
expect(setUser).not.toHaveBeenCalled() so the tests assert no user context is
set when card.userId is null or unresolvable.
| it("returns ok with null userId", async () => { | ||
| const response = await appClient.index.$post({ | ||
| header: { signature: "panda-signature" }, | ||
| json: { | ||
| resource: "card", | ||
| action: "notification", | ||
| id: "webhook-id", | ||
| body: { | ||
| id: "notification-id", | ||
| card: { id: "card", userId: null }, | ||
| tokenWallet: "Apple", | ||
| reasonCode: "PROVISIONING_DECLINED", | ||
| decisionReason: { code: "WALLET_PROVIDER_RISK_THRESHOLD_EXCEEDED", description: "declined" }, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| await expect(response.json()).resolves.toStrictEqual({ code: "ok" }); | ||
| }); | ||
|
|
||
| it("returns ok with unknown userId", async () => { | ||
| const response = await appClient.index.$post({ | ||
| header: { signature: "panda-signature" }, | ||
| json: { | ||
| resource: "card", | ||
| action: "notification", | ||
| id: "webhook-id", | ||
| body: { | ||
| id: "notification-id", | ||
| card: { id: "card", userId: "unknown" }, | ||
| tokenWallet: "Apple", | ||
| reasonCode: "PROVISIONING_DECLINED", | ||
| decisionReason: { code: "WALLET_PROVIDER_RISK_THRESHOLD_EXCEEDED", description: "declined" }, | ||
| }, | ||
| }, | ||
| }); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| await expect(response.json()).resolves.toStrictEqual({ code: "ok" }); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding negative assertions for setUser in the null/unknown userId tests.
Tests 2 and 3 verify the 200 response but don't assert that setUser was not called. Adding expect(setUser).not.toHaveBeenCalled() would strengthen the tests by confirming no spurious user context is set when pandaId is null or unresolvable.
Proposed additions
For "returns ok with null userId" (after line 1299):
expect(response.status).toBe(200);
await expect(response.json()).resolves.toStrictEqual({ code: "ok" });
+ expect(setUser).not.toHaveBeenCalled();For "returns ok with unknown userId" (after line 1320):
expect(response.status).toBe(200);
await expect(response.json()).resolves.toStrictEqual({ code: "ok" });
+ expect(setUser).not.toHaveBeenCalled();🤖 Prompt for AI Agents
In `@server/test/hooks/panda.test.ts` around lines 1281 - 1321, Add negative
assertions to both "returns ok with null userId" and "returns ok with unknown
userId" tests to confirm setUser was not called: after the existing response
assertions in the it blocks (the ones that expect status 200 and JSON { code:
"ok" }), add expect(setUser).not.toHaveBeenCalled() so the tests assert no user
context is set when card.userId is null or unresolvable.
1 participant
sentry issues:
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores