feat: use batch ingestion for browser analytics requests#3616
Closed
marandaneto wants to merge 19 commits into
Closed
feat: use batch ingestion for browser analytics requests#3616marandaneto wants to merge 19 commits into
marandaneto wants to merge 19 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Contributor
Prompt To Fix All With AIFix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/browser/src/posthog-core.ts:1036-1047
**Retry path bypasses batch routing due to `retry_count` URL mutation**
`RetryQueue.retriableRequest` calls `extendURLParams(options.url, { retry_count: retriesPerformedSoFar })` before forwarding to `_send_request`, so on the first and all subsequent retries `options.url` is `https://…/e/?retry_count=1` rather than the clean endpoint URL. The strict equality check then evaluates to `false`, the `if` block is skipped, and the retried event is sent to `/e/` in the legacy single-event format instead of `/batch/` — contradicting the goal of this PR. A URL prefix/path check (e.g. `options.url.startsWith(…)`) or an explicit `_isAnalyticsRequest` flag on the queued request options would survive URL extension by callers.
### Issue 2 of 3
packages/browser/src/posthog-core.ts:1046
The right-hand side `Compression.GZipJS` simply repeats the condition — this is equivalent to `options.compression === Compression.GZipJS ? options.compression : undefined`. Using `options.compression` directly expresses the intent more clearly and avoids the repeated literal (OnceAndOnlyOnce).
```suggestion
options.compression = options.compression === Compression.GZipJS ? options.compression : undefined
```
### Issue 3 of 3
packages/browser/src/types.ts:192-198
**Narrow string literal used as a boolean flag**
`_compressionEncoding?: 'content-encoding'` only ever takes two values: `undefined` (falsy) or the single literal `'content-encoding'` (truthy). The check in `useHTTPContentEncoding` is therefore a roundabout boolean. A plain `_useContentEncoding?: boolean` would be simpler (no exported constant required, no string comparison at runtime) and makes the intent immediately obvious at both the definition and call sites.
Reviews (1): Last reviewed commit: "feat: use batch ingestion for browser an..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Contributor
|
Size Change: +7.65 kB (+0.05%) Total Size: 16.1 MB
ℹ️ View Unchanged
|
turnipdabeets
approved these changes
May 18, 2026
Member
Author
|
@PostHog/team-ingestion remote config still returns if the SDK should honor |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Contributor
|
marandaneto
commented
May 19, 2026
| normalizedForCheck.startsWith('vbscript:') || | ||
| normalizedForCheck.startsWith('data:') || | ||
| normalizedForCheck.startsWith('file:') | ||
| normalizedForCheck.indexOf('javascript:') === 0 || |
Member
Author
There was a problem hiding this comment.
startsWith isnt IE11 compat so it was always failing in the E2E test, now we have a linter, so i replaced all calls with indexOf
Contributor
|
👋 Hey @marandaneto FYI all the "legacy" capture ingestion endpoints ( |
Member
Author
thanks, so not really needed to do this then since they are routing to the same place, no need to risk some disruption for no benefits |
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
4 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.
Problem
Browser analytics requests still used the legacy
/e/ingestion shape and query-param gzip signaling for regular fetch/XHR calls, while core-based SDKs use/batch/with standard HTTP gzip headers. We want regular browser requests to align with core ingestion without losing reliablesendBeacondelivery during page unload.Changes
sendBeaconbrowser analytics requests through/batch/using the core-style{ api_key, batch, sent_at }payload envelope.Content-Type: application/jsonandContent-Encoding: gzipfor gzip-compressed non-beacon analytics requests./e/+compression=gzip-jsbehavior forsendBeacon, since beacon cannot set custom headers./batch/and HTTP gzip handling.Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file