-
Notifications
You must be signed in to change notification settings - Fork 1.5k
fix: prevent PostHog console errors when API key not configured #2739
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: main
Are you sure you want to change the base?
Conversation
@reddygtvs is attempting to deploy a commit to the Onlook Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughAdds a hasValidKey guard to validate NEXT_PUBLIC_POSTHOG_KEY, conditionally initializes PostHog and user identification based on key validity, updates effect dependencies to include hasValidKey, and returns children early (skipping provider) when the key is invalid. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Provider as PostHogProvider
participant SDK as PostHog SDK
participant Auth as User Context
App->>Provider: Render with children
Provider->>Provider: Compute hasValidKey
alt hasValidKey is true
Provider->>SDK: posthog.init()
Auth-->>Provider: user (if available)
alt user present
Provider->>SDK: posthog.identify(user)
else user absent
Note over Provider: Skip identification
end
Provider-->>App: Render <PostHogProvider>{children}</PostHogProvider>
else hasValidKey is false
Note over Provider: Early return (no SDK, no provider)
Provider-->>App: Render children directly
end
Note over Provider: Effects depend on hasValidKey (and user)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/client/src/app/layout.tsx (1)
79-94
: Provider ordering is safe. FeatureFlagsProvider does not import or consume any PostHog context, so wrapping ConditionalPostHogProvider inside FeatureFlagsProvider has no hidden dependencies.Optional: you can further gate analytics in non-production by checking
env.NODE_ENV === 'production'
(or a similar flag) before rendering ConditionalPostHogProvider to reduce noise and bundle work in dev/preview.apps/web/client/src/components/conditional-posthog-provider.tsx (1)
1-17
: Avoid eager import of PostHogProvider to prevent accidental side effects and unnecessary bundle weight; simplify key validation.
- If PostHogProvider (or the SDK it imports) has module-scope side effects, a static import will run them even when the key is invalid. Use next/dynamic so the module is only loaded when we actually render it.
- Normalize the key once for clarity and safer checks.
Apply:
-"use client" - -import { env } from "@/env" -import { PostHogProvider } from "./posthog-provider" - -export const ConditionalPostHogProvider = ({ children }: { children: React.ReactNode }) => { - const hasValidKey = env.NEXT_PUBLIC_POSTHOG_KEY && - env.NEXT_PUBLIC_POSTHOG_KEY.trim() !== '' && - !env.NEXT_PUBLIC_POSTHOG_KEY.includes('<Your PostHog API key') && - !env.NEXT_PUBLIC_POSTHOG_KEY.startsWith('<') - - if (hasValidKey) { - return <PostHogProvider>{children}</PostHogProvider> - } - - return <>{children}</> -} +\"use client\" + +import dynamic from "next/dynamic" +import { env } from "@/env" +import React from "react" + +// Load the provider lazily only when actually used; also disables SSR for the client SDK. +const LazyPostHogProvider = dynamic( + () => import("./posthog-provider").then((m) => m.PostHogProvider), + { ssr: false } +) + +export const ConditionalPostHogProvider = ({ children }: { children: React.ReactNode }) => { + const key = env.NEXT_PUBLIC_POSTHOG_KEY?.trim() + const hasValidKey = + !!key && + key !== "" && + !key.includes("<Your PostHog API key") && + !key.startsWith("<") + + if (!hasValidKey) return <>{children}</> + return <LazyPostHogProvider>{children}</LazyPostHogProvider> +}Optional tightening (if you want to accept only likely-real PostHog keys): match known prefixes to reduce false positives.
- const hasValidKey = - !!key && - key !== "" && - !key.includes("<Your PostHog API key") && - !key.startsWith("<") + const hasValidKey = + !!key && + !key.startsWith("<") && + !key.includes("<Your PostHog API key") && + // PostHog public keys commonly start with "phc_" or "ph_" + /^(phc_|ph_)/.test(key)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/web/client/src/app/layout.tsx
(3 hunks)apps/web/client/src/components/conditional-posthog-provider.tsx
(1 hunks)
🔇 Additional comments (2)
apps/web/client/src/app/layout.tsx (1)
4-4
: Swap to ConditionalPostHogProvider looks correct and aligns with the PR goal.Import update is scoped and preserves the rest of the layout. No regressions apparent.
apps/web/client/src/components/conditional-posthog-provider.tsx (1)
1-17
: ConditionalPostHogProvider is safe: client‐side env and no module‐scope initializationI’ve verified the two main concerns raised:
• PostHogProvider does not initialize at module scope
– The onlyposthog.init
call lives inside auseEffect
in
apps/web/client/src/components/posthog-provider.tsx
, so importing this component won’t trigger any side‐effect until it mounts.
• Theenv
import is client‐safe
–apps/web/client/src/env.ts
only pulls in@t3-oss/env-nextjs
andzod
, with no server‐only (fs
,path
, etc.) imports. Its use ofprocess.env
is the standard Next.js pattern for exposingNEXT_PUBLIC_*
vars to the browser and won’t break the client bundle.No further action needed here.
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.
Thanks @reddygtvs, PostHogProvider is already a custom component so could we move the logic in there instead?
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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/web/client/src/components/posthog-provider.tsx (2)
18-29
: Prevent double-initialization under React 18 StrictMode dev mounts.
useEffect
can run twice in development. Guard with a ref so PostHog isn't initialized twice (avoids duplicate network calls and odd state).Apply this refactor to the effect and add a ref:
-import { useEffect } from "react" +import { useEffect, useRef } from "react" @@ export const PostHogProvider = observer(({ children }: { children: React.ReactNode }) => { + const initRef = useRef(false) @@ - useEffect(() => { - if (!hasValidKey) { - return; - } - posthog.init( - env.NEXT_PUBLIC_POSTHOG_KEY, { - api_host: env.NEXT_PUBLIC_POSTHOG_HOST, - capture_pageview: 'history_change', - capture_pageleave: true, - capture_exceptions: true, - }) - }, [hasValidKey]) + useEffect(() => { + if (!hasValidKey || initRef.current) return + posthog.init(env.NEXT_PUBLIC_POSTHOG_KEY, { + api_host: env.NEXT_PUBLIC_POSTHOG_HOST, + capture_pageview: "history_change", + capture_pageleave: true, + capture_exceptions: true, + }) + initRef.current = true + // Optional cleanup if you expect toggling key validity during HMR: + // return () => { posthog.shutdown?.(); initRef.current = false } + }, [hasValidKey])Note: If you expect
hasValidKey
to change at runtime (HMR), consider the optional cleanup to fully stop the client.
32-47
: Identify only after initialization; add a cheap guard to avoid race with init.While likely fine in practice, add the init guard so
identify
never runs beforeinit
completes.- useEffect(() => { - if (user && hasValidKey) { + useEffect(() => { + if (user && hasValidKey /* and initialized */ && initRef?.current) { try { posthog.identify(user.id, { firstName: user.firstName, lastName: user.lastName, displayName: user.displayName, email: user.email, avatar_url: user.avatarUrl, }, { signup_date: new Date().toISOString(), }) } catch (error) { console.error('Error identifying user:', error); } } - }, [user, hasValidKey]) + }, [user, hasValidKey])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/web/client/src/components/posthog-provider.tsx
(3 hunks)
const hasValidKey = env.NEXT_PUBLIC_POSTHOG_KEY && | ||
env.NEXT_PUBLIC_POSTHOG_KEY.trim() !== '' && | ||
!env.NEXT_PUBLIC_POSTHOG_KEY.includes('<Your PostHog API key') && | ||
!env.NEXT_PUBLIC_POSTHOG_KEY.startsWith('<') | ||
|
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.
🛠️ Refactor suggestion
Harden key validation and ensure hasValidKey
is a boolean; also gate the user query to avoid unnecessary network calls.
- Using string short-circuiting makes
hasValidKey
typestring | boolean
. Prefer an explicit boolean. - PostHog public browser keys start with "phc_". Checking this is more robust than filtering placeholders.
- Since user data is only used to identify with PostHog, avoid fetching it when analytics is disabled.
Apply this refactor (reorders and replaces the validation; also gates the query):
export const PostHogProvider = observer(({ children }: { children: React.ReactNode }) => {
- const { data: user } = api.user.get.useQuery();
-
- const hasValidKey = env.NEXT_PUBLIC_POSTHOG_KEY &&
- env.NEXT_PUBLIC_POSTHOG_KEY.trim() !== '' &&
- !env.NEXT_PUBLIC_POSTHOG_KEY.includes('<Your PostHog API key') &&
- !env.NEXT_PUBLIC_POSTHOG_KEY.startsWith('<')
+ const key = env.NEXT_PUBLIC_POSTHOG_KEY?.trim()
+ const hasValidKey: boolean = Boolean(key && key.startsWith("phc_"))
+ // Avoid the user fetch when analytics is disabled
+ const { data: user } = api.user.get.useQuery(undefined, { enabled: hasValidKey })
If you prefer to keep the existing order, at minimum coerce to boolean:
- const hasValidKey = env.NEXT_PUBLIC_POSTHOG_KEY &&
+ const hasValidKey = Boolean(env.NEXT_PUBLIC_POSTHOG_KEY &&
env.NEXT_PUBLIC_POSTHOG_KEY.trim() !== '' &&
!env.NEXT_PUBLIC_POSTHOG_KEY.includes('<Your PostHog API key') &&
- !env.NEXT_PUBLIC_POSTHOG_KEY.startsWith('<')
+ !env.NEXT_PUBLIC_POSTHOG_KEY.startsWith('<'))
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const hasValidKey = env.NEXT_PUBLIC_POSTHOG_KEY && | |
env.NEXT_PUBLIC_POSTHOG_KEY.trim() !== '' && | |
!env.NEXT_PUBLIC_POSTHOG_KEY.includes('<Your PostHog API key') && | |
!env.NEXT_PUBLIC_POSTHOG_KEY.startsWith('<') | |
export const PostHogProvider = observer(({ children }: { children: React.ReactNode }) => { | |
const key = env.NEXT_PUBLIC_POSTHOG_KEY?.trim() | |
const hasValidKey: boolean = Boolean(key && key.startsWith("phc_")) | |
// Avoid the user fetch when analytics is disabled | |
const { data: user } = api.user.get.useQuery(undefined, { enabled: hasValidKey }) | |
// …rest of PostHog setup and return… |
🤖 Prompt for AI Agents
In apps/web/client/src/components/posthog-provider.tsx around lines 13 to 17,
hasValidKey is computed via string short-circuiting which yields a string |
boolean and the validation is weak; change it to an explicit boolean that
validates the key starts with "phc_" (and is non-empty) instead of only
filtering placeholder text, and then gate any user data query so it only runs
when analytics are enabled (hasValidKey is true) to avoid unnecessary network
calls; if you must keep the existing checks/order, at minimum coerce the result
to a boolean (e.g., wrap in Boolean(...)) and add the "phc_" prefix check, plus
wrap the user query invocation in a condition that checks the final boolean
flag.
if (!hasValidKey) { | ||
return <>{children}</> | ||
} |
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.
💡 Verification agent
🧩 Analysis chain
Avoid removing the Provider; returning children directly can break consumers that assume a non-null PostHog context.
If any component calls usePostHog()
and then does posthog.capture(...)
without a null-check, rendering no provider will return null
from the hook and cause a runtime error. Previously, the provider was always mounted, so this was safe (despite console errors). You can still prevent network chatter without removing the provider by simply not calling init
when the key is invalid.
- Recommended change: always render
<PHProvider client={posthog}>
and rely on thehasValidKey
gate in the init effect (and identification effect). Remove the early return.
- if (!hasValidKey) {
- return <>{children}</>
- }
- Optional hardening: if you want complete no-op semantics, you can also opt out when invalid (no network, no buffering) right before returning JSX:
if (!hasValidKey) {
posthog.opt_out_capturing?.()
} else {
posthog.opt_in_capturing?.()
}
To assess risk, search the repo for unguarded usage of usePostHog()
:
🏁 Script executed:
#!/bin/bash
# Finds calls to usePostHog and flags likely unsafe direct usage without null checks.
rg -nP -C2 --type=ts --type=tsx '\busePostHog\s*\('
rg -nP --type=ts --type=tsx '\busePostHog\s*\(\)\s*(?:\.|!|\?\.)' -C1
Length of output: 181
I’ve updated the search to include both .ts
and .tsx
files and also look for direct capture
calls on the returned posthog
object. Running these should reveal any unguarded usages:
This will confirm whether any components invoke posthog.capture
(or other methods) without a surrounding <PHProvider>
. If results are still empty, manual verification of critical consumers is recommended.
🏁 Script executed:
#!/bin/bash
# Search all TS and TSX files for any use of the hook
rg -nP '\busePostHog\s*\(' -g '*.ts' -g '*.tsx' -n -C2
# Look for direct property/method access on the hook return value (e.g. usePostHog().capture)
rg -nP 'usePostHog\(\)\s*(\.|!|\?)' -g '*.ts' -g '*.tsx' -n -C1
# Find any posthog.capture calls to ensure they're guarded
rg -nP 'posthog\.capture\s*\(' -g '*.ts' -g '*.tsx' -n -C2
Length of output: 17061
Always render the <PHProvider>
—even with an invalid key—to prevent null‐context runtime errors
Your scan confirms multiple components call usePostHog()
and then immediately invoke posthog.capture(...)
(without guarding against a missing provider). If you return children
early, these calls will throw when the context is null. You must remove the early return and keep the provider mounted, using your hasValidKey
flag only inside the init/effect logic to suppress network activity.
Key unguarded usages found:
-
apps/web/client/src/app/project/[id]/_hooks/use-chat.tsx (line 70):
posthog.capture('user_send_message', …)
-
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-drag-drop.ts (line 96):
posthog.capture('image_drag_start')
-
apps/web/client/src/app/project/[id]/_components/top-bar/project-breadcrumb.tsx (lines 67, 82):
posthog.capture('download_project_code', …)
and
posthog.capture('download_project_code_failed', …)
-
apps/web/client/src/app/projects/_components/edit-app.tsx (line 27):
posthog.capture('open_project', { id: project.id })
Recommended refactor:
- if (!hasValidKey) {
- return <>{children}</>
- }
// ← always render <PHProvider client={posthog}>{children}</PHProvider>
Then inside your provider component you can gate calls to init
(and any identify effects) on hasValidKey
. For a full no-op mode, you can additionally:
if (!hasValidKey) {
posthog.opt_out_capturing?.()
} else {
posthog.opt_in_capturing?.()
}
🤖 Prompt for AI Agents
In apps/web/client/src/components/posthog-provider.tsx around lines 49–51, do
not return children early when hasValidKey is false — always render the
<PHProvider> so usePostHog() consumers never get a null context; instead move
the hasValidKey guard into the initialization/effect logic (only call
posthog.init/identify when hasValidKey is true) and suppress network activity
when invalid by calling posthog.opt_out_capturing() (or opt_in_capturing() when
valid). Ensure the provider remains mounted unconditionally while gating
init/identify/effects on hasValidKey to prevent runtime errors in the listed
callers.
That sounds good, I just made the change :) |
Description
Simple fix. Added a conditional wrapper that validates PostHog API key before initialization to eliminate 401 errors and CORS failures when using placeholder keys from .env file. Primary reason was to suppress the console errors during local development.
Now, PostHog only initializes when a valid API key is provided.
Related Issues
related to #2707
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Introduces
ConditionalPostHogProvider
to prevent PostHog initialization with invalid API keys, avoiding console errors.ConditionalPostHogProvider
inconditional-posthog-provider.tsx
to check for a valid PostHog API key before initializingPostHogProvider
.PostHogProvider
withConditionalPostHogProvider
inlayout.tsx
.This description was created by
for 586afe7. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit