Repository navigation
docs(http-client): specify request body compression and its opt-out - #81
Conversation
The http-client spec mentioned gzip only in passing, with no default, no opt-out surface, and no failure behavior. Three mobile SDKs just added a compression config after gzipped bodies were rewritten in transit on managed networks, and posthog-js has long carried a boolean disable switch. Adds an http-client requirement covering the gzip default, both shipped option shapes, the observable off-state, the reserved Content-Encoding header, and the decision not to downgrade after a server rejection. Leaves the per-endpoint compression set unspecified. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: fb243d92-37c8-4181-8365-9985f279f6a8
turnipdabeets
left a comment
There was a problem hiding this comment.
Thanks, the mobile/browser side of this reads right and matches android#793 / ios#851 / flutter#603 / posthog-js. Pulling back my earlier approval though, because of scope: http-client/spec.md:250 applies to any SDK that compresses bodies, and the scenarios are all @both, but several server SDKs ship something different today:
- default off: posthog-python
gzip=False(posthog/client.py:681), posthog-phpcompress_request = false(lib/QueueConsumer.php:36), posthog-goCompressionNone(config.go:217), posthog-rscapture_compression: None(src/client/mod.rs:290) - non-gzip codecs: python/go/rs offer zstd/deflate/br for capture v1
- no config opt-out: posthog-elixir needs a custom
api_client_module(lib/posthog/api/client.ex:5-8)
Could you scope "gzip, on by default, config opt-out" to client SDKs (tag those scenarios @client), and for server SDKs keep only what holds everywhere: no gzip Content-Encoding when not compressing, local-failure fallback allowed, no silent downgrade after a server rejection? Small nit: line 252 names only compression (enum) or disable_compression; server SDKs also ship positive booleans (compress_request, .NET EnableCompression, node disableCompression), so "a boolean switch" without pinning a name would be safer.
Scoped it in 81a457e. The gzip default and the config opt-out now only bind client SDKs, and those three scenarios are What still applies to every SDK:
Took the nit too, the boolean is now named in the SDK's own style with While in there I also dropped the @turnipdabeets Mind taking another look? |
|
@turnipdabeets ping |
turnipdabeets
left a comment
There was a problem hiding this comment.
Thanks, the client/server scoping reads right now, so approving. The inline comments are small wording fixes, and I'd take them before merging or in a quick follow-up.
Two things that don't have a line in this diff:
- [suggestion]
logs/spec.md:226-228andtraces/spec.md:694-697still require gzip unless it's unavailable and don't mention an opt-out, but iOS.nonesends/i/v1/logsas plain JSON and this requirement says it must. Could each of them defer tohttp-clientfor the opt-out? Traces also says the SDK "SHALL still signal explicitly", which doesn't match the no-declaration MAY for browsers here. - [nit] posthog-kmp has no compression option that I could find, even though it wraps the natives. Could the proposal's Impact section name it so it gets tracked?
design.mdcould also mention the client/server split, and the PR body still reads as if the opt-out binds every SDK.
These come from reading the SDK source, I didn't run anything against the scenarios.
ioannisj
left a comment
There was a problem hiding this comment.
🤖 An AI agent wrote this on behalf of @ioannisj.
Took the two from the review body too, also in 41df4f4. Logs and traces now defer to the http-client opt-out with a scenario each, and traces only asks for an explicit signal where the transport can set it. posthog-kmp is named in Impact as missing an option, design.md has the client/server split, and the PR body is updated.
Keep both new http-client requirements: capture v1 transport (this branch) and request body compression and opt-out (#81). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Problem
http-clientmentions compression three times, always as an aside: "optional Content-Encoding (gzip)", "gzip compression for batched event uploads", and "timeout / compression / custom transport configuration". It never states the default, what an opt-out looks like, or what happens when compression is off or fails.That gap became visible when apps on managed networks and Android work profiles had their gzipped bodies rewritten in transit and rejected with HTTP 400, with no way out because
Content-Encodingis a reserved header. Three SDKs added acompressionconfig for it, and posthog-js has carried a boolean for much longer:PostHogConfig.compression,PostHogCompression.GZIP(default) /NONEPostHogCompression { gzip, none }, default.gzipdisable_compression: boolean, defaultfalseChanges
Adds one
http-clientrequirement, applied and archived on this branch, and makes the logs and traces compression requirements defer to it:Content-Encodingwhere the transport can set it. A CORS-limited browser transport may rely on the server detecting gzip, as posthog-js does.Content-Encodingagainst custom headers cannot point at that as the workaround.logsandtracessend raw bodies when thehttp-clientopt-out is on, each with a scenario.Needs review
/flags; iOS says/flagswas never gzipped. Neither PR treats that as a defect, so this PR records the off-state ("nothing you were compressing is compressed anymore") rather than inventing an endpoint list. Whether/flagsshould be gzipped at all is a real divergence someone should settle separately.Validation
openspec validate document-request-body-compression --strict: valid.openspec validate --specs --strict: 63 passed, 0 failed.git diff --checkpass.Created with PostHog Desktop
🤖 Generated with Claude Code