Repository navigation
feat!: remove the /batch fallback from capture V1 #940
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
Changes from all commits
8ac03d0
a35f65c
9c7b166
65b2cc3
3479164
eb60169
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |
|
|
||
| **Breaking:** send events to capture V1 (`/i/v1/analytics/events`) with the project token in an `Authorization: Bearer` header. | ||
|
|
||
| - If you use a reverse proxy, forward `/i/v1/analytics/events`. A 404 falls back to `/batch` until the app restarts; other errors such as 403 or 405 drop the events. | ||
| - If you use a reverse proxy or self-host PostHog, make sure `/i/v1/analytics/events` is served: a 404 or other non-retriable response drops the events. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR removes the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in a7797fc: the guide no longer mentions the |
||
| - Capture requests replace any `Authorization` header from `requestHeaders`, so authenticate your proxy with a different header. | ||
| - Only a 307 or 308 redirect to the same origin as `host` is followed, at most 5 times. Other redirects drop the events, so point `host` at the final host. | ||
| - 429 responses are no longer retried, and `Retry-After` pauses sending for at most 30 seconds, including session replay and logs. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
fwiw, (nit; paranoid take) i think we could be more resilient than this. a routing misconfiguration would mean certain data loss. considering the route is not configurable and we expect it to exist, it seems reasonable to take a position that a 404 would be retriable (or at least, shouldn't discard the event data) in that it should never happen, and if it did, is likely going to be rectified quickly.
i'm not sure there's many situations where i'd consider outright dropping data from the client (aside from a full event buffer), and perhaps this is something we should think about
Uh oh!
There was an error while loading. Please reload this page.
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.
hm I see what you mean. It looks like all the sdks drop this on 404: python, go and node on capture v1, and android/RN/iOS on /batch, replay and logs today. I was removing it in this PR to make it easier to review instead of all in 1. Ultimately we would want it to use capture v1 and not /batch here, but I agree it's worth rethinking, especially for mobile. It might be a bigger change that affects more than capture v1 adn the fallback probably isn’t /batch but some sort of retry.
Uh oh!
There was an error while loading. Please reload this page.
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.
I'd lean against retrying 404s here: 3.x didn't retry them on
/batch, the other v1 SDKs treat 404 as final, and the draft public docs for this release list only 408, 5xx and network errors as retried. If I'm missing context or you feel strongly we need it, let me know and I'm happy to update the decision (will need to update v1 SDKs and release docs/specs for parity)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.
discussed offline, we're pivoting on this and adding
404and429to retry allowlist for all v1 SDKs ✅ thanks again!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.
Covered by #979: capture v1 and the AI endpoint now retry 404 and 429.