sentry-apache-http-client-5: transport sends a truncated gzip body, Relay rejects every envelope with HTTP 400

[alacoste]

Integration

sentry-apache-http-client-5

Java Version

17

Other Error Monitoring Solution

No

Other Error Monitoring Solution Name

No response

Version

8.21.1 (also present on main, and in every release since the module was added in #1114)

Steps to Reproduce

ApacheHttpClientTransport.send reads the gzip buffer inside the try-with-resources that owns the GZIPOutputStream, i.e. before close():

https://github.com/getsentry/sentry-java/blob/main/sentry-apache-http-client-5/src/main/java/io/sentry/transport/apache/ApacheHttpClientTransport.java

try (final ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
    final GZIPOutputStream gzip = new GZIPOutputStream(outputStream)) {
  options.getSerializer().serialize(envelopeWithClientReport, gzip);

  final SimpleHttpRequest request = SimpleHttpRequests.post(requestDetails.getUrl().toString());
  request.setBody(
      outputStream.toByteArray(), ContentType.create("application/x-sentry-envelope"));  // <-- before close()
  request.setHeader("Content-Encoding", "gzip");

Deflater buffers its compressed output and only emits the final deflate block plus the 8-byte gzip trailer (CRC32 + ISIZE) on close()/finish(). JsonSerializer.serialize deliberately does not close the stream it is handed ("we do not want to close these as we would also close the stream that was passed in"), and flush() on a GZIPOutputStream constructed without syncFlush does not flush the deflater either. So at the point toByteArray() is called, the buffer holds only the 10-byte gzip header.

To reproduce end-to-end: configure the SDK with options.setTransportFactory(new ApacheHttpClientTransportFactory()) and capture anything. Every envelope is rejected.

The truncation itself reproduces with no Sentry code at all, mirroring what the serializer does to the stream:

byte[] payload = ("{\"event_id\":\"9ec79c33ec9942ab8353589fcb2e04dc\"}\n"
    + "{\"type\":\"event\",\"length\":42}\n" + "x".repeat(400)).getBytes("UTF-8");

byte[] body;
try (ByteArrayOutputStream out = new ByteArrayOutputStream();
     GZIPOutputStream gzip = new GZIPOutputStream(out)) {
  BufferedOutputStream bos = new BufferedOutputStream(gzip);   // as JsonSerializer does
  bos.write(payload);
  bos.flush();
  body = out.toByteArray();     // read before close(), as the transport does
}

System.out.println(payload.length + " bytes in -> " + body.length + " bytes out");
new GZIPInputStream(new ByteArrayInputStream(body)).readAllBytes();   // throws

469 bytes in -> 10 bytes out
Exception in thread "main" java.io.EOFException: Unexpected end of ZLIB input stream

Moving toByteArray() after the try-with-resources block (or calling gzip.finish() before reading) produces a complete, decodable stream.

Expected Result

The POST body is a complete gzip stream containing the serialized envelope, and Sentry accepts it — the same body the default HttpConnection-based transport produces. HttpConnection gets this right incidentally, because it gzips straight into the connection's output stream and so never reads a buffer early.

Actual Result

The body is always the bare 10-byte gzip header: no payload, no trailer, regardless of envelope size. Sentry/Relay rejects 100% of envelopes:

HTTP 400 Failed to buffer the request body: unexpected end of file

Verified against a Relay instance: the same envelope sent uncompressed, or gzipped with the ordering fixed, returns HTTP 200; gzipped the way this transport does it returns the 400 above.

Two things make this hard to notice, which is presumably why the module has shipped this way since #1114:

1. The failure is silent. The transport reports it via options.getLogger(), and DiagnosticLogger.isEnabled returns false unless options.isDebug() is set, so by default every event is dropped with no output at all. We lost five days of production error reporting before noticing, and only found it by reading Relay's own request logs.
2. Content-Encoding: gzip is set unconditionally, so there is no non-gzip path that would have worked.

Possibly relevant to #4742 ("Verify that the Apache Transport works ... Make a decision on how to move forward with it") — this is the answer to the first bullet: as shipped, it cannot deliver an envelope. Happy to send a PR with the reorder plus a test asserting the request body inflates back to the serialized envelope, if useful.

[linear-code]

JAVA-707

[alacoste]

A second, unrelated defect in the same class, spotted while working around the one above — happy to fold it into the same PR.

ApacheHttpClientTransport.close(boolean) awaits a shutdown it never initiates:

try {
  httpclient.awaitShutdown(TimeValue.ofSeconds(isRestarting ? 0 : 1));
} catch (InterruptedException e) {
  ...
} finally {
  httpclient.close(CloseMode.GRACEFUL);
}

AbstractHttpAsyncClientBase.awaitShutdown waits for the reactor to reach SHUT_DOWN, but nothing has asked it to shut down, so the status stays ACTIVE and the call simply blocks until the timeout expires. close(CloseMode.GRACEFUL) in the finally then performs the actual shutdown. Net effect: a flat extra second on every non-restarting close, and the intended "give in-flight uploads a moment, then close" never happens.

Timed against a live reactor with one request in flight (httpclient5 5.5.1):

--- awaitShutdown only (current code) ---
  status before      : ACTIVE
  status after await : ACTIVE
  awaitShutdown took : 1005 ms
--- initiateShutdown() first ---
  status after await : SHUTTING_DOWN
  awaitShutdown took : 0 ms

Adding httpclient.initiateShutdown() before the await fixes it. Skipping the await outright when isRestarting is also a bit clearer than passing TimeValue.ofSeconds(0), which today happens to be indistinguishable from the non-restarting path anyway (both just block until they give up).

Low severity on its own, since AsyncHttpTransport-style draining is done by flush() before close() in Sentry.close() — it costs shutdown latency rather than events.

[alacoste]

Correcting my previous comment: I understated the shutdown issue. It is not just the ~1s wasted on the no-op awaitShutdown — the close(CloseMode.GRACEFUL) in the finally adds up to a further 5 seconds.

MultiCoreIOReactor (httpcore5 5.3.6):

public final void close(final CloseMode closeMode) {
    close(closeMode, Timeout.ofSeconds(5));
}

public void close(final CloseMode closeMode, final Timeout timeout) {
    if (closeMode == CloseMode.GRACEFUL) {
        initiateShutdown();
        try {
            awaitShutdown(timeout);
        } catch (final InterruptedException e) {
            Thread.currentThread().interrupt();
        }
    }
    ...

So close(GRACEFUL) already does the initiate-then-await that the transport is trying to do by hand, on its own hardcoded 5s timeout that isn't reachable through the CloseableHttpAsyncClient API. The transport's own awaitShutdown(TimeValue.ofSeconds(isRestarting ? 0 : 1)) is therefore both a no-op (nothing has initiated shutdown yet) and redundant with what follows.

Measured with one 4s upload in flight (httpclient5 5.5.1):

initiate + await(1s) + close(GRACEFUL)    3780 ms
initiate + close(GRACEFUL)                3706 ms   <- isRestarting, still waits
initiate + await(1s) + close(IMMEDIATE)   1003 ms
initiate + close(IMMEDIATE)                  1 ms

Two consequences for ApacheHttpClientTransport.close(boolean) as written:

1. Worst case is ~5s, not the ~1s the code reads as.
2. isRestarting has no effect. Its whole purpose per the ITransport javadoc is to "avoid locking the main thread by dropping the current connection", but TimeValue.ofSeconds(0) only shortens a wait that was doing nothing anyway, and the close(GRACEFUL) afterwards blocks identically either way.

initiateShutdown(), then awaitShutdown only when not restarting, then close(CloseMode.IMMEDIATE) gives ~1s / ~0s respectively and makes the flag meaningful. Happy to include this in the same PR as the gzip fix.

[alacoste]

Third one in the same method, for completeness: ApacheHttpClientTransport.close(boolean) never closes its RateLimiter.

RateLimiter implements Closeable, owns a lazily created Timer and a CopyOnWriteArrayList<IRateLimitObserver>, and its close() cancels the timer and clears the observers. AsyncHttpTransport.close(boolean) calls rateLimiter.close() as its first statement:

@Override
public void close(final boolean isRestarting) throws IOException {
  rateLimiter.close();
  executor.shutdown();
  ...

ApacheHttpClientTransport.close(boolean) has no equivalent, so on Sentry.close() — and, more relevantly, on a re-init that goes through close(true) — the old limiter's timer and observers are stranded.

Modest severity: the timer is only created once a rate limit has actually been applied, and it's a daemon (new Timer(true)), so it can't block JVM exit. But it's a straightforward divergence from the reference transport.

So the three things in that class, in the order they matter:

1. Truncated gzip body → every envelope rejected with 400. This one is fatal and is the reason the module cannot deliver an event as shipped.
2. close() awaits a shutdown it never initiates, then close(GRACEFUL) adds up to a hardcoded 5s, making isRestarting a no-op.
3. RateLimiter never closed.

All three are one- or two-line fixes. Still happy to send a PR covering them plus a test that asserts the request body inflates back to the serialized envelope — just say if you'd rather split them.

[adinauer]

Thank you for opening this issue @alacoste and sorry for the troubles you're having.

I recommend just sticking with the default AsyncHttpTransport.

We haven't given ApacheHttpClientTransport much love in the past and what you're experiencing makes me think we should just throw it out in the next major. We've been debating about this internally and as far as we're aware usage numbers are fairly low anyways.

There's also more complex transport / queue related work we want to tackle (not soon though). This likely means further transport changes and time investment required where having two transports just adds a lot of overhead.

I'm afraid my suggestion if you want to keep using the ApacheHttpClientTransport is to copy the code and fix it as we're not planning on working on it anymore. Again - Sorry!

[alacoste]

Thanks for the reply!

For context, the reason we wanted to switch to this new transport is because we noticed that the default one is making one TCP connection per event to send to sentry (even though all these connections are to the same sentry endpoint).

This can quickly be a lot of connections if you have something a bit spammy going on, and can threaten to starve the machine of outgoing connections through the NAT of our infra setup (Google kubernetes engine).

Granted, it's not great to have something spamming sentry events in the first place, but it's scary if it can lead to a much more serious outage by starving the machine of connections to the outside internet!

I don't know if this context is enough motivation for your team to consider keeping this code alive or providing another solution to that end!

In any case, if you guys change your mind about this lmk, and if we're successful in using the copy of this we're currently trying out, I'll happily propose a fix PR here!