Avoid showing the interim error output

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)
 }

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,34 +73,51 @@ 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);
             }
         };
 
         $stdErr->writeln(sprintf('Running command: <info>%s</info>', $censor($commandline)), OutputInterface::VERBOSITY_VERBOSE);
 
         $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;
     }