-
Notifications
You must be signed in to change notification settings - Fork 5.5k
18501 bug posthog triggers and actions should use instance url #18506
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
18501 bug posthog triggers and actions should use instance url #18506
Conversation
- Bump version of @pipedream/posthog to 0.2.1 and update @pipedream/platform dependency to ^3.1.0. - Increment action versions for capture-event, create-query, get-cohorts, get-persons, get-surveys, and new-action-performed to reflect recent changes. - Modify _baseUrl method to support public URL generation based on authentication instance.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughThis update introduces instance_url-aware request routing in PostHog app internals via a new publicUrl flag, updates two internal callers to use public URLs, bumps package and action/source versions, and updates a dependency in package.json. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor UserCode as Action/Source
participant PH as PostHog App
participant URL as _baseUrl(publicUrl)
participant HTTP as PostHog API
rect rgb(242, 248, 255)
note right of UserCode: Example: createQuery / captureEvent
UserCode->>PH: _makeRequest({ path, method, publicUrl: true, ... })
PH->>URL: compute base URL with instance_url (publicUrl=true)
URL-->>PH: https://<public-domain>/...
PH->>HTTP: Request to public URL
HTTP-->>PH: Response
PH-->>UserCode: Result
end
rect rgb(248, 248, 248)
note right of UserCode: Other calls (default)
UserCode->>PH: _makeRequest({ path, method })
PH->>URL: compute base URL (publicUrl=false)
URL-->>PH: https://<instance-url>/...
PH->>HTTP: Request to instance URL
HTTP-->>PH: Response
PH-->>UserCode: Result
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/posthog/posthog.app.mjs (1)
141-158: RemovepublicUrl: truefromcreateQuery. Query endpoints under/api/projects/:project_id/queryare private REST endpoints that require authentication and the standard API domain, not the ingestion URL. Only POST-only ingestion endpoints (/i/v0/e,/batch,/flags) should setpublicUrl: true. (posthog.com)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
components/posthog/actions/capture-event/capture-event.mjs(1 hunks)components/posthog/actions/create-query/create-query.mjs(1 hunks)components/posthog/actions/get-cohorts/get-cohorts.mjs(1 hunks)components/posthog/actions/get-persons/get-persons.mjs(1 hunks)components/posthog/actions/get-surveys/get-surveys.mjs(1 hunks)components/posthog/package.json(2 hunks)components/posthog/posthog.app.mjs(2 hunks)components/posthog/sources/new-action-performed/new-action-performed.mjs(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). (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (11)
components/posthog/sources/new-action-performed/new-action-performed.mjs (2)
8-8: Version bump is appropriate.The version bump from 0.0.2 to 0.0.3 correctly reflects the updated dependency behavior in the PostHog app layer.
83-91: Verified: createQuery uses instance_url for instance routingcreateQuery(andcaptureEvent) invoke_makeRequestwithpublicUrl: true, and_baseUrlconstructs the URL fromthis.$auth.instance_url.components/posthog/actions/capture-event/capture-event.mjs (2)
7-7: Version bump is appropriate.The version bump from 0.0.2 to 0.0.3 correctly reflects the updated dependency behavior in the PostHog app layer.
54-64: Ensure captureEvent routes to the correct instance URL.This action calls
this.posthog.captureEvent()which, per PR objectives, must route to the correct PostHog instance (US, EU, or self-hosted). The AI summary indicates this method now usespublicUrl: truein the app layer, but the core implementation is not provided in this review set. The verification script in the previous file will check this.components/posthog/actions/get-cohorts/get-cohorts.mjs (2)
7-7: Version bump is appropriate.The version bump from 0.0.1 to 0.0.2 aligns with the package-wide updates for instance_url support.
34-41: Verify listCohorts uses the correct instance URL.Unlike
createQueryandcaptureEvent,listCohortslikely calls the private API and should use the instance URL without thepublicUrlflag. Confirm that the base URL construction inposthog.app.mjsdefaults toinstance_urlwhenpublicUrlis not set.components/posthog/actions/create-query/create-query.mjs (2)
7-7: Version bump is appropriate.The version bump from 0.0.1 to 0.0.2 correctly reflects the package-wide instance_url routing updates.
39-49: Confirm createQuery uses instance_url routing.This action calls
this.posthog.createQuery(), which should now usepublicUrl: trueto route queries to the correct PostHog instance. The verification script in the first file will validate this implementation.components/posthog/actions/get-persons/get-persons.mjs (2)
7-7: Version bump is appropriate.The version bump from 0.0.1 to 0.0.2 aligns with the package-wide updates for instance_url support.
46-57: Verify listPersons uses the correct instance URL.Like
listCohorts,listPersonscalls the private API and should use the instance URL without thepublicUrlflag. Confirm that all private API methods inposthog.app.mjscorrectly useinstance_urlfor routing.components/posthog/posthog.app.mjs (1)
79-94: LGTM: Clean implementation of publicUrl parameter propagation.The destructuring pattern cleanly extracts
publicUrlfrom opts and passes it through to_baseUrl(). This approach allows opt-in public URL usage per endpoint.
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.
LGTM!
Resolves #18501
Summary by CodeRabbit
Bug Fixes
Chores