From b08062093491167e906f567051ae2ad658c1d197 Mon Sep 17 00:00:00 2001 From: Idel Pivnitskiy Date: Fri, 8 Nov 2024 19:20:00 -0800 Subject: [PATCH] Detect unexpected response leaks for multi-address client instances Motivation: We already apply `HttpMessageDiscardWatchdogClientFilter` to single-address clients to detect response leaks that could be caused by unhandled exceptions in the filter chain. Multi-address client adds more logic around a client group and therefore also could leak responses if case on unexpected exceptions. Modifications: - Apply `HttpMessageDiscardWatchdogClientFilter` to multi-address client instances; - Enhance `HttpMessageDiscardWatchdogClientFilter` to include unhandled exception in the logs to help narrow down the leak cause; Result: Users will see warn message if their multi-address client leaks responses. --- .../http/netty/DefaultMultiAddressUrlHttpClientBuilder.java | 3 +++ .../http/netty/HttpMessageDiscardWatchdogClientFilter.java | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/DefaultMultiAddressUrlHttpClientBuilder.java b/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/DefaultMultiAddressUrlHttpClientBuilder.java index 98367f88d1..02315f1b1b 100644 --- a/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/DefaultMultiAddressUrlHttpClientBuilder.java +++ b/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/DefaultMultiAddressUrlHttpClientBuilder.java @@ -135,6 +135,9 @@ public StreamingHttpClient buildStreaming() { urlClient = redirectConfig == null ? urlClient : new RedirectingHttpRequesterFilter(redirectConfig).create(urlClient); + // Detect leaks that can be caused by unexpected exceptions + urlClient = HttpMessageDiscardWatchdogClientFilter.CLIENT_CLEANER.create(urlClient); + LOGGER.debug("Multi-address client created with base strategy {}", executionContext.executionStrategy()); return new FilterableClientToClient(urlClient, executionContext); } catch (final Throwable t) { diff --git a/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/HttpMessageDiscardWatchdogClientFilter.java b/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/HttpMessageDiscardWatchdogClientFilter.java index 3a38b74c7f..af481c2bf0 100644 --- a/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/HttpMessageDiscardWatchdogClientFilter.java +++ b/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/HttpMessageDiscardWatchdogClientFilter.java @@ -115,7 +115,7 @@ protected Single request(final StreamingHttpRequester del LOGGER.warn("Discovered un-drained HTTP response message body which has " + "been dropped by user code - this is a strong indication of a bug " + "in a user-defined filter. Response payload (message) body must " + - "be fully consumed before discarding."); + "be fully consumed before discarding.", cause); } return Single.failed(cause).shareContextOnSubscribe(); });