-
Notifications
You must be signed in to change notification settings - Fork 1
DC-4894 Move PostHog to worker #48
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
Conversation
WalkthroughDeleted the in-CLI PostHog client and its exports, replaced CLI analytics with timed POSTs to a create-db worker endpoint (using CLI_RUN_ID), added worker-side PostHog client and a POST /analytics endpoint, extended worker Env for PostHog, and added worker config Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI (create-db)
participant API as Create Service (/create)
participant W as Worker (/analytics)
participant PH as PostHog
CLI->>API: POST /create { ..., analytics?: { eventName, properties } }
API->>API: create project & start background tasks
alt analytics present
API->>W: POST /analytics { eventName, properties } (background)
W->>PH: POST /capture { api_key, event, distinct_id, properties }
alt PH OK
PH-->>W: 200 OK
W-->>API: queued/acknowledged
else PH error
PH-->>W: non-OK / error
W->>W: log error (EventCaptureError)
W-->>API: logged (non-fatal)
end
end
Note over CLI,W: CLI also sends immediate telemetry directly to worker via sendAnalyticsToWorker (2s timeout, errors swallowed)
CLI->>W: POST /analytics { eventName, properties } (direct)
W->>PH: POST /capture
PH-->>W: response
Possibly related PRs
Suggested reviewers
✨ 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 (
|
Deploying with
|
Status | Name | Latest Commit | Preview URL | Updated (UTC) |
---|---|---|---|---|
✅ Deployment successful! View logs |
claim-db-worker | 8ca7ecd | Commit Preview URL Branch Preview URL |
Aug 27 2025, 05:05 PM |
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
create-db/analytics.js (2)
3-7
: Single source of logging; include status code and type your error
- Double-logs occur because both analytics.js and callers log errors. Prefer logging in callers; keep this module pure (throw only).
- Include numeric status for better diagnostics and set the error name.
class EventCaptureError extends Error { constructor(event, status) { super(`Failed to submit PostHog event '${event}': ${status}`); + this.name = "EventCaptureError"; + this.event = event; } } @@ - if (!response.ok) { - throw new EventCaptureError(eventName, response.statusText); - } + if (!response.ok) { + throw new EventCaptureError( + eventName, + `${response.status} ${response.statusText}` + ); + } @@ - // Log all analytics errors for debugging - console.error(`${eventName}: Failed - ${error.message}`); - - // Re-throw the error so calling code can handle it if needed - throw error; + // Re-throw for caller to handle/log. + throw error;Also applies to: 33-35, 40-44
24-31
: Add a short timeout to avoid hanging the CLI on network issuesA 2s AbortController keeps the UX snappy and prevents stuck processes.
- try { - const response = await fetch(POSTHOG_CAPTURE_URL, { + try { + const ac = new AbortController(); + const timeout = setTimeout(() => ac.abort(), 2000); + const response = await fetch(POSTHOG_CAPTURE_URL, { method: "POST", headers: { "Content-Type": "application/json", }, - body: JSON.stringify(payload), + body: JSON.stringify(payload), + signal: ac.signal, }); + clearTimeout(timeout);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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)
create-db/analytics.js
(2 hunks)create-db/index.js
(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: claim-db-worker
- GitHub Check: Workers Builds: create-db-worker
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
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: 7
♻️ Duplicate comments (1)
create-db/index.js (1)
272-274
: Avoid double-logging analytics failures (ensure analytics.js doesn’t log and rethrow).Per the earlier review, keep these logs in index.js and remove internal console.error from create-db/analytics.js catch handlers to prevent duplicate messages.
Run to confirm analytics.js has no internal console.error in catch blocks:
#!/bin/bash set -e analytics_file=$(fd --max-results 1 '^analytics\.js$' | head -n1) echo "Analytics file: ${analytics_file:-not found}" if [ -n "$analytics_file" ]; then echo "=== console.error in analytics.js ===" rg -n -C2 'console\.error' "$analytics_file" || true echo "=== catch blocks in analytics.js ===" rg -n -C2 'catch\s*\(' "$analytics_file" || true fiAlso applies to: 315-320, 349-354, 418-423, 494-496, 526-531
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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)
create-db/index.js
(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: claim-db-worker
- GitHub Check: Workers Builds: create-db-worker
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
create-db/analytics.js (1)
25-33
: Add a short timeout to avoid hanging the CLI on network stalls.Abort after a few seconds; keep it simple and local.
- const response = await fetch(POSTHOG_CAPTURE_URL, { + const ac = new AbortController(); + const t = setTimeout(() => ac.abort(), Number(process.env.ANALYTICS_TIMEOUT_MS ?? 3000)); + const response = await fetch(POSTHOG_CAPTURE_URL, { method: "POST", headers: { "Content-Type": "application/json", }, - body: JSON.stringify(payload), + body: JSON.stringify(payload), + signal: ac.signal, }); + clearTimeout(t);
♻️ Duplicate comments (3)
create-db/analytics.js (3)
19-22
: Enforce PII-safe flag ($process_person_profile) cannot be overridden by callers.Place the flag last so it always wins.
- properties: { - $process_person_profile: false, - ...properties, - }, + properties: { + ...properties, + $process_person_profile: false, + },
11-14
: Guard against missing envs; normalize URL; gate debug logs.Avoid "undefined/capture" and noisy logs when not configured. Return early if either env is absent, trim and de-trail-slash host, and emit diagnostics only in debug mode.
- const POSTHOG_CAPTURE_URL = process.env.POSTHOG_API_HOST + "/capture"; - const POSTHOG_KEY = process.env.POSTHOG_API_KEY; - console.log("POSTHOG_KEY set?", !!POSTHOG_KEY); - console.log("POSTHOG_CAPTURE_URL:", POSTHOG_CAPTURE_URL); + const host = process.env.POSTHOG_API_HOST?.trim(); + const key = process.env.POSTHOG_API_KEY?.trim(); + if (!host || !key) { + if (process.env.DEBUG_ANALYTICS === "1") { + console.debug("Analytics disabled: missing POSTHOG_API_HOST or POSTHOG_API_KEY"); + } + return; + } + const POSTHOG_CAPTURE_URL = `${host.replace(/\/+$/, "")}/capture`; + const POSTHOG_KEY = key; + if (process.env.DEBUG_ANALYTICS === "1") { + console.debug("POSTHOG_KEY set?", true); + console.debug("POSTHOG_CAPTURE_URL:", POSTHOG_CAPTURE_URL); + }
38-39
: Silence success logs unless debugging.Avoid cluttering normal CLI output; gate behind a debug flag.
- // Log success message - console.log(`${eventName}: Success`); + if (process.env.DEBUG_ANALYTICS === "1") { + console.debug(`${eventName}: Success`); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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)
create-db/analytics.js
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: claim-db-worker
- GitHub Check: Workers Builds: create-db-worker
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
create-db-worker/src/index.ts (1)
163-164
: Return 404 for unknown routes.405 implies method is invalid for a known route; this is a catch-all path.
- return new Response('Method Not Allowed', { status: 405 }); + return new Response('Not Found', { status: 404 });
♻️ Duplicate comments (1)
create-db-worker/src/analytics.ts (1)
28-45
: Remove internal logs to prevent duplicate logging; include timeout and better error.Let callers (the /analytics route) own logging. Add a short timeout and include status code.
- try { - const response = await fetch(POSTHOG_CAPTURE_URL, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - }, - body: JSON.stringify(payload), - }); - - if (!response.ok) { - throw new EventCaptureError(eventName, response.statusText); - } - - console.log(`${eventName}: Success`); - } catch (error) { - console.error(`${eventName}: Failed - ${error instanceof Error ? error.message : String(error)}`); - throw error; - } + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 5000); + const response = await fetch(POSTHOG_CAPTURE_URL, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(payload), + signal: controller.signal, + }); + clearTimeout(timeoutId); + + if (!response.ok) { + throw new EventCaptureError(eventName, response.status, response.statusText); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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 (4)
create-db-worker/src/analytics.ts
(1 hunks)create-db-worker/src/index.ts
(3 hunks)create-db/analytics.js
(0 hunks)create-db/index.js
(8 hunks)
💤 Files with no reviewable changes (1)
- create-db/analytics.js
🧰 Additional context used
🧬 Code graph analysis (3)
create-db-worker/src/analytics.ts (2)
create-db/index.js (1)
response
(12-16)create-db-worker/src/index.ts (1)
fetch
(15-165)
create-db-worker/src/index.ts (1)
create-db-worker/src/analytics.ts (1)
PosthogEventCapture
(49-49)
create-db/index.js (1)
create-db-worker/src/index.ts (1)
fetch
(15-165)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: claim-db-worker
- GitHub Check: Workers Builds: create-db-worker
🔇 Additional comments (2)
create-db-worker/src/analytics.ts (1)
21-25
: Random distinct_id breaks user/session continuity — confirm intent.Using crypto.randomUUID() per event prevents tying events together. If you need anonymous continuity, pass a stable anon id from the caller or derive one (e.g., hashed CLI fingerprint).
create-db/index.js (1)
287-292
: Best‑effort analytics confirmed?These empty catch blocks intentionally swallow analytics failures so the CLI UX is unaffected. If that’s the goal, keep as-is; otherwise consider logging at debug level.
Also applies to: 339-345, 368-375, 432-438, 495-509, 534-539
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
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: 4
♻️ Duplicate comments (2)
create-db-worker/src/analytics.ts (2)
1-5
: Include status code/details in EventCaptureError for actionable failures.Expose statusCode and statusText; improves tests and ops.
-class EventCaptureError extends Error { - constructor(event: string, status: string) { - super(`Failed to submit PostHog event '${event}': ${status}`); - } -} +class EventCaptureError extends Error { + constructor( + public readonly event: string, + public readonly statusCode: number, + public readonly statusText: string + ) { + super(`Failed to submit PostHog event '${event}': ${statusCode} ${statusText}`); + } +}
14-17
: No-op when PostHog is disabled/misconfigured; build URL robustly.Prevents calls to "undefined/capture" and undefined api_key.
- const POSTHOG_CAPTURE_URL = this.env.POSTHOG_API_HOST + '/capture'; - const POSTHOG_KEY = this.env.POSTHOG_API_KEY; + const host = this.env.POSTHOG_API_HOST?.replace(/\/+$/, ''); + const key = this.env.POSTHOG_API_KEY; + if (!host || !key) { + // Analytics disabled or misconfigured — no-op + return; + } + const normalizedHost = /^https?:\/\//.test(host) ? host : `https://${host}`; + const POSTHOG_CAPTURE_URL = new URL('/capture', normalizedHost).toString(); + const POSTHOG_KEY = key;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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)
create-db-worker/src/analytics.ts
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
create-db-worker/src/analytics.ts (2)
create-db/index.js (1)
response
(12-16)create-db-worker/src/index.ts (1)
fetch
(15-165)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: claim-db-worker
- GitHub Check: Workers Builds: create-db-worker
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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)
create-db-worker/wrangler.jsonc
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: create-db-worker
- GitHub Check: Workers Builds: claim-db-worker
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
create-db/index.js (4)
278-283
: Call-site catch is only needed if helper throwsIf you adopt the helper refactor above (throw on non-OK), keeping this try/catch is correct. If not, this catch is redundant noise.
330-337
: Standardize analytics property keysPrefer a single casing convention (e.g., snake_case) for easier ingestion/search.
- "error-type": "rate_limit", - "status-code": 429, + error_type: "rate_limit", + status_code: 429,Apply the same normalization wherever these keys recur.
423-429
: Include HTTP status with api_error event and normalize keysAdds critical context and keeps naming consistent.
- await sendAnalyticsToWorker("create_db:database_creation_failed", { - command: CLI_NAME, - region: region, - "error-type": "api_error", - "error-message": result.error.message, - }); + await sendAnalyticsToWorker("create_db:database_creation_failed", { + command: CLI_NAME, + region: region, + error_type: "api_error", + error_message: result.error.message, + status_code: resp.status, + });
486-500
: Remove unstructuredfull-command
from analytics
Thefull-command
property captures the entire argv array—including arbitrary values that may contain sensitive information—despite already emitting each flag in a structured way. Drop this redundant field to avoid potential PII leakage.• File: create-db/index.js
Lines: ≈488- "full-command": `${CLI_NAME} ${rawArgs.join(" ")}`.trim(),
♻️ Duplicate comments (1)
create-db/index.js (1)
10-18
: Make analytics helper fail-fast; remove empty catch and check response.okSwallowing all errors hides non-OK responses and makes failures untestable. Let callers decide whether to ignore.
-async function sendAnalyticsToWorker(eventName, properties = {}) { - try { - await fetch(`${CREATE_DB_WORKER_URL}/analytics`, { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ eventName, properties }), - }); - } catch (error) {} -} +async function sendAnalyticsToWorker(eventName, properties = {}) { + const response = await fetch(`${CREATE_DB_WORKER_URL}/analytics`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ eventName, properties }), + }); + if (!response.ok) { + throw new Error( + `Analytics request failed: ${response.status} ${response.statusText}` + ); + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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)
create-db/index.js
(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
create-db/index.js (1)
create-db-worker/src/index.ts (1)
fetch
(15-165)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: create-db-worker
- GitHub Check: Workers Builds: claim-db-worker
🔇 Additional comments (1)
create-db/index.js (1)
525-531
: LGTM: consistent region_selected analytics for flag pathEvent name and properties match the interactive path usage.
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
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: 1
♻️ Duplicate comments (5)
create-db/index.js (5)
591-607
: Make the error log specific and resilient.Use the event id and guard against non-Error rejections.
- } catch (error) { - console.error("Error:", error.message); - } + } catch (error) { + console.error( + "Failed to send create_db:cli_command_ran analytics:", + error?.message ?? String(error) + ); + }
13-24
: Minor: declare CREATE_DB_WORKER_URL before sendAnalyticsToWorker.Improves top-down readability and avoids TDZ surprises if this helper is ever invoked during module init.
417-424
: Normalize error analytics keys to snake_case for consistency.Matches prior convention feedback and avoids quoting keys.
- await sendAnalyticsToWorker("create_db:database_creation_failed", { - command: CLI_NAME, - region: region, - "error-type": "rate_limit", - "status-code": 429, - "user-agent": userAgent, - }); + await sendAnalyticsToWorker("create_db:database_creation_failed", { + command: CLI_NAME, + region, + error_type: "rate_limit", + status_code: 429, + user_agent: userAgent, + });
447-454
: Apply the same key normalization for the invalid_json path.- await sendAnalyticsToWorker("create_db:database_creation_failed", { - command: CLI_NAME, - region, - "error-type": "invalid_json", - "status-code": resp.status, - "user-agent": userAgent, - }); + await sendAnalyticsToWorker("create_db:database_creation_failed", { + command: CLI_NAME, + region, + error_type: "invalid_json", + status_code: resp.status, + user_agent: userAgent, + });
518-525
: Apply the same key normalization for the api_error path.- await sendAnalyticsToWorker("create_db:database_creation_failed", { - command: CLI_NAME, - region: region, - "error-type": "api_error", - "error-message": result.error.message, - "user-agent": userAgent, - }); + await sendAnalyticsToWorker("create_db:database_creation_failed", { + command: CLI_NAME, + region, + error_type: "api_error", + error_message: result.error.message, + user_agent: userAgent, + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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)
create-db/index.js
(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 🚧 Preview release (PR
- GitHub Check: Workers Builds: claim-db-worker
- GitHub Check: Workers Builds: create-db-worker
🔇 Additional comments (2)
create-db/index.js (2)
372-378
: LGTM: interactive region analytics event looks correct.Event name and core properties are consistent.
633-639
: LGTM: flag-based region analytics event looks correct.Consistent with the interactive path.
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
create-db-worker/src/index.ts (1)
119-138
: Don’t schedule deletion/log “database_created” when create failed or projectID is missingGuard on
prismaResponse.ok
and a validprojectID
before enqueuing side effects.const backgroundTasks = async () => { try { - const response = JSON.parse(prismaText); - const projectID = response.data ? response.data.id : response.id; - - const workflowPromise = env.DELETE_DB_WORKFLOW.create({ params: { projectID } }); - - const analyticsPromise = env.CREATE_DB_DATASET.writeDataPoint({ - blobs: ['database_created'], - indexes: ['create_db'], - }); + if (!prismaResponse.ok) return; + const response = JSON.parse(prismaText); + const projectID = response.data ? response.data.id : response.id; + if (!projectID) return; + + const workflowPromise = env.DELETE_DB_WORKFLOW.create({ + params: { projectID }, + }); + + const analyticsPromise = env.CREATE_DB_DATASET.writeDataPoint({ + blobs: ['database_created'], + indexes: ['create_db'], + });
♻️ Duplicate comments (3)
create-db-worker/src/analytics.ts (3)
29-37
: Honor caller-provided distinct_id and avoid duplicating it in propertiesUse
properties.distinct_id
when present and omit it fromproperties
.- const payload = { - api_key: POSTHOG_KEY, - event: eventName, - distinct_id: crypto.randomUUID(), - properties: { - $process_person_profile: false, - ...properties, - }, - }; + const { distinct_id, ...restProps } = properties || {}; + const payload = { + api_key: POSTHOG_KEY, + event: eventName, + distinct_id: distinct_id ?? crypto.randomUUID(), + properties: { + $process_person_profile: false, + ...restProps, + }, + };
52-56
: Normalize network errors to EventCaptureError and drop noisy success logKeep worker logs lean; provide a single error type to callers.
- console.log(`${eventName}: Success`); - } catch (error) { - console.error(`${eventName}: Failed - ${error instanceof Error ? error.message : String(error)}`); - throw error; - } + } catch (error) { + console.error( + `[analytics] ${eventName}: ${error instanceof Error ? error.message : String(error)}` + ); + if (error instanceof EventCaptureError) throw error; + throw new EventCaptureError( + eventName, + 0, + error instanceof Error ? error.message : String(error) + ); + }
48-50
: Fix constructor args: pass numeric status to EventCaptureErrorCurrently calling with only
statusText
; TS/Runtime mismatch with the declared constructor.- if (!response.ok) { - throw new EventCaptureError(eventName, response.statusText); - } + if (!response.ok) { + throw new EventCaptureError(eventName, response.status, response.statusText); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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 (3)
create-db-worker/src/analytics.ts
(1 hunks)create-db-worker/src/index.ts
(3 hunks)create-db/index.js
(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
create-db-worker/src/analytics.ts (1)
create-db-worker/src/index.ts (1)
fetch
(15-153)
create-db-worker/src/index.ts (1)
create-db-worker/src/analytics.ts (1)
PosthogEventCapture
(60-60)
create-db/index.js (1)
create-db-worker/src/index.ts (1)
fetch
(15-153)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: claim-db-worker
- GitHub Check: Workers Builds: create-db-worker
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
create-db/index.js (1)
1-708
: Ensure global fetch availability in Node.jsOur grep and package inspection confirmed:
- There are numerous unprefixed fetch calls in create-db/index.js (lines 24, 38, 120, 294, 397, …), relying on Node’s global fetch.
- No “engines” field exists in any package.json (including create-db/package.json), so older Node.js versions (which lack built-in fetch) aren’t constrained.
To prevent runtime errors, please choose one of the following fixes:
Enforce Node ≥ 18
In create-db/package.json, add:"engines": { "node": ">=18.0.0" }This guarantees a Node.js runtime with global fetch.
Polyfill fetch
Install a fetch polyfill (e.g. node-fetch):npm install node-fetchThen at the very top of create-db/index.js:
import fetch from 'node-fetch';ensuring fetch is defined even on Node < 18.
Either enforce an appropriate Node.js engine or include a polyfill to guarantee fetch is always available.
♻️ Duplicate comments (3)
create-db-worker/src/analytics.ts (2)
29-37
: Respect caller-provided distinct_id and avoid duplicating it inside properties.Use distinct_id from properties when present; otherwise fallback to crypto.randomUUID(). Remove distinct_id from properties before sending.
- const payload = { - api_key: POSTHOG_KEY, - event: eventName, - distinct_id: crypto.randomUUID(), - properties: { - $process_person_profile: false, - ...properties, - }, - }; + const { distinct_id, ...restProps } = properties || {}; + const payload = { + api_key: POSTHOG_KEY, + event: eventName, + distinct_id: distinct_id ?? crypto.randomUUID(), + properties: { + $process_person_profile: false, + ...restProps, + }, + };
52-56
: Remove internal console logging; normalize thrown errors to EventCaptureError.Avoid duplicate logs (callers already log) and return a single error type.
- console.log(`${eventName}: Success`); - } catch (error) { - console.error(`${eventName}: Failed - ${error instanceof Error ? error.message : String(error)}`); - throw error; - } + } catch (error) { + if (error instanceof EventCaptureError) throw error; + throw new EventCaptureError( + eventName, + 0, + error instanceof Error ? error.message : String(error) + ); + }create-db/index.js (1)
378-385
: Remove redundant try/catch wrappers; helper already swallows and times out.Keeps logs clean and simplifies flow.
- try { await sendAnalyticsToWorker("create_db:region_selected", { command: CLI_NAME, region: region, "selection-method": "interactive", "user-agent": userAgent, }); - } catch (error) {}- try { await sendAnalyticsToWorker("create_db:database_creation_failed", { command: CLI_NAME, region: region, "error-type": "rate_limit", "status-code": 429, "user-agent": userAgent, }); - } catch (error) {}- try { await sendAnalyticsToWorker("create_db:database_creation_failed", { command: CLI_NAME, region, "error-type": "invalid_json", "status-code": resp.status, "user-agent": userAgent, }); - } catch (error) {}- try { await sendAnalyticsToWorker("create_db:database_creation_failed", { command: CLI_NAME, region: region, "error-type": "api_error", "error-message": result.error.message, "user-agent": userAgent, }); - } catch (error) {}- try { await sendAnalyticsToWorker("create_db:database_created", { command: CLI_NAME, region, utm_source: CLI_NAME, }); - } catch {}- try { await sendAnalyticsToWorker("create_db:cli_command_ran", { command: CLI_NAME, "full-command": `${CLI_NAME} ${rawArgs.join(" ")}`.trim(), "has-region-flag": rawArgs.includes("--region") || rawArgs.includes("-r"), "has-interactive-flag": rawArgs.includes("--interactive") || rawArgs.includes("-i"), "has-help-flag": rawArgs.includes("--help") || rawArgs.includes("-h"), "has-list-regions-flag": rawArgs.includes("--list-regions"), "has-json-flag": rawArgs.includes("--json") || rawArgs.includes("-j"), "has-user-agent-from-env": !!userAgent, "node-version": process.version, platform: process.platform, arch: process.arch, "user-agent": userAgent, }); - } catch (error) { - console.error("Error:", error.message); - }- try { await sendAnalyticsToWorker("create_db:region_selected", { command: CLI_NAME, region: region, "selection-method": "flag", "user-agent": userAgent, }); - } catch (error) {}Also applies to: 423-431, 454-462, 524-532, 584-590, 606-624, 647-655
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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)
create-db-worker/src/analytics.ts
(1 hunks)create-db/index.js
(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
create-db-worker/src/analytics.ts (1)
create-db-worker/src/index.ts (1)
fetch
(15-153)
create-db/index.js (1)
create-db-worker/src/index.ts (1)
fetch
(15-153)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: claim-db-worker
- GitHub Check: Workers Builds: create-db-worker
🔇 Additional comments (1)
create-db/index.js (1)
20-34
: Optional: don’t await analytics to reduce tail latency even further.Non-blocking fire-and-forget is fine since the helper has a timeout and swallows errors.
- await fetch(`${CREATE_DB_WORKER_URL}/analytics`, { + void fetch(`${CREATE_DB_WORKER_URL}/analytics`, {Likely an incorrect or invalid review comment.
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
create-db-worker/src/index.ts (3)
19-23
: Rate limiter key should be per-client, not global by path.Keying on request.url makes the limit effectively global per route. Prefer per-client keys (e.g., cf-connecting-ip + pathname).
- const { success } = await env.CREATE_DB_RATE_LIMITER.limit({ key: request.url }); + const ip = request.headers.get('cf-connecting-ip') || 'unknown'; + const path = new URL(request.url).pathname; + const { success } = await env.CREATE_DB_RATE_LIMITER.limit({ key: `${ip}:${path}` });
45-49
: Nit: unify health service name.Test endpoint returns service: "create-db-worker" but health returns "create-db". Align for observability.
- return new Response(JSON.stringify({ status: 'ok', service: 'create-db', timestamp: Date.now() }), { + return new Response(JSON.stringify({ status: 'ok', service: 'create-db-worker', timestamp: Date.now() }), {
116-135
: Guard background tasks to only run on successful creation.Currently, dataset write and workflow creation may run on API errors (and could emit “database_created” signals). Gate on prismaResponse.ok and a valid projectID.
- const backgroundTasks = async () => { + const backgroundTasks = async () => { try { - const response = JSON.parse(prismaText); - const projectID = response.data ? response.data.id : response.id; - - const workflowPromise = env.DELETE_DB_WORKFLOW.create({ params: { projectID } }); - - const analyticsPromise = env.CREATE_DB_DATASET.writeDataPoint({ - blobs: ['database_created'], - indexes: ['create_db'], - }); + if (!prismaResponse.ok) return; // only on success + const response = JSON.parse(prismaText); + const projectID = response?.data?.id ?? response?.id; + if (!projectID) return; + + const workflowPromise = env.DELETE_DB_WORKFLOW.create({ params: { projectID } }); + + const analyticsPromise = env.CREATE_DB_DATASET.writeDataPoint({ + blobs: ['database_created'], + indexes: ['create_db'], + });create-db/index.js (2)
261-270
: Await showHelp() in single-short-flag branch to avoid race/partial output.One path awaits, this one doesn’t. Align behavior.
- if (mappedFlag === "help") showHelp(); + if (mappedFlag === "help") { + await showHelp(); + return; + }
136-145
: Clarify offline error message (it’s the worker, not the API).The health check calls the worker. Tweak copy to reduce confusion.
- console.error( - chalk.red.bold("\n✖ Error: Cannot reach Prisma Postgres API server.\n") - ); + console.error( + chalk.red.bold("\n✖ Error: Cannot reach the Prisma Postgres create-db worker.\n") + );
♻️ Duplicate comments (1)
create-db/index.js (1)
15-20
: Fix crash when env vars are unset (.replace on undefined).Calling replace on undefined throws. Apply fallback before trimming.
-const CREATE_DB_WORKER_URL = - process.env.CREATE_DB_WORKER_URL.replace(/\/+$/, "") || - "https://create-db-temp.prisma.io"; -const CLAIM_DB_WORKER_URL = - process.env.CLAIM_DB_WORKER_URL.replace(/\/+$/, "") || - "https://create-db.prisma.io"; +const CREATE_DB_WORKER_URL = ( + process.env.CREATE_DB_WORKER_URL || "https://create-db-temp.prisma.io" +).replace(/\/+$/, ""); +const CLAIM_DB_WORKER_URL = ( + process.env.CLAIM_DB_WORKER_URL || "https://create-db.prisma.io" +).replace(/\/+$/, "");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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)
create-db-worker/src/index.ts
(3 hunks)create-db/index.js
(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-27T16:39:21.247Z
Learnt from: aidankmcalister
PR: prisma/create-db#48
File: create-db/index.js:423-431
Timestamp: 2025-08-27T16:39:21.247Z
Learning: In the prisma/create-db project, analytics property keys should use the existing kebab-case convention (e.g., "user-agent", "error-type", "status-code") rather than snake_case, to maintain consistency with existing analytics data and avoid duplicate attributes.
Applied to files:
create-db/index.js
🧬 Code graph analysis (2)
create-db-worker/src/index.ts (1)
create-db-worker/src/analytics.ts (1)
PosthogEventCapture
(60-60)
create-db/index.js (1)
create-db-worker/src/index.ts (1)
fetch
(15-150)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: create-db-worker
- GitHub Check: Workers Builds: claim-db-worker
🔇 Additional comments (4)
create-db-worker/src/index.ts (2)
63-86
: Analytics endpoint: good async semantics and disabled guard.Using ctx.waitUntil with a 202 response and returning 204 (no body) when POSTHOG is unset is correct and non-blocking.
128-133
: Confirm analytics.capture no-ops when PostHog is unset.The /analytics route guards POSTHOG_* vars, but this path does not. If PosthogEventCapture doesn’t internally no-op, add the same guard here.
Would you like me to patch this path to check env.POSTHOG_API_HOST/KEY before capturing?
create-db/index.js (2)
22-44
: Confirm distinct_id handling end-to-end.distinct_id is nested under properties; ensure the worker maps it to PostHog’s top-level distinct_id (or PosthogEventCapture handles it). Otherwise correlation won’t work.
If needed, I can add a tiny mapper in create-db-worker/src/analytics.ts to lift properties.distinct_id.
431-437
: LGTM on analytics payloads and kebab-case keys.Consistent with existing dataset conventions (e.g., "user-agent", "error-type", "status-code").
Also applies to: 460-466, 530-536, 588-593, 608-622, 645-650
✅ Preview CLIs & Workers are live! Test the CLIs locally under tag npx create-db@pr48
npx create-pg@pr48
npx create-postgres@$pr48 Worker URLs
|
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
create-db-worker/src/index.ts (2)
19-23
: Rate limiting key is bypassable; key by client IP + path and parse URL before limiting.Using request.url lets attackers vary the query string to evade limits; also URL is parsed after the limit call. Move URL parsing up and include cf-connecting-ip in the key.
- const { success } = await env.CREATE_DB_RATE_LIMITER.limit({ key: request.url }); - - const url = new URL(request.url); + const url = new URL(request.url); + const ip = request.headers.get('cf-connecting-ip') || 'unknown'; + const { success } = await env.CREATE_DB_RATE_LIMITER.limit({ + key: `${url.pathname}:${ip}`, + });Also applies to: 25-26
158-160
: Prefer 404 over generic 405, or include Allow header.A 405 should include an Allow header for the resource; if paths are unknown, 404 is more accurate.
- return new Response('Method Not Allowed', { status: 405 }); + return new Response('Not Found', { status: 404 });
♻️ Duplicate comments (2)
create-db/index.js (2)
3-9
: ESM + Node 18 requirement: ensure package.json is set accordingly.This file relies on ESM and global fetch/AbortController. Verify root package.json sets "type": "module" and engines.node >= 18 (as previously noted).
15-20
: Critical: .replace() on possibly undefined env vars throws.Coalesce before trimming to avoid crashes when env vars aren’t set.
-const CREATE_DB_WORKER_URL = - process.env.CREATE_DB_WORKER_URL.replace(/\/+$/, "") || - "https://create-db-temp.prisma.io"; -const CLAIM_DB_WORKER_URL = - process.env.CLAIM_DB_WORKER_URL.replace(/\/+$/, "") || - "https://create-db.prisma.io"; +const CREATE_DB_WORKER_URL = ( + process.env.CREATE_DB_WORKER_URL || "https://create-db-temp.prisma.io" +).replace(/\/+$/, ""); +const CLAIM_DB_WORKER_URL = ( + process.env.CLAIM_DB_WORKER_URL || "https://create-db.prisma.io" +).replace(/\/+$/, "");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
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)
create-db-worker/src/index.ts
(3 hunks)create-db/index.js
(8 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-27T16:39:21.247Z
Learnt from: aidankmcalister
PR: prisma/create-db#48
File: create-db/index.js:423-431
Timestamp: 2025-08-27T16:39:21.247Z
Learning: In the prisma/create-db project, analytics property keys should use the existing kebab-case convention (e.g., "user-agent", "error-type", "status-code") rather than snake_case, to maintain consistency with existing analytics data and avoid duplicate attributes.
Applied to files:
create-db-worker/src/index.ts
create-db/index.js
🧬 Code graph analysis (2)
create-db-worker/src/index.ts (1)
create-db-worker/src/analytics.ts (1)
PosthogEventCapture
(60-60)
create-db/index.js (1)
create-db-worker/src/index.ts (1)
fetch
(15-160)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: create-db-worker
- GitHub Check: Workers Builds: claim-db-worker
🔇 Additional comments (3)
create-db-worker/src/index.ts (2)
84-88
: Verify distinct_id handling (PostHog expects it top-level).CLI sends distinct_id inside properties. Ensure PosthogEventCapture lifts properties.distinct_id to the top-level distinct_id field when posting to PostHog, not leaving it inside properties.
Suggested helper in create-db-worker/src/analytics.ts (for illustration):
async capture(eventName: string, properties: Record<string, unknown> = {}) { const { distinct_id, ...props } = properties; const payload = { event: eventName, distinct_id: typeof distinct_id === 'string' ? distinct_id : undefined, properties: props, }; return this.posthogFetch(payload); }
2-2
: LGTM on PostHog wiring and typed bodies.Imports, Env additions, analytics instantiation, and request body typing look good.
Also applies to: 8-9, 16-16, 93-100, 107-108
create-db/index.js (1)
384-389
: Telemetry calls are correctly fire-and-forget; property keys stay kebab-case.Good use of void and consistent kebab-case keys as per existing dataset conventions.
Also applies to: 427-433, 456-462, 526-532, 585-589, 604-618, 641-646
Summary by CodeRabbit
New Features
Chores
Bug Fixes
Documentation