fix(posthog): server flush, event data quality & dedup#2163
Open
sleyter93 wants to merge 3 commits into
Open
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
scguaquetam
reviewed
Jun 25, 2026
| }, | ||
| }) | ||
| await posthog.shutdown() | ||
| // flushAt: 1 sends in the background; do NOT shutdown()/flush() per request (singleton). |
Member
There was a problem hiding this comment.
I think we can delete this comment.
scguaquetam
reviewed
Jun 25, 2026
| import { ConnectWorkflow } from '@/shared/walletConnection/connection/ConnectWorkflow' | ||
|
|
||
| export default function ProposalsPage() { | ||
| useEffect(() => { |
Member
There was a problem hiding this comment.
I disagree taking out this, because we can track with Posthog data how many people really goes from the proposals general page, to one specific proposal, so we should keep track of it.
shutdown: - login route no longer calls posthog.shutdown() per request; the server client is a long-lived singleton and flushAt:1 already sends in the background. Per-request shutdown tore down shared state and raced across concurrent logins. - drain the queue once on SIGTERM/SIGINT in instrumentation.ts instead. events: - lowercase delegatee_address / previous_delegatee_address to match the wallet_address identity model (no more split breakdowns). - add tx_hash to rewards_claim_failed (backer + builder) for parity with the other tx failure events; expose hash from the claim hooks. - enrich proposal-submit captureException with category + proposal_name. - drop redundant proposal_page_viewed ($pageview already covers it). - extract txFailureProps() helper: unifies user_rejected/tx_failed classification (works for executeTxFlow's synthetic error and raw wagmi errors) and truncates error_message (full detail lives in Sentry). Replaces the duplicated 'Rejected TX' literal across 9 sites. - extract useRewardsClaimFailedCapture() so backer/builder modals share one capture and can't drift. - docs/POSTHOG_ANALYTICS.md updated to match all of the above.
sleyter93
force-pushed
the
fix/posthog-shutdown-and-event-quality
branch
from
91d4426 to
d664fde
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review fixes on top of #2162 (PostHog). Targets
v1.21.0.Server-side flush (the important one)
login/route.ts: removedawait posthog.shutdown()per request. The server client is a long-lived singleton (Node standalone) andflushAt: 1already sends in the background. Per-requestshutdown()tore down shared state (feature-flag poller, error tracking) and raced across concurrent logins (sharedshutdownPromise).instrumentation.ts: drain the queue once onSIGTERM/SIGINTinstead — the correct place forshutdown().Event data quality
delegatee_address/previous_delegatee_addressto match thewallet_addressidentity model (otherwise breakdowns split checksummed vs lowercase).rewards_claim_failed: addtx_hash(backer + builder) for parity with the other tx-failure events; exposedhashfrom the claim hooks. (Present only when a tx was actually broadcast —undefinedon wallet rejection / pre-broadcast failure, which is correct signal.)captureExceptionwithcategory+proposal_name(kept as an exception on purpose — it's a "submission broke" case, not a user-rejection metric; wallet is already attached via super property).proposal_page_viewed: removed. Redundant with auto-captured$pageview(filter by$pathname/$current_url), and it didn't even fire on proposal detail pages.Dedup / consistency
txFailureProps()helper incommonErrors.ts: unifiesuser_rejectedvstx_failedclassification (works for bothexecuteTxFlow's synthetic error and raw wagmi errors) and truncateserror_message(full detail lives in Sentry). Replaces the duplicated'Rejected TX'literal across 9 call sites.useRewardsClaimFailedCapture(): backer/builder modals now share one capture effect so they can't drift.Docs
docs/POSTHOG_ANALYTICS.mdupdated to match all of the above (including the corrected server-side flush guidance, which previously recommendedshutdown()).Verified with
tsc --noEmitandeslint(0 errors). Pre-existing, unrelated test failures on the branch (vault/health/staking/fund-manager) are not touched by this change.🤖 Generated with Claude Code