-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat(vercel): Add webhook handler for marketplace.invoice.paid (Slice 2b) #42879
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
base: master
Are you sure you want to change the base?
Conversation
Adds PostHog endpoint to receive webhooks from Vercel and forward billing events (marketplace.*) to the billing service. - Add /webhooks/vercel endpoint with HMAC-SHA1 signature verification - Add handle_billing_provider_webhook to BillingManager for forwarding - Support configurationId in various payload locations - Return 500 on errors to trigger Vercel retry 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Consolidate to single webhook URL following the /webhooks/<provider> convention, which is more standard for external webhooks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Companion PR: https://github.com/PostHog/billing/pull/1669 |
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.
4 files reviewed, 1 comment
Apply Clean Code principles to improve readability: - Extract _is_valid_signature() for signature verification - Extract _extract_config_id() for config ID lookup logic - Extract _is_billing_event() for event type checking - Extract _get_integration() for database lookup - Extract _forward_to_billing_service() for billing call - Remove implementation comments (code is self-documenting) - Flatten main function with early returns 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
afecd62 to
ddffa1d
Compare
Already applied in ee/urls.py when registering the route. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
ddffa1d to
1249ea3
Compare
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.
We're phasing ee out. You can add this to the main posthog folder.
Even better you can talk to the devex team to figure out where this kind of code should live. Ideally it should live inside the products folder but I have a hard time coming up with a "product" to file this under
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.
I don't have much context on the transition from ee to posthog but won't it cause issues for hobby installs to move it now on its own? At least until we complete the migration and have some other way to separate the APIs that are only relevant for cloud.
rafaeelaudibert
left a 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.
Will let Billing give you the final stamp
pawel-cebula
left a 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.
- Use
capture_exceptionfor Sentry error tracking
😮
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.
I don't have much context on the transition from ee to posthog but won't it cause issues for hobby installs to move it now on its own? At least until we complete the migration and have some other way to separate the APIs that are only relevant for cloud.
ee/api/vercel/vercel_webhooks.py
Outdated
|
|
||
| logger = structlog.get_logger(__name__) | ||
|
|
||
| BILLING_EVENT_PREFIX = "marketplace." |
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.
Do we want to forward all marketplace events if we're handling just one? Maybe also keep a list of speciifc events that's in sync with billing?
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Only check documented fields: payload.installationId and payload.configuration.id - Remove undocumented configurationId checks - Update tests to use installationId (per Vercel webhook API docs) - Add reference to Vercel docs in comment 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Both fields have the same value per docs, no need to check both. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Makes misconfiguration more visible via capture_exception. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Change URL from /api/billing-provider-webhook to /api/webhooks/billing-provider - Filter to only forward marketplace.invoice.paid (not all marketplace.* events) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
| secret = getattr(settings, "VERCEL_CLIENT_INTEGRATION_SECRET", None) | ||
| if not secret: | ||
| logger.error("vercel_webhook_missing_secret") | ||
| capture_exception(Exception("VERCEL_CLIENT_INTEGRATION_SECRET not configured"), {}) |
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.
@MattBro I don't believe just capturing an exception is enough for this. I'm not monitoring exceptions closely. The best solution here is something that feeds into incident.io. Nice to have, ofc, but we'll miss this if we never implement it. Add to the polishing list - if you arleady have one
rafaeelaudibert
left a 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.
Looks good from my end
Problem
When Vercel customers pay their invoices, Vercel sends a
marketplace.invoice.paidwebhook. We need to receive this and mark the corresponding Stripe invoice as paid so our billing flows work correctly.Changes
/webhooks/vercelendpoint with HMAC-SHA1 signature verificationmarketplace.*events to billing servicecapture_exceptionfor error trackingHow did you test this code?
E2E: Created Stripe invoice → finalized → Vercel webhook received → Stripe invoice marked paid.
🤖 Generated with Claude Code