fix(billing): log + surface root cause when checkout 502s#3660
Conversation
Add logger.error in the checkout and extrasCheckout controller catch blocks (502 path only) and wrap stripe.customers.create / stripe.checkout.sessions.create / stripe.checkout.sessions.create (extras) with explicit try/catch + logger.error so the underlying Stripe error message appears in pod logs before being re-thrown. Without this, production 502s from the billing checkout flow had no stack trace in pod logs — root cause was invisible. Regression tests: billing.checkout.unit.tests.js — three new tests covering stripe.customers.create reject, stripe.checkout.sessions.create reject (subscription), and stripe.checkout.sessions.create reject (extras pack), each asserting logger.error is called with the correct marker + re-thrown. Also adds logger mock to billing.extras.controller and billing.checkout test suites to prevent config-read crash on minimal config objects.
Winston format.json() does not serialize Error non-enumerable properties,
so logging `error: err` produces `{}` in JSON mode. Switch to
`error: err?.message ?? String(err), stack: err?.stack` to match the
existing convention in billing.webhook.service.js. Update regression test
assertions to verify the extracted message string.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3660 +/- ##
==========================================
+ Coverage 89.20% 89.39% +0.19%
==========================================
Files 136 136
Lines 4669 4687 +18
Branches 1452 1459 +7
==========================================
+ Hits 4165 4190 +25
+ Misses 392 388 -4
+ Partials 112 109 -3
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR adds structured error logging around Stripe operations in the billing module. The billing service now wraps Stripe API calls (customer creation, checkout session creation) in try/catch blocks that log context and re-throw. The controller endpoints conditionally emit error logs for 502 failures. Tests mock the logger and verify the error logging behavior. ChangesStripe Error Logging in Service and Controller
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
There was a problem hiding this comment.
Pull request overview
This PR improves observability for Stripe-related failures during billing checkout flows by adding explicit error logging in the billing controller and by wrapping key Stripe API calls in the billing service so failures are logged before being re-thrown. It also updates unit tests to cover the new error-logging paths and to mock the logger module during ESM imports.
Changes:
- Add
logger.error(...)inbilling.controller.jsfor checkout/extrasCheckout when the handler returns 502. - Wrap
stripe.customers.createandstripe.checkout.sessions.createcalls inbilling.service.jswith try/catch logging before re-throwing. - Add/extend unit tests and ESM mocks to exercise the new error-logging paths without importing the real logger in tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| modules/billing/services/billing.service.js | Adds try/catch logging around Stripe customer/session creation paths. |
| modules/billing/controllers/billing.controller.js | Logs 502 checkout failures with request/org context before returning the standard error envelope. |
| modules/billing/tests/billing.checkout.unit.tests.js | Adds regression tests asserting error logging occurs when Stripe calls reject; adds logger mocking. |
| modules/billing/tests/billing.extras.controller.unit.tests.js | Adds logger mocking to prevent logger/config initialization issues during controller import. |
| ); | ||
| } catch (err) { | ||
| logger.error('[billing.service] stripe.customers.create failed', { | ||
| error: err?.message ?? String(err), |
| error: err?.message ?? String(err), | ||
| stack: err?.stack, |
| ); | ||
| } catch (err) { | ||
| logger.error('[billing.service] stripe.checkout.sessions.create (extras) failed', { | ||
| error: err?.message ?? String(err), |
| const status = err.message?.startsWith('Invalid') || err.message?.includes('not found') ? 422 : 502; | ||
| if (status === 502) { | ||
| logger.error('[billing.checkout] createCheckout failed', { | ||
| error: err?.message ?? String(err), |
| const status = err.message?.startsWith('Invalid') || err.message?.includes('not found') ? 422 : 502; | ||
| if (status === 502) { | ||
| logger.error('[billing.checkout] createExtrasCheckout failed', { | ||
| error: err?.message ?? String(err), |
| expect(mockLogger.error).toHaveBeenCalledWith( | ||
| '[billing.service] stripe.customers.create failed', | ||
| expect.objectContaining({ | ||
| organizationId: orgId, | ||
| error: 'Stripe Test: customer creation blocked', | ||
| }), | ||
| ); |
| expect(mockLogger.error).toHaveBeenCalledWith( | ||
| '[billing.service] stripe.checkout.sessions.create failed', | ||
| expect.objectContaining({ | ||
| organizationId: orgId, | ||
| priceId: 'price_starter_m', | ||
| error: 'Stripe Test: session creation blocked', | ||
| }), | ||
| ); |
| expect(mockLogger.error).toHaveBeenCalledWith( | ||
| '[billing.service] stripe.checkout.sessions.create (extras) failed', | ||
| expect.objectContaining({ | ||
| organizationId: orgId, | ||
| packId: 'pack_500k', | ||
| error: 'Stripe Test: extras session blocked', | ||
| }), | ||
| ); |
Summary
logger.error('[billing.checkout] ...')in the checkout controller catch + Stripe call wrappers in_ensureStripeCustomerandcreateCheckoutWhy
POST /api/billing/checkoutreturns 502 with no stack trace. Pod logs only showstatus 502 ms 154— no error context. This change does NOT fix the underlying 502 — it surfaces the root cause so the fix can target the right layer (likely Stripe secret rotation, stale customer, or price-id mismatch).Test plan
billing.checkout.unit.tests.js(115 lines new)test@gmail.comon prod → grep pod logs for[billing.checkout] createCheckout failed→ identify root cause → file follow-up PRFollow-up
The actual 502 fix PR (
WS-A T1b) will be filed once the underlying error is identified from prod logs after this PR deploys.Summary by CodeRabbit