[Feat][SDK-594] Compress payload and update payload max size to 1mb#371
[Feat][SDK-594] Compress payload and update payload max size to 1mb#371buongarzoni wants to merge 9 commits intomasterfrom
Conversation
|
@claude review |
…ng SyncSender behaviour
…hird-party extensibility
|
@claude review |
There was a problem hiding this comment.
All previously raised feedback has been addressed across follow-up commits — LGTM.
Extended reasoning...
Overview
This PR adds opt-out gzip compression of outbound Rollbar payloads (default on) and raises the max payload size from 512KB to 1MB. It touches the sync sender (SyncSender), the reactive-streams AsyncSender plus its bundled HTTP clients (ReactorAsyncHttpClient, ApacheRequestPublisher), the public AsyncHttpRequest interface, and both ConfigBuilder variants. Tests were added for the gzip-on and gzip-off paths in SyncSenderTest.
Security risks
Low. The change is a content-encoding wrapper around the existing HTTPS POST to the Rollbar ingest endpoint — no new auth surface, no new input parsing, no new sinks. The only behavioral concern (silent default-on for third-party AsyncHttpClient implementations) was resolved by switching to a compression-intent pattern: the request body stays a String and isCompressionRequested() is just a hint, so a pre-PR third-party client keeps working (uncompressed) instead of receiving a null body.
Level of scrutiny
Moderate-to-high warranted on the initial revision (default-on behavior change, public extension point, native off-heap resources via Deflater). After the four follow-up commits the PR is in a clean state and matches what I would expect from a careful implementation: try-with-resources around GZIPOutputStream, try/catch + buffer.release() around the pooled Netty ByteBuf, and a raw-stream-first acquire pattern in SyncSender.sendJson so the underlying connection stream is closed if the gzip header write fails.
Other factors
Every prior review point I raised across three reviews has a corresponding remediation commit (44f2889, be1e055, 1f5f214, 9541f0a), all inline comments are marked resolved, and the bug hunting system found nothing new in the current state. Test coverage exists for both compression-on (decompresses captured bytes and asserts equality) and compression-off (asserts Content-Encoding header is never set) paths in SyncSenderTest. The author has been responsive and methodical — comfortable approving without further human review.
Description of the change
Larger payload limit
The maximum payload size has been raised from 512 KB to 1 MB.
Gzip compression
Payloads are now gzip-compressed by default before being sent to Rollbar. This significantly reduces bandwidth — in practice around 10× on typical JSON error payloads.
Compression can be turned off if needed:
rollbar-java / rollbar-android
rollbar-reactive-streams
▎ Note for custom AsyncHttpClient implementations: if you have plugged in your own AsyncHttpClient via ConfigBuilder.httpClient(...), compression is signalled via request.isCompressionRequested(). Your client will still receive the
▎ full JSON body via request.getBody() — it will just be sent uncompressed unless you handle the flag yourself.
1 - Big uncompressed example payload (841kb)
2 - Big uncompressed example payload exceeding limit (1.27mb)
3- Same payload as 2, but compressed (10kb)
Type of change
Related issues
Checklists
Development
Code review