From 81765bf81c63c0b6fa2e1e17db8f335c8a77e1da Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Thu, 24 Apr 2025 18:07:44 -0700 Subject: [PATCH 01/18] Properly handle closeable HttpResponses in test proxy paths. --- .../test/http/TestProxyPlaybackClient.java | 43 +++++++++----- .../core/test/http/TestProxyTestServer.java | 5 +- .../test/policy/TestProxyRecordPolicy.java | 57 +++++++++++++++++-- .../core/test/utils/TestProxyManager.java | 11 ++-- 4 files changed, 90 insertions(+), 26 deletions(-) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java index 5777b3a330c2..31f0a0a9de93 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java @@ -93,15 +93,17 @@ public Queue startPlayback(File recordFile, Path testClassPath) { .setBody(new RecordFilePayload(recordFile.toString(), assetJsonPath).toJsonString()) .setHeader(HttpHeaderName.ACCEPT, "application/json") .setHeader(HttpHeaderName.CONTENT_TYPE, "application/json"); - HttpResponse response = sendRequestWithRetries(request); - checkForTestProxyErrors(response); - xRecordingId = response.getHeaderValue(X_RECORDING_ID); - xRecordingFileLocation - = new String(Base64.getUrlDecoder().decode(response.getHeaders().getValue(X_RECORDING_FILE_LOCATION)), - StandardCharsets.UTF_8); - addProxySanitization(this.sanitizers); - addMatcherRequests(this.matchers); - String body = response.getBodyAsString().block(); + String body; + try (HttpResponse response = sendRequestWithRetries(request)) { + checkForTestProxyErrors(response); + xRecordingId = response.getHeaderValue(X_RECORDING_ID); + xRecordingFileLocation + = new String(Base64.getUrlDecoder().decode(response.getHeaders().getValue(X_RECORDING_FILE_LOCATION)), + StandardCharsets.UTF_8); + addProxySanitization(this.sanitizers); + addMatcherRequests(this.matchers); + body = response.getBodyAsString().block(); + } // The test proxy stores variables in a map with no guaranteed order. // The Java implementation of recording did not use a map, but relied on the order // of the variables as they were stored. Our scheme instead sets an increasing integer @@ -136,8 +138,13 @@ private HttpResponse sendRequestWithRetries(HttpRequest request) { try { HttpResponse response = client.sendSync(request, Context.NONE); if (response.getStatusCode() / 100 != 2) { + String body = response.getBodyAsString().block(); + int statusCode = response.getStatusCode(); + // We don't generally want to close the response here as the caller will need it, + // but if we're throwing we should clean it up. + response.close(); throw new RuntimeException("Test proxy returned a non-successful status code. " - + response.getStatusCode() + "; response: " + response.getBodyAsString().block()); + + statusCode + "; response: " + body); } return response; } catch (Exception e) { @@ -165,7 +172,9 @@ private void sleep(int durationInSeconds) { public void stopPlayback() { HttpRequest request = new HttpRequest(HttpMethod.POST, proxyUrl + "/playback/stop").setHeader(X_RECORDING_ID, xRecordingId); - sendRequestWithRetries(request); + try (HttpResponse response = sendRequestWithRetries(request)) { + checkForTestProxyErrors(response); + } } /** @@ -224,7 +233,9 @@ public void addProxySanitization(List sanitizers) { HttpRequest request = createAddSanitizersRequest(sanitizers, proxyUrl).setHeader(X_RECORDING_ID, xRecordingId); - sendRequestWithRetries(request); + try (HttpResponse response = sendRequestWithRetries(request)) { + checkForTestProxyErrors(response); + } } else { this.sanitizers.addAll(sanitizers); } @@ -250,7 +261,9 @@ public void removeProxySanitization(List sanitizers) { throw new RuntimeException(e); } - sendRequestWithRetries(request); + try (HttpResponse response = sendRequestWithRetries(request)) { + checkForTestProxyErrors(response); + } } } @@ -266,7 +279,9 @@ public void addMatcherRequests(List matchers) { } matcherRequests.forEach(request -> { request.setHeader(X_RECORDING_ID, xRecordingId); - sendRequestWithRetries(request); + try (HttpResponse response = sendRequestWithRetries(request)) { + checkForTestProxyErrors(response); + } }); } else { this.matchers.addAll(matchers); diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyTestServer.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyTestServer.java index be127cdfd159..d3ea84db49ff 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyTestServer.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyTestServer.java @@ -74,6 +74,9 @@ public int port() { @Override public void close() { - server.disposeNow(); + if (server != null) { + server.disposeNow(); + } } } + diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java index 2c6cc542413d..38abf156d549 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java @@ -18,6 +18,7 @@ import com.azure.core.test.utils.HttpURLConnectionHttpClient; import com.azure.core.test.utils.TestProxyUtils; import com.azure.core.util.Context; +import com.azure.core.util.logging.ClientLogger; import com.azure.json.JsonProviders; import com.azure.json.JsonWriter; import reactor.core.publisher.Mono; @@ -32,6 +33,7 @@ import java.util.List; import java.util.Map; import java.util.Queue; +import java.util.concurrent.TimeUnit; import static com.azure.core.test.utils.TestProxyUtils.checkForTestProxyErrors; import static com.azure.core.test.utils.TestProxyUtils.createAddSanitizersRequest; @@ -43,6 +45,7 @@ * A {@link HttpPipelinePolicy} for redirecting traffic through the test proxy for recording. */ public class TestProxyRecordPolicy implements HttpPipelinePolicy { + private static final ClientLogger LOGGER = new ClientLogger(TestProxyRecordPolicy.class); private static final HttpHeaderName X_RECORDING_ID = HttpHeaderName.fromString("x-recording-id"); private final HttpClient client; private final URL proxyUrl; @@ -79,7 +82,7 @@ public void startRecording(File recordFile, Path testClassPath) { .setBody(new RecordFilePayload(recordFile.toString(), assetJsonPath).toJsonString()) .setHeader(HttpHeaderName.CONTENT_TYPE, "application/json"); - try (HttpResponse response = client.sendSync(request, Context.NONE)) { + try (HttpResponse response = sendRequestWithRetries(request)) { checkForTestProxyErrors(response); if (response.getStatusCode() != 200) { throw new RuntimeException(response.getBodyAsBinaryData().toString()); @@ -99,7 +102,9 @@ private void setDefaultRecordingOptions() { HttpRequest request = new HttpRequest(HttpMethod.POST, proxyUrl + "/Admin/SetRecordingOptions") .setBody("{\"HandleRedirects\": false}") .setHeader(HttpHeaderName.CONTENT_TYPE, "application/json"); - client.sendSync(request, Context.NONE).close(); + try (HttpResponse response = sendRequestWithRetries(request)) { + checkForTestProxyErrors(response); + } } /** @@ -113,7 +118,7 @@ public void stopRecording(Queue variables) { .setHeader(X_RECORDING_ID, xRecordingId) .setBody(serializeVariables(variables)); - try (HttpResponse response = client.sendSync(request, Context.NONE)) { + try (HttpResponse response = sendRequestWithRetries(request)) { checkForTestProxyErrors(response); if (response.getStatusCode() != 200) { throw new RuntimeException(response.getBodyAsBinaryData().toString()); @@ -197,7 +202,9 @@ public void addProxySanitization(List sanitizers) { HttpRequest request = createAddSanitizersRequest(sanitizers, proxyUrl).setHeader(X_RECORDING_ID, xRecordingId); - client.sendSync(request, Context.NONE).close(); + try (HttpResponse response = sendRequestWithRetries(request)) { + checkForTestProxyErrors(response); + } } else { this.sanitizers.addAll(sanitizers); } @@ -227,7 +234,9 @@ public void removeProxySanitization(List sanitizers) { throw new RuntimeException(e); } - client.sendSync(request, Context.NONE).close(); + try (HttpResponse response = sendRequestWithRetries(request)) { + checkForTestProxyErrors(response); + } } } @@ -241,9 +250,45 @@ public void setRecordingOptions(TestProxyRecordingOptions testProxyRecordingOpti HttpRequest request = new HttpRequest(HttpMethod.POST, proxyUrl + "/admin/setrecordingoptions") .setBody(testProxyRecordingOptions.toJsonString()) .setHeader(HttpHeaderName.CONTENT_TYPE, "application/json"); - client.sendSync(request, Context.NONE).close(); + try (HttpResponse response = sendRequestWithRetries(request)) { + checkForTestProxyErrors(response); + } } catch (IOException ex) { throw new IllegalArgumentException("Failed to process JSON input", ex); } } + + private HttpResponse sendRequestWithRetries(HttpRequest request) { + int retries = 0; + while (true) { + try { + HttpResponse response = client.sendSync(request, Context.NONE); + if (response.getStatusCode() / 100 != 2) { + String body = response.getBodyAsString().block(); + int statusCode = response.getStatusCode(); + // We don't generally want to close the response here as the caller will need it, + // but if we're throwing we should clean it up. + response.close(); + throw new RuntimeException("Test proxy returned a non-successful status code. " + + statusCode + "; response: " + body); + } + return response; + } catch (Exception e) { + retries++; + if (retries >= 3) { + throw e; + } + sleep(1); + LOGGER.warning("Retrying request to test proxy. Retry attempt: " + retries); + } + } + } + + private void sleep(int durationInSeconds) { + try { + TimeUnit.SECONDS.sleep(durationInSeconds); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } } diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyManager.java index f79342d6ef36..8896db94eeee 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyManager.java @@ -112,12 +112,13 @@ private static boolean checkAlive(int loops, Duration waitTime, Process proxy) t return false; } - try { - HttpResponse response = client.sendSync(request, Context.NONE); - if (response != null && response.getStatusCode() == 200) { - return true; + try (HttpResponse response = client.sendSync(request, Context.NONE)) { + if (response != null) { + if (response.getStatusCode() == 200) { + return true; + } + TestProxyUtils.checkForTestProxyErrors(response); } - TestProxyUtils.checkForTestProxyErrors(response); } catch (Exception ignored) { } From ab6483583eca97229b6e94b71d9274ab410970c4 Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Thu, 24 Apr 2025 18:08:58 -0700 Subject: [PATCH 02/18] spotless --- .../azure/core/test/http/TestProxyPlaybackClient.java | 10 +++++----- .../com/azure/core/test/http/TestProxyTestServer.java | 1 - .../azure/core/test/policy/TestProxyRecordPolicy.java | 4 ++-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java index 31f0a0a9de93..7c8446671fd8 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java @@ -97,9 +97,9 @@ public Queue startPlayback(File recordFile, Path testClassPath) { try (HttpResponse response = sendRequestWithRetries(request)) { checkForTestProxyErrors(response); xRecordingId = response.getHeaderValue(X_RECORDING_ID); - xRecordingFileLocation - = new String(Base64.getUrlDecoder().decode(response.getHeaders().getValue(X_RECORDING_FILE_LOCATION)), - StandardCharsets.UTF_8); + xRecordingFileLocation = new String( + Base64.getUrlDecoder().decode(response.getHeaders().getValue(X_RECORDING_FILE_LOCATION)), + StandardCharsets.UTF_8); addProxySanitization(this.sanitizers); addMatcherRequests(this.matchers); body = response.getBodyAsString().block(); @@ -143,8 +143,8 @@ private HttpResponse sendRequestWithRetries(HttpRequest request) { // We don't generally want to close the response here as the caller will need it, // but if we're throwing we should clean it up. response.close(); - throw new RuntimeException("Test proxy returned a non-successful status code. " - + statusCode + "; response: " + body); + throw new RuntimeException( + "Test proxy returned a non-successful status code. " + statusCode + "; response: " + body); } return response; } catch (Exception e) { diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyTestServer.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyTestServer.java index d3ea84db49ff..4d36a3482560 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyTestServer.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyTestServer.java @@ -79,4 +79,3 @@ public void close() { } } } - diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java index 38abf156d549..2954d059e485 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java @@ -269,8 +269,8 @@ private HttpResponse sendRequestWithRetries(HttpRequest request) { // We don't generally want to close the response here as the caller will need it, // but if we're throwing we should clean it up. response.close(); - throw new RuntimeException("Test proxy returned a non-successful status code. " - + statusCode + "; response: " + body); + throw new RuntimeException( + "Test proxy returned a non-successful status code. " + statusCode + "; response: " + body); } return response; } catch (Exception e) { From 23400457474d8c8fd203a352e9dd3cb63266eb16 Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Fri, 25 Apr 2025 11:36:45 -0700 Subject: [PATCH 03/18] audit log --- eng/pipelines/templates/jobs/ci.tests.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/eng/pipelines/templates/jobs/ci.tests.yml b/eng/pipelines/templates/jobs/ci.tests.yml index 6195ff3979ac..2d58dc43d779 100644 --- a/eng/pipelines/templates/jobs/ci.tests.yml +++ b/eng/pipelines/templates/jobs/ci.tests.yml @@ -174,3 +174,11 @@ jobs: } displayName: Dump Test-Proxy Error Logs condition: succeededOrFailed() + + - pwsh: | + $content = (Invoke-WebRequest -Uri "http://localhost:5000/Audit/Logs") + Write-Host $content.Content + displayName: Dump Test-Proxy Audit Logs + condition: succeededOrFailed() + + - template: /eng/common/testproxy/test-proxy-tool-shutdown.yml \ No newline at end of file From bdaacf86eebc592d44cee4df374d79e5e882a29d Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Fri, 25 Apr 2025 11:58:57 -0700 Subject: [PATCH 04/18] remove extra shutdown --- eng/pipelines/templates/jobs/ci.tests.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/eng/pipelines/templates/jobs/ci.tests.yml b/eng/pipelines/templates/jobs/ci.tests.yml index 2d58dc43d779..a7b1214891fd 100644 --- a/eng/pipelines/templates/jobs/ci.tests.yml +++ b/eng/pipelines/templates/jobs/ci.tests.yml @@ -150,11 +150,7 @@ jobs: VersionOverride: ${{ parameters.VersionOverride }} - template: /eng/pipelines/templates/steps/post-job-cleanup.yml - - # Shut down proxy to prevent file locks on the log file for auto-injected credscan - # steps on windows - - template: /eng/common/testproxy/test-proxy-tool-shutdown.yml - + - pwsh: | $files = Get-ChildItem -Path $(Build.SourcesDirectory) -Filter test-proxy.log foreach($file in $files){ From c31183bacc896eb86806165f7135a9c749e1c26d Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Fri, 25 Apr 2025 14:52:38 -0700 Subject: [PATCH 05/18] more loogging --- .../java/com/azure/core/test/http/TestProxyPlaybackClient.java | 2 ++ .../java/com/azure/core/test/policy/TestProxyRecordPolicy.java | 3 +++ 2 files changed, 5 insertions(+) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java index 7c8446671fd8..f8d35e33ffac 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java @@ -149,6 +149,8 @@ private HttpResponse sendRequestWithRetries(HttpRequest request) { return response; } catch (Exception e) { retries++; + + LOGGER.error("Failed in the retry loop. RecordingID " + xRecordingId + " message: " + e.getMessage()); if (retries >= 3) { throw e; } diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java index 2954d059e485..a1d3b12933e9 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/policy/TestProxyRecordPolicy.java @@ -274,6 +274,9 @@ private HttpResponse sendRequestWithRetries(HttpRequest request) { } return response; } catch (Exception e) { + + LOGGER.error("Failed in the retry loop. RecordingID " + xRecordingId + " message: " + e.getMessage()); + retries++; if (retries >= 3) { throw e; From c2248d1193dfa333b3a6dae40abb4765c44e376d Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Mon, 28 Apr 2025 15:48:52 -0700 Subject: [PATCH 06/18] wiretap --- sdk/core/azure-core-test/pom.xml | 20 +++++++++++++ .../com/azure/core/test/TestProxyTests.java | 29 +++++++++++++++---- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/sdk/core/azure-core-test/pom.xml b/sdk/core/azure-core-test/pom.xml index 3a26f23e58c0..75489d487376 100644 --- a/sdk/core/azure-core-test/pom.xml +++ b/sdk/core/azure-core-test/pom.xml @@ -133,6 +133,26 @@ + + org.apache.maven.plugins + maven-surefire-plugin + + + + * + * + + + TRACE + TRACE + TRACE + TRACE + + + INFO + + + org.apache.maven.plugins maven-enforcer-plugin diff --git a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java index 46d5c39d903b..ef9c18b9c81d 100644 --- a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java +++ b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java @@ -11,6 +11,7 @@ import com.azure.core.http.HttpPipelineBuilder; import com.azure.core.http.HttpRequest; import com.azure.core.http.HttpResponse; +import com.azure.core.http.netty.NettyAsyncHttpClientBuilder; import com.azure.core.http.policy.RedirectPolicy; import com.azure.core.test.annotation.DoNotRecord; import com.azure.core.test.annotation.RecordWithoutRequestBody; @@ -19,19 +20,20 @@ import com.azure.core.test.models.CustomMatcher; import com.azure.core.test.models.TestProxySanitizer; import com.azure.core.test.models.TestProxySanitizerType; -import com.azure.core.test.utils.HttpURLConnectionHttpClient; import com.azure.core.test.utils.TestProxyUtils; import com.azure.core.util.Context; import com.azure.json.JsonProviders; import com.azure.json.JsonReader; import com.azure.json.JsonSerializable; import com.azure.json.JsonWriter; +import io.netty.handler.logging.LogLevel; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledIfEnvironmentVariable; import org.junit.jupiter.api.condition.DisabledIfSystemProperty; +import reactor.netty.transport.logging.AdvancedByteBufFormat; import java.io.BufferedReader; import java.io.IOException; @@ -69,6 +71,8 @@ public class TestProxyTests extends TestProxyTestBase { private static final HttpHeaderName OCP_APIM_SUBSCRIPTION_KEY = HttpHeaderName.fromString("Ocp-Apim-Subscription-Key"); + private HttpClient wiretapClient; + static { CUSTOM_SANITIZER.add(new TestProxySanitizer("$..modelId", null, REDACTED, TestProxySanitizerType.BODY_KEY)); CUSTOM_SANITIZER.add(new TestProxySanitizer("TableName\\\"*:*\\\"(?.*)\\\"", REDACTED, @@ -85,10 +89,22 @@ public static void teardownClass() { server.close(); } + @Override + protected void beforeTest() { + super.beforeTest(); + reactor.netty.http.client.HttpClient client = reactor.netty.http.client.HttpClient.create().wiretap("reactor.netty.http.client.HttpClient", LogLevel.TRACE, AdvancedByteBufFormat.TEXTUAL); + wiretapClient = new NettyAsyncHttpClientBuilder(client).build(); + interceptorManager.setHttpClient(wiretapClient); + } + + private HttpClient getRecordHttpClient() { + return wiretapClient; + } + @Test @Tag("Record") public void testBasicRecord() { - HttpURLConnectionHttpClient client = new HttpURLConnectionHttpClient(); + HttpClient client = getRecordHttpClient(); HttpPipeline pipeline = new HttpPipelineBuilder().httpClient(client).policies(interceptorManager.getRecordPolicy()).build(); @@ -137,7 +153,8 @@ public void testMismatch() { @Tag("Record") @RecordWithoutRequestBody public void testRecordWithPath() { - HttpURLConnectionHttpClient client = new HttpURLConnectionHttpClient(); + + HttpClient client = getRecordHttpClient(); HttpPipeline pipeline = new HttpPipelineBuilder().httpClient(client).policies(interceptorManager.getRecordPolicy()).build(); @@ -156,7 +173,7 @@ public void testRecordWithPath() { @Test @Tag("Record") public void testRecordWithHeaders() { - HttpURLConnectionHttpClient client = new HttpURLConnectionHttpClient(); + HttpClient client = getRecordHttpClient(); HttpPipeline pipeline = new HttpPipelineBuilder().httpClient(client).policies(interceptorManager.getRecordPolicy()).build(); @@ -328,7 +345,7 @@ public void canGetTestProxyVersion() { @Test @Tag("Record") public void testResetTestProxyData() { - HttpURLConnectionHttpClient client = new HttpURLConnectionHttpClient(); + HttpClient client = getRecordHttpClient(); final HttpPipeline pipeline = new HttpPipelineBuilder().httpClient(client).policies(interceptorManager.getRecordPolicy()).build(); @@ -347,7 +364,7 @@ public void testResetTestProxyData() { @Test @Tag("Record") public void testRecordWithRedirect() { - HttpURLConnectionHttpClient client = new HttpURLConnectionHttpClient(); + HttpClient client = getRecordHttpClient(); HttpPipeline pipeline = new HttpPipelineBuilder().httpClient(client) .policies(new RedirectPolicy(), interceptorManager.getRecordPolicy()) From 4a3b617934fc00585735ff0644a4a3587daf0db3 Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Wed, 30 Apr 2025 15:21:32 -0700 Subject: [PATCH 07/18] move extra logging to parent pom --- sdk/core/azure-core-test/pom.xml | 20 -------------------- sdk/parents/azure-client-sdk-parent/pom.xml | 9 +++++++++ 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/sdk/core/azure-core-test/pom.xml b/sdk/core/azure-core-test/pom.xml index 75489d487376..3a26f23e58c0 100644 --- a/sdk/core/azure-core-test/pom.xml +++ b/sdk/core/azure-core-test/pom.xml @@ -133,26 +133,6 @@ - - org.apache.maven.plugins - maven-surefire-plugin - - - - * - * - - - TRACE - TRACE - TRACE - TRACE - - - INFO - - - org.apache.maven.plugins maven-enforcer-plugin diff --git a/sdk/parents/azure-client-sdk-parent/pom.xml b/sdk/parents/azure-client-sdk-parent/pom.xml index 6c3617728832..acb7c49c2434 100644 --- a/sdk/parents/azure-client-sdk-parent/pom.xml +++ b/sdk/parents/azure-client-sdk-parent/pom.xml @@ -929,6 +929,15 @@ dd MMM yyyy HH:mm:ss,SSS info debug + + * + * + + + TRACE + TRACE + TRACE + TRACE 1 false From 4498878a27b0dc432b27abef5fd1f4a91c3d4dfd Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Wed, 30 Apr 2025 15:22:00 -0700 Subject: [PATCH 08/18] try okhttp for everything just to see --- .../java/com/azure/core/test/InterceptorManager.java | 6 +++++- .../com/azure/core/test/utils/TestProxyUtils.java | 2 +- .../test/java/com/azure/core/test/TestProxyTests.java | 11 ----------- 3 files changed, 6 insertions(+), 13 deletions(-) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java index 1bb85bbf65ca..89de39d7ce95 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java @@ -3,6 +3,7 @@ package com.azure.core.test; import com.azure.core.http.HttpClient; +import com.azure.core.http.okhttp.OkHttpAsyncHttpClientBuilder; import com.azure.core.http.policy.HttpPipelinePolicy; import com.azure.core.test.http.PlaybackClient; import com.azure.core.test.http.TestProxyPlaybackClient; @@ -331,7 +332,10 @@ public HttpClient getPlaybackClient() { throw new IllegalStateException("A playback client can only be requested in PLAYBACK mode."); } if (testProxyPlaybackClient == null) { - testProxyPlaybackClient = new TestProxyPlaybackClient(httpClient, skipRecordingRequestBody); +// reactor.netty.http.client.HttpClient client = reactor.netty.http.client.HttpClient.create().wiretap("reactor.netty.http.client.HttpClient", LogLevel.TRACE, AdvancedByteBufFormat.TEXTUAL); +// HttpClient localClient = new NettyAsyncHttpClientBuilder(client).build(); + HttpClient localClient = new OkHttpAsyncHttpClientBuilder().build(); + testProxyPlaybackClient = new TestProxyPlaybackClient(localClient, skipRecordingRequestBody); proxyVariableQueue .addAll(testProxyPlaybackClient.startPlayback(getTestProxyRecordFile(), testClassPath)); xRecordingFileLocation = testProxyPlaybackClient.getRecordingFileLocation(); diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java index 25f3b8adbb4e..3aa8ac0f16a2 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java @@ -290,7 +290,7 @@ public static List loadSanitizers() { private static String createCustomMatcherRequestBody(CustomMatcher customMatcher) { return String.format( - "{\"ignoredHeaders\":\"%s\",\"excludedHeaders\":\"%s\",\"compareBodies\":%s,\"ignoredQueryParameters\":\"%s\",\"ignoreQueryOrdering\":%s}", + "{\"ignoredHeaders\":\"%s\",\"excludedHeaders\":\"Accept, Accept-Encoding, %s\",\"compareBodies\":%s,\"ignoredQueryParameters\":\"%s\",\"ignoreQueryOrdering\":%s}", getCommaSeperatedString(customMatcher.getHeadersKeyOnlyMatch()), getCommaSeperatedString(customMatcher.getExcludedHeaders()), customMatcher.isComparingBodies(), getCommaSeperatedString(customMatcher.getIgnoredQueryParameters()), customMatcher.isQueryOrderingIgnored()); diff --git a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java index ef9c18b9c81d..112cd47c15ef 100644 --- a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java +++ b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java @@ -11,7 +11,6 @@ import com.azure.core.http.HttpPipelineBuilder; import com.azure.core.http.HttpRequest; import com.azure.core.http.HttpResponse; -import com.azure.core.http.netty.NettyAsyncHttpClientBuilder; import com.azure.core.http.policy.RedirectPolicy; import com.azure.core.test.annotation.DoNotRecord; import com.azure.core.test.annotation.RecordWithoutRequestBody; @@ -26,14 +25,12 @@ import com.azure.json.JsonReader; import com.azure.json.JsonSerializable; import com.azure.json.JsonWriter; -import io.netty.handler.logging.LogLevel; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledIfEnvironmentVariable; import org.junit.jupiter.api.condition.DisabledIfSystemProperty; -import reactor.netty.transport.logging.AdvancedByteBufFormat; import java.io.BufferedReader; import java.io.IOException; @@ -89,14 +86,6 @@ public static void teardownClass() { server.close(); } - @Override - protected void beforeTest() { - super.beforeTest(); - reactor.netty.http.client.HttpClient client = reactor.netty.http.client.HttpClient.create().wiretap("reactor.netty.http.client.HttpClient", LogLevel.TRACE, AdvancedByteBufFormat.TEXTUAL); - wiretapClient = new NettyAsyncHttpClientBuilder(client).build(); - interceptorManager.setHttpClient(wiretapClient); - } - private HttpClient getRecordHttpClient() { return wiretapClient; } From d944459bf46b07955bd19a37712e64f3c40acf2c Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Thu, 1 May 2025 11:34:16 -0700 Subject: [PATCH 09/18] also ignore Connect header --- .../src/main/java/com/azure/core/test/utils/TestProxyUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java index 3aa8ac0f16a2..aa9136ed1060 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java @@ -290,7 +290,7 @@ public static List loadSanitizers() { private static String createCustomMatcherRequestBody(CustomMatcher customMatcher) { return String.format( - "{\"ignoredHeaders\":\"%s\",\"excludedHeaders\":\"Accept, Accept-Encoding, %s\",\"compareBodies\":%s,\"ignoredQueryParameters\":\"%s\",\"ignoreQueryOrdering\":%s}", + "{\"ignoredHeaders\":\"%s\",\"excludedHeaders\":\"Accept, Accept-Encoding, Connect, %s\",\"compareBodies\":%s,\"ignoredQueryParameters\":\"%s\",\"ignoreQueryOrdering\":%s}", getCommaSeperatedString(customMatcher.getHeadersKeyOnlyMatch()), getCommaSeperatedString(customMatcher.getExcludedHeaders()), customMatcher.isComparingBodies(), getCommaSeperatedString(customMatcher.getIgnoredQueryParameters()), customMatcher.isQueryOrderingIgnored()); From 9e1bb9ccb132d67c9c0074a2622f7e90dd48ae8b Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Mon, 5 May 2025 13:16:49 -0700 Subject: [PATCH 10/18] one more header diff --- .../src/main/java/com/azure/core/test/utils/TestProxyUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java index aa9136ed1060..3c834285c39b 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java @@ -290,7 +290,7 @@ public static List loadSanitizers() { private static String createCustomMatcherRequestBody(CustomMatcher customMatcher) { return String.format( - "{\"ignoredHeaders\":\"%s\",\"excludedHeaders\":\"Accept, Accept-Encoding, Connect, %s\",\"compareBodies\":%s,\"ignoredQueryParameters\":\"%s\",\"ignoreQueryOrdering\":%s}", + "{\"ignoredHeaders\":\"%s\",\"excludedHeaders\":\"Accept, Accept-Encoding, Connect, Connection, %s\",\"compareBodies\":%s,\"ignoredQueryParameters\":\"%s\",\"ignoreQueryOrdering\":%s}", getCommaSeperatedString(customMatcher.getHeadersKeyOnlyMatch()), getCommaSeperatedString(customMatcher.getExcludedHeaders()), customMatcher.isComparingBodies(), getCommaSeperatedString(customMatcher.getIgnoredQueryParameters()), customMatcher.isQueryOrderingIgnored()); From f353fb3a4517ea68e68348fe3d6664a6006a47f5 Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Mon, 5 May 2025 16:53:43 -0700 Subject: [PATCH 11/18] ever more logging --- .../azure/core/test/http/TestProxyPlaybackClient.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java index f8d35e33ffac..9c340077aac5 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java @@ -4,6 +4,7 @@ package com.azure.core.test.http; import com.azure.core.http.HttpClient; +import com.azure.core.http.HttpHeader; import com.azure.core.http.HttpHeaderName; import com.azure.core.http.HttpMethod; import com.azure.core.http.HttpRequest; @@ -136,6 +137,15 @@ private HttpResponse sendRequestWithRetries(HttpRequest request) { int retries = 0; while (true) { try { + String message = "Sending request to test proxy. RecordingID " + xRecordingId + + " request: " + request.getUrl(); + for(HttpHeader entry : request.getHeaders()) { + message += " " + entry.getName() + ": " + entry.getValue(); + } + if (request.getBody() != null) { + message += " body: " + request.getBodyAsBinaryData().toString(); + } + LOGGER.verbose(message); HttpResponse response = client.sendSync(request, Context.NONE); if (response.getStatusCode() / 100 != 2) { String body = response.getBodyAsString().block(); From 0c65528d1948ce484ece51a704e14f2eb3f22c0c Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Tue, 6 May 2025 16:22:26 -0700 Subject: [PATCH 12/18] need to be clever-er about tests with no matchers and also tests with bodiless matchers. --- .../com/azure/core/test/http/TestProxyPlaybackClient.java | 5 +++++ .../java/com/azure/core/test/utils/TestProxyUtils.java | 7 +++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java index 9c340077aac5..0e0db504c647 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java @@ -9,6 +9,7 @@ import com.azure.core.http.HttpMethod; import com.azure.core.http.HttpRequest; import com.azure.core.http.HttpResponse; +import com.azure.core.test.models.CustomMatcher; import com.azure.core.test.models.RecordFilePayload; import com.azure.core.test.models.TestProxyRequestMatcher; import com.azure.core.test.models.TestProxySanitizer; @@ -30,6 +31,7 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.Base64; +import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.LinkedList; @@ -285,6 +287,9 @@ public void removeProxySanitization(List sanitizers) { */ public void addMatcherRequests(List matchers) { if (isPlayingBack()) { + if (matchers.isEmpty()) { + matchers.add(new CustomMatcher().setExcludedHeaders(Collections.singletonList("Connect"))); + } List matcherRequests = getMatcherRequests(matchers, proxyUrl); if (skipRecordingRequestBody) { matcherRequests.add(TestProxyUtils.setCompareBodiesMatcher()); diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java index 3c834285c39b..90670b7143fd 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java @@ -476,8 +476,11 @@ public static List getMatcherRequests(List break; case BODILESS: - matcherType = TestProxyRequestMatcher.TestProxyRequestMatcherType.BODILESS.getName(); - request = new HttpRequest(HttpMethod.POST, proxyUrl + "/Admin/setmatcher"); +// matcherType = TestProxyRequestMatcher.TestProxyRequestMatcherType.BODILESS.getName(); + CustomMatcher fakeBodilessMatcher = new CustomMatcher().setComparingBodies(false); + String fakeRequestBody = createCustomMatcherRequestBody(fakeBodilessMatcher); + matcherType = TestProxyRequestMatcher.TestProxyRequestMatcherType.CUSTOM.getName(); + request = new HttpRequest(HttpMethod.POST, proxyUrl + "/Admin/setmatcher").setBody(fakeRequestBody); break; case CUSTOM: From 37e381d0871254390c8f005a005a579378339e4f Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Fri, 9 May 2025 10:02:06 -0700 Subject: [PATCH 13/18] swap back to netty to gather more log data --- .../java/com/azure/core/test/InterceptorManager.java | 9 +++++---- .../java/com/azure/core/test/utils/TestProxyUtils.java | 9 +++------ sdk/core/azure-core-test/src/main/java/module-info.java | 1 + 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java index 89de39d7ce95..23f0160088b6 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java @@ -3,7 +3,7 @@ package com.azure.core.test; import com.azure.core.http.HttpClient; -import com.azure.core.http.okhttp.OkHttpAsyncHttpClientBuilder; +import com.azure.core.http.netty.NettyAsyncHttpClientBuilder; import com.azure.core.http.policy.HttpPipelinePolicy; import com.azure.core.test.http.PlaybackClient; import com.azure.core.test.http.TestProxyPlaybackClient; @@ -21,6 +21,8 @@ import com.azure.json.JsonProviders; import com.azure.json.JsonReader; import com.azure.json.JsonWriter; +import io.netty.handler.logging.LogLevel; +import reactor.netty.transport.logging.AdvancedByteBufFormat; import java.io.BufferedReader; import java.io.BufferedWriter; @@ -332,9 +334,8 @@ public HttpClient getPlaybackClient() { throw new IllegalStateException("A playback client can only be requested in PLAYBACK mode."); } if (testProxyPlaybackClient == null) { -// reactor.netty.http.client.HttpClient client = reactor.netty.http.client.HttpClient.create().wiretap("reactor.netty.http.client.HttpClient", LogLevel.TRACE, AdvancedByteBufFormat.TEXTUAL); -// HttpClient localClient = new NettyAsyncHttpClientBuilder(client).build(); - HttpClient localClient = new OkHttpAsyncHttpClientBuilder().build(); + reactor.netty.http.client.HttpClient client = reactor.netty.http.client.HttpClient.create().wiretap("reactor.netty.http.client.HttpClient", LogLevel.TRACE, AdvancedByteBufFormat.TEXTUAL); + HttpClient localClient = new NettyAsyncHttpClientBuilder(client).build(); testProxyPlaybackClient = new TestProxyPlaybackClient(localClient, skipRecordingRequestBody); proxyVariableQueue .addAll(testProxyPlaybackClient.startPlayback(getTestProxyRecordFile(), testClassPath)); diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java index 90670b7143fd..25f3b8adbb4e 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/utils/TestProxyUtils.java @@ -290,7 +290,7 @@ public static List loadSanitizers() { private static String createCustomMatcherRequestBody(CustomMatcher customMatcher) { return String.format( - "{\"ignoredHeaders\":\"%s\",\"excludedHeaders\":\"Accept, Accept-Encoding, Connect, Connection, %s\",\"compareBodies\":%s,\"ignoredQueryParameters\":\"%s\",\"ignoreQueryOrdering\":%s}", + "{\"ignoredHeaders\":\"%s\",\"excludedHeaders\":\"%s\",\"compareBodies\":%s,\"ignoredQueryParameters\":\"%s\",\"ignoreQueryOrdering\":%s}", getCommaSeperatedString(customMatcher.getHeadersKeyOnlyMatch()), getCommaSeperatedString(customMatcher.getExcludedHeaders()), customMatcher.isComparingBodies(), getCommaSeperatedString(customMatcher.getIgnoredQueryParameters()), customMatcher.isQueryOrderingIgnored()); @@ -476,11 +476,8 @@ public static List getMatcherRequests(List break; case BODILESS: -// matcherType = TestProxyRequestMatcher.TestProxyRequestMatcherType.BODILESS.getName(); - CustomMatcher fakeBodilessMatcher = new CustomMatcher().setComparingBodies(false); - String fakeRequestBody = createCustomMatcherRequestBody(fakeBodilessMatcher); - matcherType = TestProxyRequestMatcher.TestProxyRequestMatcherType.CUSTOM.getName(); - request = new HttpRequest(HttpMethod.POST, proxyUrl + "/Admin/setmatcher").setBody(fakeRequestBody); + matcherType = TestProxyRequestMatcher.TestProxyRequestMatcherType.BODILESS.getName(); + request = new HttpRequest(HttpMethod.POST, proxyUrl + "/Admin/setmatcher"); break; case CUSTOM: diff --git a/sdk/core/azure-core-test/src/main/java/module-info.java b/sdk/core/azure-core-test/src/main/java/module-info.java index ab7d1b01d2c4..558a22584a9f 100644 --- a/sdk/core/azure-core-test/src/main/java/module-info.java +++ b/sdk/core/azure-core-test/src/main/java/module-info.java @@ -18,6 +18,7 @@ requires reactor.netty.core; requires io.netty.codec.http; requires ant; + requires io.netty.handler; exports com.azure.core.test; exports com.azure.core.test.annotation; From d775c4db05d39781d826c3aa8b64ea6f17371168 Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Fri, 9 May 2025 14:19:34 -0700 Subject: [PATCH 14/18] try handling the inactive state --- .../http/netty/implementation/AzureSdkHandler.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/implementation/AzureSdkHandler.java b/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/implementation/AzureSdkHandler.java index f4600bc5fbc4..70e1e722d11b 100644 --- a/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/implementation/AzureSdkHandler.java +++ b/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/implementation/AzureSdkHandler.java @@ -83,6 +83,17 @@ public void handlerAdded(ChannelHandlerContext ctx) { @Override public void handlerRemoved(ChannelHandlerContext ctx) { + disposeTimeouts(); + } + + @Override + public void channelInactive(ChannelHandlerContext ctx) throws Exception { + disposeTimeouts(); + ctx.fireChannelInactive(); + ctx.pipeline().remove(this); + } + + private void disposeTimeouts() { disposeWriteTimeoutWatcher(); disposeResponseTimeoutWatcher(); disposeReadTimeoutWatcher(); From e5f0de4dab6d8cbd9b50ec552905edbb0c73a891 Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Fri, 9 May 2025 16:01:15 -0700 Subject: [PATCH 15/18] add a logger --- .../azure/core/http/netty/implementation/AzureSdkHandler.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/implementation/AzureSdkHandler.java b/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/implementation/AzureSdkHandler.java index 70e1e722d11b..d1bacf7c0af7 100644 --- a/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/implementation/AzureSdkHandler.java +++ b/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/implementation/AzureSdkHandler.java @@ -4,6 +4,7 @@ package com.azure.core.http.netty.implementation; import com.azure.core.util.ProgressReporter; +import com.azure.core.util.logging.ClientLogger; import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBufHolder; import io.netty.channel.ChannelDuplexHandler; @@ -31,7 +32,7 @@ public final class AzureSdkHandler extends ChannelDuplexHandler { * Name of the handler when it's added into a ChannelPipeline. */ public static final String HANDLER_NAME = "azureSdkHandler"; - + private static final ClientLogger LOGGER = new ClientLogger(AzureSdkHandler.class); private final long writeTimeoutMillis; private final ProgressReporter progressReporter; private long lastWriteMillis; @@ -88,6 +89,7 @@ public void handlerRemoved(ChannelHandlerContext ctx) { @Override public void channelInactive(ChannelHandlerContext ctx) throws Exception { + LOGGER.verbose("In channelInactive for " + ctx.name()); disposeTimeouts(); ctx.fireChannelInactive(); ctx.pipeline().remove(this); From f43f8d8d8d632940ef35dfe689776973d114d3b0 Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Mon, 12 May 2025 09:04:18 -0700 Subject: [PATCH 16/18] remove the handler --- .../com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java b/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java index 18c9b4bb7f35..9808bc53eb53 100644 --- a/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java +++ b/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java @@ -200,8 +200,7 @@ public com.azure.core.http.HttpClient build() { (int) getTimeout(connectTimeout, getDefaultConnectTimeout()).toMillis()) // TODO (alzimmer): What does validating HTTP response headers get us? .httpResponseDecoder(httpResponseDecoderSpec -> initialSpec.validateHeaders(false)) - .doOnRequest( - (request, connection) -> addHandler(request, connection, writeTimeout, responseTimeout, readTimeout)) + .doAfterResponseSuccess((ignored, connection) -> removeHandler(connection)); LoggingHandler loggingHandler = nettyHttpClient.configuration().loggingHandler(); From 2ca4325b8039099048459e084eccede6a0be1d66 Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Mon, 12 May 2025 14:04:22 -0700 Subject: [PATCH 17/18] fix build for testing --- .../http/netty/NettyAsyncHttpClientBuilder.java | 11 ++++------- .../http/netty/implementation/AzureSdkHandler.java | 13 ++----------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java b/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java index 9808bc53eb53..e4f7c8f6ceb2 100644 --- a/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java +++ b/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/NettyAsyncHttpClientBuilder.java @@ -40,9 +40,6 @@ import java.util.regex.Pattern; import static com.azure.core.implementation.util.HttpUtils.getDefaultConnectTimeout; -import static com.azure.core.implementation.util.HttpUtils.getDefaultReadTimeout; -import static com.azure.core.implementation.util.HttpUtils.getDefaultResponseTimeout; -import static com.azure.core.implementation.util.HttpUtils.getDefaultWriteTimeout; import static com.azure.core.implementation.util.HttpUtils.getTimeout; /** @@ -186,10 +183,10 @@ public com.azure.core.http.HttpClient build() { nettyHttpClient = HttpClient.create().resolver(DefaultAddressResolverGroup.INSTANCE); addressResolverWasSetByBuilder = true; } - - long writeTimeout = getTimeout(this.writeTimeout, getDefaultWriteTimeout()).toMillis(); - long responseTimeout = getTimeout(this.responseTimeout, getDefaultResponseTimeout()).toMillis(); - long readTimeout = getTimeout(this.readTimeout, getDefaultReadTimeout()).toMillis(); + // + // long writeTimeout = getTimeout(this.writeTimeout, getDefaultWriteTimeout()).toMillis(); + // long responseTimeout = getTimeout(this.responseTimeout, getDefaultResponseTimeout()).toMillis(); + // long readTimeout = getTimeout(this.readTimeout, getDefaultReadTimeout()).toMillis(); // Get the initial HttpResponseDecoderSpec and update it. // .httpResponseDecoder passes a new HttpResponseDecoderSpec and any existing configuration should be updated diff --git a/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/implementation/AzureSdkHandler.java b/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/implementation/AzureSdkHandler.java index d1bacf7c0af7..a54dcd8dfb25 100644 --- a/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/implementation/AzureSdkHandler.java +++ b/sdk/core/azure-core-http-netty/src/main/java/com/azure/core/http/netty/implementation/AzureSdkHandler.java @@ -82,23 +82,14 @@ public void handlerAdded(ChannelHandlerContext ctx) { this.ctx = ctx; } - @Override - public void handlerRemoved(ChannelHandlerContext ctx) { - disposeTimeouts(); - } - @Override public void channelInactive(ChannelHandlerContext ctx) throws Exception { LOGGER.verbose("In channelInactive for " + ctx.name()); - disposeTimeouts(); - ctx.fireChannelInactive(); - ctx.pipeline().remove(this); - } - - private void disposeTimeouts() { disposeWriteTimeoutWatcher(); disposeResponseTimeoutWatcher(); disposeReadTimeoutWatcher(); + ctx.fireChannelInactive(); + ctx.pipeline().remove(this); } /** From 9ceb1a50d385141e95ae046f30be6c4c9ff34d3b Mon Sep 17 00:00:00 2001 From: Bill Wert Date: Mon, 12 May 2025 14:31:45 -0700 Subject: [PATCH 18/18] more build fixes --- .../main/java/com/azure/core/test/InterceptorManager.java | 3 ++- .../com/azure/core/test/http/TestProxyPlaybackClient.java | 6 +++--- .../src/test/java/com/azure/core/test/TestProxyTests.java | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java index 23f0160088b6..a6e33179ef76 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/InterceptorManager.java @@ -334,7 +334,8 @@ public HttpClient getPlaybackClient() { throw new IllegalStateException("A playback client can only be requested in PLAYBACK mode."); } if (testProxyPlaybackClient == null) { - reactor.netty.http.client.HttpClient client = reactor.netty.http.client.HttpClient.create().wiretap("reactor.netty.http.client.HttpClient", LogLevel.TRACE, AdvancedByteBufFormat.TEXTUAL); + reactor.netty.http.client.HttpClient client = reactor.netty.http.client.HttpClient.create() + .wiretap("reactor.netty.http.client.HttpClient", LogLevel.TRACE, AdvancedByteBufFormat.TEXTUAL); HttpClient localClient = new NettyAsyncHttpClientBuilder(client).build(); testProxyPlaybackClient = new TestProxyPlaybackClient(localClient, skipRecordingRequestBody); proxyVariableQueue diff --git a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java index 0e0db504c647..7db30c830673 100644 --- a/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java +++ b/sdk/core/azure-core-test/src/main/java/com/azure/core/test/http/TestProxyPlaybackClient.java @@ -139,9 +139,9 @@ private HttpResponse sendRequestWithRetries(HttpRequest request) { int retries = 0; while (true) { try { - String message = "Sending request to test proxy. RecordingID " + xRecordingId - + " request: " + request.getUrl(); - for(HttpHeader entry : request.getHeaders()) { + String message + = "Sending request to test proxy. RecordingID " + xRecordingId + " request: " + request.getUrl(); + for (HttpHeader entry : request.getHeaders()) { message += " " + entry.getName() + ": " + entry.getValue(); } if (request.getBody() != null) { diff --git a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java index 112cd47c15ef..c1bf197ef3a1 100644 --- a/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java +++ b/sdk/core/azure-core-test/src/test/java/com/azure/core/test/TestProxyTests.java @@ -142,7 +142,7 @@ public void testMismatch() { @Tag("Record") @RecordWithoutRequestBody public void testRecordWithPath() { - + HttpClient client = getRecordHttpClient(); HttpPipeline pipeline = new HttpPipelineBuilder().httpClient(client).policies(interceptorManager.getRecordPolicy()).build();