Skip to content

Commit 9841dec

Browse files
committed
core: Warn when server doesn't advertise grpc-accept-encoding
When the client sends compressed messages (grpc-encoding header set), but the server's response headers don't include grpc-accept-encoding, log a FINE-level warning. This helps diagnose compression compatibility issues between client and server. The warning is at FINE level to avoid disrupting existing systems while providing useful diagnostic information when needed. Fixes #1804 Signed-off-by: anirudhbh-ucb <[email protected]>
1 parent 9d5842b commit 9841dec

File tree

2 files changed

+167
-0
lines changed

2 files changed

+167
-0
lines changed

core/src/main/java/io/grpc/internal/AbstractClientStream.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static com.google.common.base.Preconditions.checkNotNull;
2020
import static com.google.common.base.Preconditions.checkState;
2121
import static io.grpc.internal.GrpcUtil.CONTENT_ENCODING_KEY;
22+
import static io.grpc.internal.GrpcUtil.MESSAGE_ACCEPT_ENCODING_KEY;
2223
import static io.grpc.internal.GrpcUtil.MESSAGE_ENCODING_KEY;
2324
import static io.grpc.internal.GrpcUtil.TIMEOUT_KEY;
2425

@@ -155,6 +156,9 @@ public final void setDecompressorRegistry(DecompressorRegistry decompressorRegis
155156
public final void start(ClientStreamListener listener) {
156157
transportState().setListener(listener);
157158
if (!useGet) {
159+
// Capture the message encoding before headers are cleared, so we can warn
160+
// if the server doesn't advertise support for it (issue #1804)
161+
transportState().setSentMessageEncoding(headers.get(MESSAGE_ENCODING_KEY));
158162
abstractClientStreamSink().writeHeaders(headers, null);
159163
headers = null;
160164
}
@@ -224,6 +228,8 @@ protected abstract static class TransportState extends AbstractStream.TransportS
224228
private ClientStreamListener listener;
225229
private boolean fullStreamDecompression;
226230
private DecompressorRegistry decompressorRegistry = DecompressorRegistry.getDefaultInstance();
231+
/** The message encoding sent by the client, or null if identity/none. */
232+
@Nullable private String sentMessageEncoding;
227233

228234
private boolean deframerClosed = false;
229235
private Runnable deframerClosedTask;
@@ -261,6 +267,16 @@ private void setDecompressorRegistry(DecompressorRegistry decompressorRegistry)
261267
checkNotNull(decompressorRegistry, "decompressorRegistry");
262268
}
263269

270+
/**
271+
* Sets the message encoding that the client is using for outbound messages.
272+
* Used to warn if the server doesn't advertise support for this encoding.
273+
*
274+
* @param messageEncoding the encoding name (e.g., "gzip"), or null for identity
275+
*/
276+
protected void setSentMessageEncoding(@Nullable String messageEncoding) {
277+
this.sentMessageEncoding = messageEncoding;
278+
}
279+
264280
@VisibleForTesting
265281
public final void setListener(ClientStreamListener listener) {
266282
checkState(this.listener == null, "Already called setListener");
@@ -342,6 +358,19 @@ protected void inboundHeadersReceived(Metadata headers) {
342358
}
343359
}
344360

361+
// Warn if client sent compressed messages but server didn't advertise support
362+
if (sentMessageEncoding != null) {
363+
byte[] acceptEncoding = headers.get(MESSAGE_ACCEPT_ENCODING_KEY);
364+
if (acceptEncoding == null) {
365+
log.log(
366+
Level.FINE,
367+
"Server did not include grpc-accept-encoding header in response. "
368+
+ "Client sent messages with encoding [{0}]. "
369+
+ "The server may not support this encoding.",
370+
sentMessageEncoding);
371+
}
372+
}
373+
345374
listener().headersRead(headers);
346375
}
347376

core/src/test/java/io/grpc/internal/AbstractClientStreamTest.java

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,13 @@
5454
import java.io.IOException;
5555
import java.io.InputStream;
5656
import java.net.SocketAddress;
57+
import java.util.ArrayList;
58+
import java.util.List;
5759
import java.util.concurrent.TimeUnit;
60+
import java.util.logging.Handler;
61+
import java.util.logging.Level;
62+
import java.util.logging.LogRecord;
63+
import java.util.logging.Logger;
5864
import org.junit.Before;
5965
import org.junit.Rule;
6066
import org.junit.Test;
@@ -527,6 +533,138 @@ public void resetOnReadyThreshold() {
527533
assertNull(options.clearOnReadyThreshold().getOnReadyThreshold());
528534
}
529535

536+
@Test
537+
public void inboundHeadersReceived_warnsWhenServerDoesNotAdvertiseAcceptEncoding() {
538+
// Set up log capture
539+
Logger logger = Logger.getLogger(AbstractClientStream.class.getName());
540+
Level originalLevel = logger.getLevel();
541+
List<LogRecord> logs = new ArrayList<>();
542+
Handler handler = new Handler() {
543+
@Override
544+
public void publish(LogRecord record) {
545+
logs.add(record);
546+
}
547+
548+
@Override
549+
public void flush() {}
550+
551+
@Override
552+
public void close() {}
553+
};
554+
logger.addHandler(handler);
555+
logger.setLevel(Level.FINE);
556+
557+
try {
558+
AbstractClientStream stream =
559+
new BaseAbstractClientStream(allocator, statsTraceCtx, transportTracer);
560+
stream.start(mockListener);
561+
562+
// Simulate that client sent gzip-compressed messages
563+
stream.transportState().setSentMessageEncoding("gzip");
564+
565+
// Server responds without grpc-accept-encoding header
566+
Metadata headers = new Metadata();
567+
stream.transportState().inboundHeadersReceived(headers);
568+
569+
// Verify warning was logged
570+
verify(mockListener).headersRead(headers);
571+
assertThat(logs).hasSize(1);
572+
LogRecord record = logs.get(0);
573+
assertThat(record.getLevel()).isEqualTo(Level.FINE);
574+
assertThat(record.getMessage()).contains("grpc-accept-encoding");
575+
// The parameter {0} contains the encoding
576+
assertThat(record.getParameters()).asList().contains("gzip");
577+
} finally {
578+
logger.removeHandler(handler);
579+
logger.setLevel(originalLevel);
580+
}
581+
}
582+
583+
@Test
584+
public void inboundHeadersReceived_noWarningWhenServerAdvertisesAcceptEncoding() {
585+
// Set up log capture
586+
Logger logger = Logger.getLogger(AbstractClientStream.class.getName());
587+
Level originalLevel = logger.getLevel();
588+
List<LogRecord> logs = new ArrayList<>();
589+
Handler handler = new Handler() {
590+
@Override
591+
public void publish(LogRecord record) {
592+
logs.add(record);
593+
}
594+
595+
@Override
596+
public void flush() {}
597+
598+
@Override
599+
public void close() {}
600+
};
601+
logger.addHandler(handler);
602+
logger.setLevel(Level.FINE);
603+
604+
try {
605+
AbstractClientStream stream =
606+
new BaseAbstractClientStream(allocator, statsTraceCtx, transportTracer);
607+
stream.start(mockListener);
608+
609+
// Simulate that client sent gzip-compressed messages
610+
stream.transportState().setSentMessageEncoding("gzip");
611+
612+
// Server responds with grpc-accept-encoding header
613+
Metadata headers = new Metadata();
614+
headers.put(GrpcUtil.MESSAGE_ACCEPT_ENCODING_KEY, "gzip".getBytes(java.nio.charset.StandardCharsets.US_ASCII));
615+
stream.transportState().inboundHeadersReceived(headers);
616+
617+
// Verify no warning was logged
618+
verify(mockListener).headersRead(headers);
619+
assertThat(logs).isEmpty();
620+
} finally {
621+
logger.removeHandler(handler);
622+
logger.setLevel(originalLevel);
623+
}
624+
}
625+
626+
@Test
627+
public void inboundHeadersReceived_noWarningWhenClientDidNotUseCompression() {
628+
// Set up log capture
629+
Logger logger = Logger.getLogger(AbstractClientStream.class.getName());
630+
Level originalLevel = logger.getLevel();
631+
List<LogRecord> logs = new ArrayList<>();
632+
Handler handler = new Handler() {
633+
@Override
634+
public void publish(LogRecord record) {
635+
logs.add(record);
636+
}
637+
638+
@Override
639+
public void flush() {}
640+
641+
@Override
642+
public void close() {}
643+
};
644+
logger.addHandler(handler);
645+
logger.setLevel(Level.FINE);
646+
647+
try {
648+
AbstractClientStream stream =
649+
new BaseAbstractClientStream(allocator, statsTraceCtx, transportTracer);
650+
stream.start(mockListener);
651+
652+
// Client did not set any encoding (using identity/no compression)
653+
// sentMessageEncoding is null by default
654+
655+
// Server responds without grpc-accept-encoding header
656+
Metadata headers = new Metadata();
657+
stream.transportState().inboundHeadersReceived(headers);
658+
659+
// Verify no warning was logged (client didn't use compression)
660+
verify(mockListener).headersRead(headers);
661+
assertThat(logs).isEmpty();
662+
} finally {
663+
logger.removeHandler(handler);
664+
logger.setLevel(originalLevel);
665+
}
666+
}
667+
530668
/**
531669
* No-op base class for testing.
532670
*/

0 commit comments

Comments
 (0)