Repository navigation
fix!: analytics setup problems never fail the app - #369
Conversation
|
👋 hey @dustinbyrne some context on reversing 356 here - while researching other parity issues and locking in global decisions for v1 SDK behavior as I prep public release docs, I found some things about the 356 change:
If you think this compromise (error on wrong token type; no-op client on missing token at init) is a bad call, or your goal is to normalize the fail-fast behavior across all v1 SDKs, that works for me, let's chat about it. If not, I'm hoping to normalize the behavior here across all v1 SDKs (including those with live prep branches and future conversions for the fleet.) |
eli-r-ph
changed the title
eli-r-ph
changed the title
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
marandaneto
left a comment
marandaneto
left a comment
There was a problem hiding this comment.
Automated advisory code review.
An empty or personal (phx_) project API key gives a disabled client and a nil error again, with one error log, so a missing key never stops an app. Constructors still return a nil client and an error for invalid config. Config.Disabled builds a disabled client on purpose, for tests. Co-Authored-By: Claude Opus 5.5 <[email protected]>
posthogmcpsdk.NewMiddleware and Instrument panicked on a nil *posthogmcp.Analytics, and the otel bridge's NewExporter and NewSpanProcessor returned an error for an empty API key. Both now report one error and return a version that does nothing, like the core client does for an empty or personal key, so analytics setup never stops the application. The otel constructors also treat a personal (phx_) key as unusable and never log the key. An invalid WithHost still returns an error. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The disabled client and the quota-limited GetAllFlags path returned one shared result and map, so a caller's fallback changed what every other caller received. Co-Authored-By: Claude Opus 5.5 <[email protected]>
f5989c8 to
d38e04c
Compare
2 participants
💡 Motivation and Context
#356 made both constructors fail on a blank API key, so a missing env var can now crash a Go app.
Every other PostHog SDK returns a disabled client instead; this restores that behavior, keeps
(Client, error)for invalid config, and applies the same rule to the MCP and OpenTelemetry integrations.Changes
phx_...) API key returns a disabled client, a nil error and one error log.ErrSDKDisabled.Config.Disabledbuilds a disabled client for tests and local development.phx_key is never sent.posthogmcpsdk.NewMiddlewareandInstrumentno longer panic on nil analytics; they report one error and pass every request through unchanged.otelbridge'sNewExporterandNewSpanProcessortreat an empty orphx_key the same way: one error log and an exporter or processor that sends nothing.@dustinbyrne this partly reverses #356, so a key problem never stops an app in any SDK.
💚 How did you test it?
phx_andConfig.Disabledkeys: no requests, one error log, no key in logs.otelconstructors, and a nil-analytics MCP server whose tool calls and schemas stay unchanged.go test -racepasses alone and merged onto the top of the v1 stack (docs: complete the v2 migration guide and changesets #368).🤖 Agent context
Autonomy: Human-driven (agent-assisted). Written with Claude Code; restores the no-op client removed in #356.
🤖 Generated with Claude Code