fix(marketing): prevent env validation from blocking React hydration#3388
fix(marketing): prevent env validation from blocking React hydration#3388michalkopanski wants to merge 1 commit into
Conversation
NEXT_PUBLIC_POSTHOG_HOST had no default, causing createEnv to throw
during module init when the var was absent. Since instrumentation-client
runs before Next.js calls hydrate(), this silently prevented all of
React from mounting — animations stayed at opacity 0, hooks never ran.
- Add .default("https://us.posthog.com") to NEXT_PUBLIC_POSTHOG_HOST
- Wrap posthog.init and Sentry.init in try/catch as a secondary safeguard
- Add apps/marketing/.env.example documenting all required vars
📝 WalkthroughWalkthroughThe marketing app gains comprehensive environment configuration with server and client-side secrets, integrations (Better Auth, Resend, Stripe, Slack, Sentry, Anthropic), and analytics setup. Client-side instrumentation initialization for PostHog and Sentry is wrapped in error handlers to prevent runtime failures. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Greptile SummaryThis PR fixes a real hydration-blocking bug on the marketing site: Key points:
Confidence Score: 4/5Safe to merge for the primary fix; the secondary resilience claim in the QA checklist is inaccurate but does not introduce a regression. The root cause fix is correct and targeted. The try/catch guards add value for SDK-internal errors. The only gap is that QA checklist item #3 implies apps/marketing/src/instrumentation-client.ts — the relationship between the module-scope env import and the try/catch scope needs attention if full resilience to missing env vars is desired. Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant NextJS as Next.js Runtime
participant InstrClient as instrumentation-client.ts
participant EnvModule as env.ts (createEnv)
participant PostHog
participant Sentry
Browser->>NextJS: Load marketing page
NextJS->>InstrClient: Import module
InstrClient->>EnvModule: import { env } [module scope]
alt NEXT_PUBLIC_POSTHOG_HOST missing (before fix)
EnvModule-->>InstrClient: throws ZodError
InstrClient-->>NextJS: Module init fails
NextJS-->>Browser: Hydration blocked
else NEXT_PUBLIC_POSTHOG_HOST missing (after fix)
EnvModule-->>InstrClient: returns env with default host
InstrClient->>PostHog: posthog.init() inside try/catch
PostHog-->>InstrClient: initialized
InstrClient->>Sentry: Sentry.init() inside try/catch
Sentry-->>InstrClient: initialized
InstrClient-->>NextJS: Module ready
NextJS-->>Browser: Hydration succeeds
end
Note over InstrClient,EnvModule: NEXT_PUBLIC_POSTHOG_KEY still required —
Note over InstrClient,EnvModule: missing key still throws at module scope
Note over InstrClient,EnvModule: before any try/catch runs
Reviews (1): Last reviewed commit: "fix(marketing): prevent env validation f..." | Re-trigger Greptile |
| try { | ||
| posthog.init(env.NEXT_PUBLIC_POSTHOG_KEY, { | ||
| api_host: "/ingest", | ||
| ui_host: "https://us.posthog.com", | ||
| defaults: "2025-11-30", | ||
| capture_pageview: "history_change", | ||
| capture_pageleave: true, | ||
| capture_exceptions: true, | ||
| debug: false, | ||
| cross_subdomain_cookie: true, | ||
| persistence: "cookie", | ||
| persistence_name: POSTHOG_COOKIE_NAME, | ||
| disable_session_recording: true, | ||
| loaded: (posthog) => { | ||
| posthog.register({ | ||
| app_name: "marketing", | ||
| domain: window.location.hostname, | ||
| }); | ||
|
|
||
| const consent = localStorage.getItem(ANALYTICS_CONSENT_KEY); | ||
| if (consent === "declined") { | ||
| posthog.opt_out_capturing(); | ||
| } | ||
| }, | ||
| }); | ||
| const consent = localStorage.getItem(ANALYTICS_CONSENT_KEY); | ||
| if (consent === "declined") { | ||
| posthog.opt_out_capturing(); | ||
| } | ||
| }, | ||
| }); | ||
| } catch (e) { | ||
| console.warn("PostHog failed to initialize", e); | ||
| } |
There was a problem hiding this comment.
try/catch does not protect against a missing PostHog key env var
The PR's QA checklist claims that removing NEXT_PUBLIC_POSTHOG_KEY should log a console.warn instead of crashing the page — but the code does not support this.
Line 5 (import { env } from "@/env") executes at module scope, before any try/catch. If NEXT_PUBLIC_POSTHOG_KEY is absent, createEnv throws during that top-level import, and the module never reaches the try block below. The try/catch only guards against posthog.init itself throwing (e.g. a bad key format or internal SDK error), not against missing-variable failures from createEnv.
To match the documented QA behavior, NEXT_PUBLIC_POSTHOG_KEY would also need to be made optional in env.ts (similar to how NEXT_PUBLIC_POSTHOG_HOST now has a default), and the posthog.init call should guard on its presence before passing it through.
Without this, QA checklist item #3 will fail when tested, and the same silent hydration block described in the PR could recur if the PostHog key var is ever absent from a deployment.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/marketing/src/instrumentation-client.ts (1)
11-11: Useenv.NEXT_PUBLIC_POSTHOG_HOSTinstead of hardcodingui_host.Line 11 duplicates the default from
env.tsand bypasses env-based overrides. Wiring this toenv.NEXT_PUBLIC_POSTHOG_HOSTkeeps one source of truth.♻️ Proposed change
- ui_host: "https://us.posthog.com", + ui_host: env.NEXT_PUBLIC_POSTHOG_HOST,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/marketing/src/instrumentation-client.ts` at line 11, Replace the hardcoded posthog host in the instrumentation client by reading the environment variable instead of the literal "https://us.posthog.com": locate where ui_host is set (property ui_host in apps/marketing/src/instrumentation-client.ts) and change it to use env.NEXT_PUBLIC_POSTHOG_HOST (or the equivalent exported env object used across the project) so the value is driven by environment/configuration rather than a duplicated hardcoded default.apps/marketing/.env.example (1)
2-21: Optional cleanup: reorder keys to silencedotenv-linterUnorderedKeywarnings.This file is functionally fine, but current ordering triggers repeated lint warnings. If dotenv-linter is part of CI gating/noise reduction, sorting keys (or grouping in the expected order) will keep checks clean.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/marketing/.env.example` around lines 2 - 21, The .env example triggers dotenv-linter UnorderedKey warnings because keys are not consistently sorted; reorder the variables to a deterministic order (e.g., alphabetically) to silence the linter: sort the top-level server keys including ANTHROPIC_API_KEY, BETTER_AUTH_SECRET, KV_REST_API_TOKEN, KV_REST_API_URL, RESEND_API_KEY, SENTRY_AUTH_TOKEN, SLACK_BILLING_WEBHOOK_URL, STRIPE_PRO_MONTHLY_PRICE_ID, STRIPE_PRO_YEARLY_PRICE_ID, STRIPE_SECRET_KEY, STRIPE_WEBHOOK_SECRET, and keep the Client block (NEXT_PUBLIC_API_URL, NEXT_PUBLIC_POSTHOG_HOST, NEXT_PUBLIC_POSTHOG_KEY, NEXT_PUBLIC_SENTRY_DSN_MARKETING, NEXT_PUBLIC_SENTRY_ENVIRONMENT, NEXT_PUBLIC_WEB_URL) sorted as well so dotenv-linter no longer reports UnorderedKey.apps/marketing/src/env.ts (1)
28-29:NEXT_PUBLIC_POSTHOG_KEYis defined as required but unused; making it optional is reasonable for resilience.
NEXT_PUBLIC_POSTHOG_KEY: z.string()at line 28 is required in the schema. However, the variable is never actually consumed in active code paths—PostHog initialization inProviders.tsxbypasses it entirely, andinstrumentation-client.ts(which does reference it) is dead code.While the env schema is validated when
CTAButtons.tsximports it (which runs at app startup), the unused required key adds unnecessary coupling. Making it optional would align with the resilience goal.🔧 Suggested schema adjustment
- NEXT_PUBLIC_POSTHOG_KEY: z.string(), + NEXT_PUBLIC_POSTHOG_KEY: z.string().optional(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/marketing/src/env.ts` around lines 28 - 29, The env schema currently declares NEXT_PUBLIC_POSTHOG_KEY as required (NEXT_PUBLIC_POSTHOG_KEY: z.string()) even though it's unused; change that schema entry to be optional or provide a sensible default so startup validation doesn't require the key. Update the symbol NEXT_PUBLIC_POSTHOG_KEY in apps/marketing/src/env.ts to z.string().optional() (or z.string().default("") if preferred) so imports like CTAButtons.tsx won't fail, and leave PostHog initialization in Providers.tsx and the dead instrumentation-client.ts code unchanged.
🤖 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/marketing/.env.example`:
- Around line 2-21: The .env example triggers dotenv-linter UnorderedKey
warnings because keys are not consistently sorted; reorder the variables to a
deterministic order (e.g., alphabetically) to silence the linter: sort the
top-level server keys including ANTHROPIC_API_KEY, BETTER_AUTH_SECRET,
KV_REST_API_TOKEN, KV_REST_API_URL, RESEND_API_KEY, SENTRY_AUTH_TOKEN,
SLACK_BILLING_WEBHOOK_URL, STRIPE_PRO_MONTHLY_PRICE_ID,
STRIPE_PRO_YEARLY_PRICE_ID, STRIPE_SECRET_KEY, STRIPE_WEBHOOK_SECRET, and keep
the Client block (NEXT_PUBLIC_API_URL, NEXT_PUBLIC_POSTHOG_HOST,
NEXT_PUBLIC_POSTHOG_KEY, NEXT_PUBLIC_SENTRY_DSN_MARKETING,
NEXT_PUBLIC_SENTRY_ENVIRONMENT, NEXT_PUBLIC_WEB_URL) sorted as well so
dotenv-linter no longer reports UnorderedKey.
In `@apps/marketing/src/env.ts`:
- Around line 28-29: The env schema currently declares NEXT_PUBLIC_POSTHOG_KEY
as required (NEXT_PUBLIC_POSTHOG_KEY: z.string()) even though it's unused;
change that schema entry to be optional or provide a sensible default so startup
validation doesn't require the key. Update the symbol NEXT_PUBLIC_POSTHOG_KEY in
apps/marketing/src/env.ts to z.string().optional() (or z.string().default("") if
preferred) so imports like CTAButtons.tsx won't fail, and leave PostHog
initialization in Providers.tsx and the dead instrumentation-client.ts code
unchanged.
In `@apps/marketing/src/instrumentation-client.ts`:
- Line 11: Replace the hardcoded posthog host in the instrumentation client by
reading the environment variable instead of the literal
"https://us.posthog.com": locate where ui_host is set (property ui_host in
apps/marketing/src/instrumentation-client.ts) and change it to use
env.NEXT_PUBLIC_POSTHOG_HOST (or the equivalent exported env object used across
the project) so the value is driven by environment/configuration rather than a
duplicated hardcoded default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5b192fa5-9067-47aa-8d70-b0e9c351b10d
📒 Files selected for processing (3)
apps/marketing/.env.exampleapps/marketing/src/env.tsapps/marketing/src/instrumentation-client.ts
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/marketing/src/instrumentation-client.ts">
<violation number="1" location="apps/marketing/src/instrumentation-client.ts:8">
P1: `try/catch` does not protect against a missing `NEXT_PUBLIC_POSTHOG_KEY`. The `import { env } from "@/env"` on line 5 runs at module evaluation time — before this `try` block is reached. If `NEXT_PUBLIC_POSTHOG_KEY` is absent, `createEnv` throws during the import, killing hydration exactly like the original bug. To actually guard against this, either make `NEXT_PUBLIC_POSTHOG_KEY` optional with a default/guard in `env.ts`, or lazily access `env` inside the `try` block (e.g., via a dynamic import or deferred access pattern).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| app_name: "marketing", | ||
| domain: window.location.hostname, | ||
| }); | ||
| try { |
There was a problem hiding this comment.
P1: try/catch does not protect against a missing NEXT_PUBLIC_POSTHOG_KEY. The import { env } from "@/env" on line 5 runs at module evaluation time — before this try block is reached. If NEXT_PUBLIC_POSTHOG_KEY is absent, createEnv throws during the import, killing hydration exactly like the original bug. To actually guard against this, either make NEXT_PUBLIC_POSTHOG_KEY optional with a default/guard in env.ts, or lazily access env inside the try block (e.g., via a dynamic import or deferred access pattern).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/marketing/src/instrumentation-client.ts, line 8:
<comment>`try/catch` does not protect against a missing `NEXT_PUBLIC_POSTHOG_KEY`. The `import { env } from "@/env"` on line 5 runs at module evaluation time — before this `try` block is reached. If `NEXT_PUBLIC_POSTHOG_KEY` is absent, `createEnv` throws during the import, killing hydration exactly like the original bug. To actually guard against this, either make `NEXT_PUBLIC_POSTHOG_KEY` optional with a default/guard in `env.ts`, or lazily access `env` inside the `try` block (e.g., via a dynamic import or deferred access pattern).</comment>
<file context>
@@ -5,41 +5,49 @@ import posthog from "posthog-js";
- app_name: "marketing",
- domain: window.location.hostname,
- });
+try {
+ posthog.init(env.NEXT_PUBLIC_POSTHOG_KEY, {
+ api_host: "/ingest",
</file context>
Summary
.default("https://us.posthog.com")toNEXT_PUBLIC_POSTHOG_HOSTinenv.ts— it's a fixed public URL with no reason to vary per environmentposthog.initandSentry.initintry/catchininstrumentation-client.tsso a bad analytics key can't block hydrationapps/marketing/.env.exampledocumenting all required varsWhy / Context
NEXT_PUBLIC_POSTHOG_HOSTwas a required field increateEnvwith no default. When the var was missing,createEnvthrew during module initialization — before Next.js calledhydrate(). This silently prevented all of React from mounting: framer-motion animations stayed at their initial state (logo opacity 0),useEffect/useStatehooks never ran (TypewriterText stayed empty), with no obvious error visible on the page.The
skipValidationescape hatch (process.env.SKIP_ENV_VALIDATION) couldn't help here because it's not aNEXT_PUBLIC_*var, so it's never available in the browser — validation always ran client-side regardless.How It Works
NEXT_PUBLIC_POSTHOG_HOSTis the PostHog ingestion endpoint. The public US cluster (https://us.posthog.com) is the correct default for all environments — there's no scenario where this should point elsewhere. Making it optional-with-default means the schema still validates it's a valid URL, but missing env no longer throws.The
try/catcharoundposthog.initandSentry.initis a secondary safeguard: if either initializer throws for any other reason (bad key format, network-unavailable-at-init, etc.), it logs a warning instead of propagating and killing hydration.Manual QA Checklist
NEXT_PUBLIC_POSTHOG_HOSTunset — animations play, TypewriterText rendersNEXT_PUBLIC_POSTHOG_HOSTset — PostHog uses the provided hostNEXT_PUBLIC_POSTHOG_KEYlogs aconsole.warninstead of crashing the pageTesting
bun run typecheckbun run lintNotes
apps/marketing/.env.exampleneededgit add -fbecause.gitignorehas a broad.env.*pattern. The root.env.examplewas already force-tracked the same way.Summary by cubic
Fixes a marketing-page hydration failure caused by client env validation. Sets a safe default for
NEXT_PUBLIC_POSTHOG_HOSTand hardens analytics init so missing/bad keys don’t block React.NEXT_PUBLIC_POSTHOG_HOSTdefault tohttps://us.posthog.cominenv.tsto prevent client-side schema throws before hydration.posthog.initandSentry.initin try/catch to warn instead of crashing when keys/hosts are misconfigured.apps/marketing/.env.exampledocumenting required server and client vars.Written for commit b126a56. Summary will update on new commits.
Summary by CodeRabbit
Chores
Bug Fixes