From 8ad2821e0f6354c7b250dfe3672231b8ff0ab11b Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Tue, 24 Feb 2026 10:17:34 +0100 Subject: [PATCH] fix(legacy): resolve flaky TestApiCurlCommand by fixing 401 detection race condition The api:curl 401 retry mechanism parsed curl's verbose stderr in individual chunks via a Process callback. With --fail-with-body (default), curl exits quickly with code 22 on 4xx, and the stderr chunk containing the HTTP status line could be missed due to chunk boundary splits or process exit timing. Accumulate all stderr during process execution and check the full buffer for 401 after the process completes, eliminating the race condition. Co-Authored-By: Claude Opus 4.6 (1M context) --- legacy/src/Service/CurlCli.php | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/legacy/src/Service/CurlCli.php b/legacy/src/Service/CurlCli.php index 5552e14e..02c72644 100644 --- a/legacy/src/Service/CurlCli.php +++ b/legacy/src/Service/CurlCli.php @@ -70,11 +70,8 @@ public function run(string $baseUrl, InputInterface $input, OutputInterface $out $shouldRetry = false; $newToken = ''; $stdoutBuffer = ''; - $onOutput = function ($type, $buffer) use ($censor, $output, $stdErr, $process, $retryOn401, &$newToken, &$shouldRetry, &$stdoutBuffer) { - if ($shouldRetry) { - // Ensure there is no output after a retry is triggered. - return; - } + $stderrBuffer = ''; + $onOutput = function ($type, $buffer) use ($censor, $output, $stdErr, $retryOn401, &$stdoutBuffer, &$stderrBuffer) { if ($type === Process::OUT) { if ($retryOn401) { // Buffer stdout when we might need to retry on 401. @@ -85,17 +82,8 @@ public function run(string $baseUrl, InputInterface $input, OutputInterface $out return; } if ($type === Process::ERR) { - if ($retryOn401 && $this->parseCurlStatusCode($buffer) === 401 && $this->api->isLoggedIn()) { - $shouldRetry = true; - $stdoutBuffer = ''; // Discard buffered stdout from the 401 response. - $process->clearErrorOutput(); - $process->clearOutput(); - - $newToken = $this->api->getAccessToken(true); - $stdErr->writeln('The access token has been refreshed. Retrying request.'); - - $process->stop(); - return; + if ($retryOn401) { + $stderrBuffer .= $buffer; } if ($stdErr->isVeryVerbose()) { $stdErr->write($censor($buffer)); @@ -107,11 +95,22 @@ public function run(string $baseUrl, InputInterface $input, OutputInterface $out $process->run($onOutput); + // Check for 401 after the process completes, using the full accumulated + // stderr buffer. This avoids a race condition where the process may exit + // before the callback processes the stderr chunk containing the status line. + if ($retryOn401 && $this->parseCurlStatusCode($stderrBuffer) === 401 && $this->api->isLoggedIn()) { + $shouldRetry = true; + $stdoutBuffer = ''; // Discard buffered stdout from the 401 response. + $newToken = $this->api->getAccessToken(true); + $stdErr->writeln('The access token has been refreshed. Retrying request.'); + } + if ($shouldRetry) { // Create a new curl process, replacing the access token. $commandline = $this->buildCurlCommand($url, $newToken, $input); $process = Process::fromShellCommandline($commandline); - $stdoutBuffer = ''; // Reset the buffer for the retry. + $stdoutBuffer = ''; // Reset the buffers for the retry. + $stderrBuffer = ''; $shouldRetry = false; // Update the $token variable in the $censor closure.