From ad018acee30c1201d5b957176f02c3cfa779ac14 Mon Sep 17 00:00:00 2001 From: Patrick Dawkins Date: Sun, 15 Dec 2024 16:11:29 +0000 Subject: [PATCH] Avoid showing the interim error output --- go-tests/api_curl_test.go | 13 +++++++++++ src/Service/CurlCli.php | 48 +++++++++++++++++++++++++++++---------- 2 files changed, 49 insertions(+), 12 deletions(-) diff --git a/go-tests/api_curl_test.go b/go-tests/api_curl_test.go index 30aa27a16..94375b831 100644 --- a/go-tests/api_curl_test.go +++ b/go-tests/api_curl_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "os/exec" "strings" "testing" @@ -24,6 +25,7 @@ func TestApiCurlCommand(t *testing.T) { if !strings.HasPrefix(r.URL.Path, "/oauth2") { if r.Header.Get("Authorization") != "Bearer "+validToken { w.WriteHeader(http.StatusUnauthorized) + _ = json.NewEncoder(w).Encode(map[string]any{"error": "invalid_token", "error_description": "Invalid access token."}) return } } @@ -60,4 +62,15 @@ func TestApiCurlCommand(t *testing.T) { assert.Equal(t, "success", run("api:curl", "/fake-api-path")) assert.Equal(t, 2, tokenFetches) + + // If --no-retry-401 and --fail are provided then the command should return exit code 22. + validToken = "another-new-valid-token" + cmd := authenticatedCommand(t, mockServer.URL, mockServer.URL, + "api:curl", "/fake-api-path", "--no-retry-401", "--fail") + t.Log("Running:", cmd) + _, err := cmd.Output() + exitErr := &exec.ExitError{} + assert.ErrorAs(t, err, &exitErr) + assert.Equal(t, 22, exitErr.ExitCode()) + assert.Equal(t, 2, tokenFetches) } diff --git a/src/Service/CurlCli.php b/src/Service/CurlCli.php index f0d83a1d0..63f1104c2 100644 --- a/src/Service/CurlCli.php +++ b/src/Service/CurlCli.php @@ -29,7 +29,8 @@ public static function configureInput(InputDefinition $definition) $definition->addOption(new InputOption('head', 'I', InputOption::VALUE_NONE, 'Fetch headers only')); $definition->addOption(new InputOption('disable-compression', null, InputOption::VALUE_NONE, 'Do not use the curl --compressed flag')); $definition->addOption(new InputOption('enable-glob', null, InputOption::VALUE_NONE, 'Enable curl globbing (remove the --globoff flag)')); - $definition->addOption(new InputOption('fail', 'f', InputOption::VALUE_NONE, 'Fail with no output on an error response')); + $definition->addOption(new InputOption('no-retry-401', null, InputOption::VALUE_NONE, 'Disable automatic retry on 401 errors')); + $definition->addOption(new InputOption('fail', 'f', InputOption::VALUE_NONE, 'Fail with no output on an error response. Default, unless --no-retry-401 is added.')); $definition->addOption(new InputOption('header', 'H', InputOption::VALUE_REQUIRED | InputOption::VALUE_IS_ARRAY, 'Extra header(s)')); } @@ -55,9 +56,15 @@ public function run($baseUrl, InputInterface $input, OutputInterface $output) { $url .= '/' . ltrim($path, '/'); } - $token = $this->api->getAccessToken(); + $retryOn401 = !$input->getOption('no-retry-401'); + if ($retryOn401) { + // Force --fail if retrying on 401 errors. + // This ensures that the error's output will not be printed, which + // is difficult to prevent otherwise. + $input->setOption('fail', true); + } - $showCurlVerboseOutput = $output->isVeryVerbose(); + $token = $this->api->getAccessToken(); // Censor the access token: this can be applied to verbose output. $censor = function ($str) use (&$token) { @@ -66,23 +73,39 @@ public function run($baseUrl, InputInterface $input, OutputInterface $output) { $commandline = $this->buildCurlCommand($url, $token, $input); + // Add --verbose if -vv is provided, or if retrying on 401 errors. + // In the latter case the verbose output will be intercepted and hidden. + if ($stdErr->isVeryVerbose() || $retryOn401) { + $commandline .= ' --verbose'; + } + $process = new Process($commandline); + $shouldRetry = false; $newToken = ''; - $onOutput = function ($type, $buffer) use ($censor, $output, $stdErr, $showCurlVerboseOutput, $process, &$newToken) { + $onOutput = function ($type, $buffer) use ($censor, $output, $stdErr, $process, $retryOn401, &$newToken, &$shouldRetry) { + if ($shouldRetry) { + // Ensure there is no output after a retry is triggered. + return; + } + if ($type === Process::OUT) { + $output->write($buffer); + return; + } if ($type === Process::ERR) { - if ($this->parseCurlStatusCode($buffer) === 401 && $newToken === '' && $this->api->isLoggedIn()) { - $newToken = $this->api->getAccessToken(true); - $stdErr->writeln('The access token has been refreshed. Retrying request.'); + if ($retryOn401 && $this->parseCurlStatusCode($buffer) === 401 && $this->api->isLoggedIn()) { + $shouldRetry = true; $process->clearErrorOutput(); $process->clearOutput(); + + $newToken = $this->api->getAccessToken(true); + $stdErr->writeln('The access token has been refreshed. Retrying request.'); + $process->stop(); return; } - if ($showCurlVerboseOutput) { + if ($stdErr->isVeryVerbose()) { $stdErr->write($censor($buffer)); } - } else { - $output->write($buffer); } }; @@ -90,10 +113,11 @@ public function run($baseUrl, InputInterface $input, OutputInterface $output) { $process->run($onOutput); - if ($newToken !== '') { + if ($shouldRetry) { // Create a new curl process, replacing the access token. $commandline = $this->buildCurlCommand($url, $newToken, $input); $process = new Process($commandline); + $shouldRetry = false; // Update the $token variable in the $censor closure. $token = $newToken; @@ -158,7 +182,7 @@ private function buildCurlCommand($url, $token, InputInterface $input) $commandline .= ' --header ' . escapeshellarg($header); } - $commandline .= ' --no-progress-meter --verbose'; + $commandline .= ' --no-progress-meter'; return $commandline; }