fix: prevent startup crashes when Stripe/PostHog env vars are missing - #1719
Conversation
WalkthroughAdds a local PostHog disabled flag and updates its default key/host values; refactors Stripe usage across API and worker modules to a lazy, cached Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Pull request overview
This pull request prevents startup crashes when optional environment variables for Stripe and PostHog are missing. The changes implement lazy initialization for Stripe clients and use valid placeholder values for PostHog when the service is disabled, ensuring the API and worker processes can start in development environments without these services configured.
Changes:
- PostHog constructor now uses
"phc_placeholder"instead of"key"as the fallback API key to pass validation, and"https://localhost"for the host - Stripe client initialization changed from eager module-level instantiation to lazy initialization via
getStripe()function - All Stripe client references updated across both API and worker apps to use the new lazy getter
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/api/src/posthog.ts | Updated placeholder API key and host values to pass PostHog validation when disabled |
| apps/api/src/routes/payments.ts | Replaced eager Stripe instantiation with lazy getStripe() function, changed export |
| apps/api/src/stripe.ts | Updated all Stripe client references to use getStripe() |
| apps/api/src/routes/subscriptions.ts | Updated import and all Stripe client references to use getStripe() |
| apps/api/src/routes/dev-plans.ts | Updated import and all Stripe client references to use getStripe() |
| apps/worker/src/worker.ts | Implemented lazy getStripe() function and updated Stripe client reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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
🧹 Nitpick comments (1)
apps/worker/src/worker.ts (1)
45-59: DuplicatedgetStripe()singleton across app boundaries — consider extracting to a shared package.The
getStripe()implementation inworker.ts(lines 47–59) is byte-for-byte identical to the one exported fromapps/api/src/routes/payments.ts(lines 16–28), including the hardcodedapiVersion. Sinceapps/workerandapps/apiare separate applications, this duplication is structurally unavoidable without a shared package, but it means theapiVersionstring and error message need to stay in sync manually.Consider extracting this to a shared package (e.g.,
@llmgateway/stripeor@llmgateway/shared) so the singleton, API version constant, and error message live in one place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/worker/src/worker.ts` around lines 45 - 59, The getStripe singleton (_stripe, getStripe) is duplicated across apps; extract the logic, constants, and error message into a shared package (e.g., `@llmgateway/stripe` or `@llmgateway/shared`) and import it from both apps: move the STRIPE_SECRET_KEY check, the apiVersion string ("2025-04-30.basil"), and the getStripe function into the shared module, export a single getStripe and any related constants, and update apps/worker/src/worker.ts and apps/api/src/routes/payments.ts to import and use that shared getStripe instead of maintaining local copies.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/src/stripe.ts`:
- Line 10: There is a circular module dependency because getStripe (and the
internal _stripe) were imported from routes/payments.ts while payments.ts also
imports ensureStripeCustomer from stripe.ts; to fix it, extract the stripe
client logic (the _stripe variable and the getStripe function implementation)
into a new standalone module (e.g., stripe-client.ts) that does not import from
stripe.ts, export getStripe (and any helpers needed), then update both stripe.ts
(where ensureStripeCustomer lives) and routes/payments.ts to import getStripe
from the new stripe-client module instead of importing across each other; ensure
no other top-level imports from stripe.ts remain in the new module so the cycle
is broken.
---
Nitpick comments:
In `@apps/worker/src/worker.ts`:
- Around line 45-59: The getStripe singleton (_stripe, getStripe) is duplicated
across apps; extract the logic, constants, and error message into a shared
package (e.g., `@llmgateway/stripe` or `@llmgateway/shared`) and import it from both
apps: move the STRIPE_SECRET_KEY check, the apiVersion string
("2025-04-30.basil"), and the getStripe function into the shared module, export
a single getStripe and any related constants, and update
apps/worker/src/worker.ts and apps/api/src/routes/payments.ts to import and use
that shared getStripe instead of maintaining local copies.
…startup crashes PostHog and Stripe constructors now validate their arguments more strictly, causing the API and worker to crash at module load time when env vars are missing. Supervisord kept restarting them silently, masking the failures. - PostHog: use "phc_placeholder" instead of "key" to pass non-empty string validation - Stripe: lazy-initialize via getStripe() so the client is only created on first use Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
c36c3f1 to
467c86a
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/worker/src/worker.ts (1)
45-59:getStripe()is duplicated fromapps/api/src/routes/payments.ts.The implementation — including the
_stripecache,STRIPE_SECRET_KEYvalidation, andapiVersion— is identical in both files. Any future change (e.g., constructor options, API version bump) must be made in two places.Consider centralising this in a shared monorepo package (e.g.,
@llmgateway/stripe) that both apps can depend on. Since the worker already consumes other@llmgateway/*packages this would be a natural fit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/worker/src/worker.ts` around lines 45 - 59, Duplicate Stripe initialization logic (the _stripe cache, getStripe(), STRIPE_SECRET_KEY validation and apiVersion) should be moved into a single shared module and imported where needed; create a new package (e.g., export a function createStripeClient or getStripe from a shared `@llmgateway/stripe` package) that encapsulates the singleton, env validation, and apiVersion, replace the local getStripe and _stripe usages with imports of that shared function in both places that currently duplicate the logic, and update imports so only the shared symbol is used going forward.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/worker/src/worker.ts`:
- Around line 45-59: Duplicate Stripe initialization logic (the _stripe cache,
getStripe(), STRIPE_SECRET_KEY validation and apiVersion) should be moved into a
single shared module and imported where needed; create a new package (e.g.,
export a function createStripeClient or getStripe from a shared
`@llmgateway/stripe` package) that encapsulates the singleton, env validation, and
apiVersion, replace the local getStripe and _stripe usages with imports of that
shared function in both places that currently duplicate the logic, and update
imports so only the shared symbol is used going forward.
Align with lazy Stripe initialization from PR #1719 that was merged to main while this branch was in development. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
disabled: true. Changed the fallback from"key"to"phc_placeholder"which passes validation.new Stripe(...)at module load time with lazygetStripe()initialization. The Stripe client is only created on first actual use, so missingSTRIPE_SECRET_KEYno longer crashes the process at startup.apps/api(posthog.ts, payments.ts, stripe.ts, dev-plans.ts, subscriptions.ts) andapps/worker(worker.ts).Test plan
POSTHOG_KEY,POSTHOG_HOST, orSTRIPE_SECRET_KEYsetSTRIPE_SECRET_KEYis providedPOSTHOG_KEYandPOSTHOG_HOSTare provided🤖 Generated with Claude Code
Summary by CodeRabbit