Skip to content

Commit 6bbd41c

Browse files
fix(legacy): resolve flaky TestApiCurlCommand by fixing 401 detection race condition (#21)
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) <noreply@anthropic.com>
1 parent bcd63b3 commit 6bbd41c

1 file changed

Lines changed: 16 additions & 17 deletions

File tree

‎legacy/src/Service/CurlCli.php‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -70,11 +70,8 @@ public function run(string $baseUrl, InputInterface $input, OutputInterface $out
7070
$shouldRetry = false;
7171
$newToken = '';
7272
$stdoutBuffer = '';
73-
$onOutput = function ($type, $buffer) use ($censor, $output, $stdErr, $process, $retryOn401, &$newToken, &$shouldRetry, &$stdoutBuffer) {
74-
if ($shouldRetry) {
75-
// Ensure there is no output after a retry is triggered.
76-
return;
77-
}
73+
$stderrBuffer = '';
74+
$onOutput = function ($type, $buffer) use ($censor, $output, $stdErr, $retryOn401, &$stdoutBuffer, &$stderrBuffer) {
7875
if ($type === Process::OUT) {
7976
if ($retryOn401) {
8077
// Buffer stdout when we might need to retry on 401.
@@ -85,17 +82,8 @@ public function run(string $baseUrl, InputInterface $input, OutputInterface $out
8582
return;
8683
}
8784
if ($type === Process::ERR) {
88-
if ($retryOn401 && $this->parseCurlStatusCode($buffer) === 401 && $this->api->isLoggedIn()) {
89-
$shouldRetry = true;
90-
$stdoutBuffer = ''; // Discard buffered stdout from the 401 response.
91-
$process->clearErrorOutput();
92-
$process->clearOutput();
93-
94-
$newToken = $this->api->getAccessToken(true);
95-
$stdErr->writeln('The access token has been refreshed. Retrying request.');
96-
97-
$process->stop();
98-
return;
85+
if ($retryOn401) {
86+
$stderrBuffer .= $buffer;
9987
}
10088
if ($stdErr->isVeryVerbose()) {
10189
$stdErr->write($censor($buffer));
@@ -107,11 +95,22 @@ public function run(string $baseUrl, InputInterface $input, OutputInterface $out
10795

10896
$process->run($onOutput);
10997

98+
// Check for 401 after the process completes, using the full accumulated
99+
// stderr buffer. This avoids a race condition where the process may exit
100+
// before the callback processes the stderr chunk containing the status line.
101+
if ($retryOn401 && $this->parseCurlStatusCode($stderrBuffer) === 401 && $this->api->isLoggedIn()) {
102+
$shouldRetry = true;
103+
$stdoutBuffer = ''; // Discard buffered stdout from the 401 response.
104+
$newToken = $this->api->getAccessToken(true);
105+
$stdErr->writeln('The access token has been refreshed. Retrying request.');
106+
}
107+
110108
if ($shouldRetry) {
111109
// Create a new curl process, replacing the access token.
112110
$commandline = $this->buildCurlCommand($url, $newToken, $input);
113111
$process = Process::fromShellCommandline($commandline);
114-
$stdoutBuffer = ''; // Reset the buffer for the retry.
112+
$stdoutBuffer = ''; // Reset the buffers for the retry.
113+
$stderrBuffer = '';
115114
$shouldRetry = false;
116115

117116
// Update the $token variable in the $censor closure.

0 commit comments

Comments
 (0)