Skip to content

fix: chain beforeSend hooks - #697

Open
posthog[bot] wants to merge 5 commits into
mainfrom
posthog/fix-before-send-hook-chaining
Open

fix: chain beforeSend hooks#697
posthog[bot] wants to merge 5 commits into
mainfrom
posthog/fix-before-send-hook-chaining

Conversation

@posthog

@posthog posthog Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

Fixes beforeSend hook chaining in the shared PostHogStateless.buildEvent() path used by both posthog-android and posthog-server.

When multiple hooks were registered, every hook received the original event instead of the previous hook's returned event. Each hook now receives the running value, so replacement events and mutations compose in registration order.

If a hook throws, the SDK logs the error and drops the event. This preserves the safer existing behavior: sending an event after a failed redaction or filtering hook could leak PII or enqueue a partially processed payload.

💚 How did you test it?

  • Added a regression test where the first hook returns a copied event with new properties and the second hook verifies it received that replacement.
  • Kept coverage asserting that a throwing hook logs the error and drops the event.
  • Ran make checkFormat.
  • Ran ./gradlew :posthog:test --tests 'com.posthog.PostHogBeforeSendTest' --tests 'com.posthog.PostHogStatelessTest.beforeSend error handling does not crash'.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran pnpm changeset to generate a changeset file

…of dropping

`buildEvent()` in `PostHogStateless` called `beforeSend.run(postHogEvent)` on every loop iteration instead of `beforeSend.run(eventChecked)`, so with 2+ registered hooks each one transformed the *original* event rather than the previous hook's output — mutations from an earlier hook were invisible to later hooks. A throwing hook also dropped the event outright (`return null`) instead of falling back to the last good value.

Both bugs affect `posthog-android` and `posthog-server`, which share this code via `PostHogStateless`.

Generated-By: PostHog Desktop
Task-Id: ea027967-6134-4b68-bcce-ef6a81e507b0
@github-actions

github-actions Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

posthog-android Compliance Report

Date: 2026-08-19 11:25:11 UTC
Duration: 118356ms

✅ All Tests Passed!

46/46 tests passed


Capture Tests

29/29 tests passed

View Details
Test Status Duration
Format Validation.Event Has Required Fields 354ms
Format Validation.Event Has Uuid 29ms
Format Validation.Event Has Lib Properties 27ms
Format Validation.Distinct Id Is String 25ms
Format Validation.Token Is Present 24ms
Format Validation.Custom Properties Preserved 28ms
Format Validation.Event Has Timestamp 30ms
Retry Behavior.Retries On 503 7025ms
Retry Behavior.Does Not Retry On 400 4026ms
Retry Behavior.Does Not Retry On 401 4025ms
Retry Behavior.Respects Retry After Header 7028ms
Retry Behavior.Implements Backoff 17035ms
Retry Behavior.Retries On 500 7020ms
Retry Behavior.Retries On 502 7017ms
Retry Behavior.Retries On 504 7018ms
Retry Behavior.Max Retries Respected 17030ms
Deduplication.Generates Unique Uuids 39ms
Deduplication.Preserves Uuid On Retry 7015ms
Deduplication.Preserves Uuid And Timestamp On Retry 12031ms
Deduplication.Preserves Uuid And Timestamp On Batch Retry 7019ms
Deduplication.No Duplicate Events In Batch 39ms
Deduplication.Different Events Have Different Uuids 26ms
Compression.Sends Gzip When Enabled 22ms
Batch Format.Uses Proper Batch Structure 20ms
Batch Format.Flush With No Events Sends Nothing 13ms
Batch Format.Multiple Events Batched Together 36ms
Error Handling.Does Not Retry On 403 4025ms
Error Handling.Does Not Retry On 413 4021ms
Error Handling.Retries On 408 5021ms

Feature_Flags Tests

17/17 tests passed

View Details
Test Status Duration
Request Payload.Request With Person Properties Device Id 32ms
Request Payload.Flags Request Uses V2 Query Param 21ms
Request Payload.Flags Request Hits Flags Path Not Decide 20ms
Request Payload.Flags Request Omits Authorization Header 25ms
Request Payload.Token In Flags Body Matches Init 21ms
Request Payload.Groups Round Trip 23ms
Request Payload.Groups Default To Empty Object 29ms
Request Payload.Disable Geoip False Propagates As Geoip Disable False 20ms
Request Payload.Disable Geoip Omitted Defaults To False 22ms
Request Payload.Flag Keys To Evaluate Contains Only Requested Key 22ms
Request Lifecycle.No Flags Request On Init Alone 11ms
Request Lifecycle.No Flags Request On Normal Capture 18ms
Request Lifecycle.Two Flag Calls Produce Two Remote Requests 36ms
Request Lifecycle.Mock Response Value Is Returned To Caller 20ms
Retry Behavior.Retries Flags On 502 321ms
Retry Behavior.Retries Flags On 504 321ms
Side Effect Events.Get Feature Flag Captures Feature Flag Called Event 20ms

@marandaneto
marandaneto marked this pull request as ready for review August 18, 2026 13:59
@marandaneto
marandaneto requested a review from a team as a code owner August 18, 2026 13:59
@marandaneto marandaneto changed the title fix: chain beforeSend hooks and fall back on exception instead of dropping fix: chain beforeSend hooks Aug 18, 2026
@marandaneto

Copy link
Copy Markdown
Member

PostHog/sdk-specs#43

@marandaneto marandaneto self-assigned this Aug 19, 2026
@marandaneto

Copy link
Copy Markdown
Member

PostHog/sdk-specs#43

@PostHog/team-client-libraries pls review if you think this is the right approach

turnipdabeets
turnipdabeets approved these changes Aug 19, 2026
"posthog-server": patch
---

Fix `beforeSend` hook chaining in `PostHogStateless.buildEvent()`: when multiple hooks are registered, each hook now receives the previous hook's output instead of the original, pre-chain event, so a mutation made by one hook is visible to the next. Hooks that throw continue to drop the event to avoid enqueueing a potentially unsafe, partially processed payload. Affects both `posthog-android` and `posthog-server`, which share this code path.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: worth trimming this?

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