HttpConnector: do not report connection timeouts as user interrupts

If a conenct() times out (because the server is not responsive), it
throws a SocketTimeoutException which is an InterruptedIOException.
The latter, however, are mapped by the HttpConnector to InterruptedExceptions
indicating a user interruption of the download, which is generally correct, but
not for this specific subclass. Fix this by handling this subclass separately
and rethrowing it as ordinary IOException.

Fixes #3851.
Closes #6209.

Change-Id: I0c3b46921a725f5a5a728a83dd75c7c83c6a46f4
PiperOrigin-RevId: 245398269

src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/HttpConnector.java

@@ -184,6 +184,11 @@ URLConnection connect(
         throw e;
       } catch (IllegalArgumentException e) {
         throw new UnrecoverableHttpException(e.getMessage());
+      } catch (SocketTimeoutException e) {
+        // SocketTimeoutExceptions are InterruptedExceptions; however they do not signify
+        // an external interruption, but simply a failed download due to some server timing
+        // out. So rethrow them as ordinary IOExceptions.
+        throw new IOException(e.getMessage(), e);
       } catch (IOException e) {
         if (connection != null) {
           // If we got here, it means we might not have consumed the entire payload of the

src/test/shell/bazel/external_integration_test.sh

@@ -413,6 +413,27 @@ EOF
   expect_log "Tra-la!"
 }
 
+function test_http_timeout() {
+  serve_timeout
+
+  cd ${WORKSPACE_DIR}
+  cat > WORKSPACE <<EOF
+load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_file")
+http_file(
+    name = 'toto',
+    urls = ['http://127.0.0.1:$nc_port/toto'],
+)
+EOF
+  date
+  bazel build --http_timeout_scaling=0.03 @toto//file > $TEST_log 2>&1 \
+      && fail "Expected failure" || :
+  date
+  kill_nc
+
+  expect_log '[Tt]imed\? \?out'
+  expect_not_log "interrupted"
+}
+
 # Tests downloading a file with a redirect.
 function test_http_redirect() {
   local test_file=$TEST_TMPDIR/toto

src/test/shell/bazel/remote_helpers.sh

@@ -143,6 +143,19 @@ function serve_not_found() {
   cd -
 }
 
+# Simulates a server timeing out while trying to generate a response.
+function serve_timeout() {
+  port_file=server-port.$$
+  cd "${TEST_TMPDIR}"
+  rm -f $port_file
+  python $python_server timeout  > $port_file &
+  nc_pid=$!
+  while ! grep started $port_file; do sleep 1; done
+  nc_port=$(head -n 1 $port_file)
+  fileserver_port=$nc_port
+  cd -
+}
+
 # Waits for the SimpleHTTPServer to actually start up before the test is run.
 # Otherwise the entire test can run before the server starts listening for
 # connections, which causes flakes.

src/test/shell/bazel/testing_server.py

@@ -31,12 +31,14 @@
 import random
 import socket
 import sys
+import time
 
 
 class Handler(BaseHTTPRequestHandler):
   """Handlers for testing HTTP server."""
   auth = False
   not_found = False
+  simulate_timeout = False
   filename = None
   redirect = None
   valid_header = b'Basic ' + base64.b64encode('foo:bar'.encode('ascii'))
@@ -53,6 +55,10 @@ def do_AUTHHEAD(self):  # pylint: disable=invalid-name
     self.end_headers()
 
   def do_GET(self):  # pylint: disable=invalid-name
+    if self.simulate_timeout:
+      while True:
+        time.sleep(1)
+
     if self.not_found:
       self.send_response(404)
       self.end_headers()
@@ -96,6 +102,8 @@ def main(argv=None):
     Handler.redirect = argv[1]
   elif argv and argv[0] == '404':
     Handler.not_found = True
+  elif argv and argv[0] == 'timeout':
+    Handler.simulate_timeout = True
   elif argv:
     Handler.auth = True