Skip to content

Conversation

reddygtvs
Copy link
Contributor

@reddygtvs reddygtvs commented Aug 23, 2025

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

  • Bug fix
  • New feature
  • Documentation update
  • Release
  • Refactor
  • Other (please describe):

Testing

Screenshots (if applicable)

Additional Notes


Important

Introduces ConditionalPostHogProvider to prevent PostHog initialization with invalid API keys, avoiding console errors.

  • Behavior:
    • Introduces ConditionalPostHogProvider in conditional-posthog-provider.tsx to check for a valid PostHog API key before initializing PostHogProvider.
    • Prevents 401 errors and CORS failures by not initializing PostHog with placeholder keys.
  • Layout Changes:
    • Replaces PostHogProvider with ConditionalPostHogProvider in layout.tsx.

This description was created by Ellipsis for 586afe7. You can customize this summary. It will automatically update as commits are pushed.

Summary by CodeRabbit

  • Bug Fixes
    • Prevented analytics from initializing without a valid key, eliminating related console warnings and errors.
    • Ensured user identification runs only when analytics is properly enabled.
  • Improvements
    • App now gracefully bypasses the analytics provider when the key is invalid, improving stability and reducing noise.
    • Analytics initialization and identification automatically re-evaluate if key validity changes, ensuring consistent behavior.

Copy link

vercel bot commented Aug 23, 2025

@reddygtvs is attempting to deploy a commit to the Onlook Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

coderabbitai bot commented Aug 23, 2025

Walkthrough

Adds 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

Cohort / File(s) Summary
PostHog provider key guard
apps/web/client/src/components/posthog-provider.tsx
Introduced hasValidKey to validate env key; gated PostHog init and user identification on hasValidKey; updated effect dependencies to [hasValidKey] and [user, hasValidKey]; added early return to skip provider when key invalid.

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)
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

Poem

I twitch my ears at keys that lie,
No bogus strings will pass me by.
If valid, hop—metrics ignite;
If not, I moonwalk out of sight.
Thump-thump logs, clean burrow flows—
Carrots in, bad keys out it goes. 🥕✨

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 Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 5d6bb58 and 586afe7.

📒 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 initialization

I’ve verified the two main concerns raised:

• PostHogProvider does not initialize at module scope
– The only posthog.init call lives inside a useEffect in
apps/web/client/src/components/posthog-provider.tsx, so importing this component won’t trigger any side‐effect until it mounts.
• The env import is client‐safe
apps/web/client/src/env.ts only pulls in @t3-oss/env-nextjs and zod, with no server‐only (fs, path, etc.) imports. Its use of process.env is the standard Next.js pattern for exposing NEXT_PUBLIC_* vars to the browser and won’t break the client bundle.

No further action needed here.

Copy link
Contributor

@Kitenite Kitenite left a 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?

Copy link

@coderabbitai coderabbitai bot left a 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 before init 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 586afe7 and 4c0dec7.

📒 Files selected for processing (1)
  • apps/web/client/src/components/posthog-provider.tsx (3 hunks)

Comment on lines +13 to +17
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('<')

Copy link

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 type string | 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.

Suggested change
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.

Comment on lines +49 to +51
if (!hasValidKey) {
return <>{children}</>
}
Copy link

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 the hasValidKey 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.

@reddygtvs
Copy link
Contributor Author

Thanks @reddygtvs, PostHogProvider is already a custom component so could we move the logic in there instead?

That sounds good, I just made the change :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants