bug #62415 [HttpClient] Don't reset unused clients in data collector (HypeMC)

fabpot · fabpot · commit f3cb2868354a · 2025-11-20T08:06:16.000+01:00
This PR was merged into the 6.4 branch. Discussion ---------- [HttpClient] Don't reset unused clients in data collector | Q | A | ------------- | --- | Branch? | 6.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT The data collector resets all clients on each request. When multiple scoped clients use the throttling feature, every rate limiter is reset each time, causing locks that were never used to be acquired only to be immediately released. Since most of the time only one or none of the clients is used, this behavior is unnecessary and inefficient. This PR ensures that only clients that were actually used are reset. Commits ------- 44368f4 [HttpClient] Don't reset unused clients in data collector
diff --git a/src/Symfony/Component/HttpClient/DataCollector/HttpClientDataCollector.php b/src/Symfony/Component/HttpClient/DataCollector/HttpClientDataCollector.php
@@ -64,7 +64,9 @@ public function lateCollect(): void
             $this->data['error_count'] += $errorCount;
             $this->data['clients'][$name]['error_count'] += $errorCount;
 
-            $client->reset();
+            if ($traces) {
+                $client->reset();
+            }
         }
     }
 
diff --git a/src/Symfony/Component/HttpClient/Tests/DataCollector/HttpClientDataCollectorTest.php b/src/Symfony/Component/HttpClient/Tests/DataCollector/HttpClientDataCollectorTest.php
@@ -13,6 +13,7 @@
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\HttpClient\DataCollector\HttpClientDataCollector;
+use Symfony\Component\HttpClient\MockHttpClient;
 use Symfony\Component\HttpClient\NativeHttpClient;
 use Symfony\Component\HttpClient\TraceableHttpClient;
 use Symfony\Contracts\HttpClient\Test\TestHttpServer;
@@ -437,4 +438,44 @@ private function httpClientThatHasTracedRequests($tracedRequests): TraceableHttp
 
         return $httpClient;
     }
+
+    /**
+     * @dataProvider provideClientIsResetWhenExpectedCases
+     */
+    public function testClientIsResetWhenExpected(\Closure $request, bool $wasReset)
+    {
+        $mockHttpClient = new class extends MockHttpClient {
+            public bool $wasReset = false;
+
+            public function reset(): void
+            {
+                parent::reset();
+
+                $this->wasReset = true;
+            }
+        };
+
+        $sut = new HttpClientDataCollector();
+        $sut->registerClient('http_client', $traceableHttpClient = new TraceableHttpClient($mockHttpClient));
+        $request($traceableHttpClient);
+        $sut->lateCollect();
+
+        $this->assertSame($wasReset, $mockHttpClient->wasReset);
+    }
+
+    public static function provideClientIsResetWhenExpectedCases(): iterable
+    {
+        yield [
+            function (TraceableHttpClient $traceableHttpClient) {
+                $response = $traceableHttpClient->request('GET', 'http://localhost/');
+                $response->getContent();
+            },
+            true,
+        ];
+
+        yield [
+            fn () => null,
+            false,
+        ];
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,9 @@ public function lateCollect(): void`
`64`	`64`	`$this->data['error_count'] += $errorCount;`
`65`	`65`	`$this->data['clients'][$name]['error_count'] += $errorCount;`
`66`	`66`
`67`		`- $client->reset();`
	`67`	`+ if ($traces) {`
	`68`	`+ $client->reset();`
	`69`	`+ }`
`68`	`70`	`}`
`69`	`71`	`}`
`70`	`72`