presubmit: Make ThreadPool surface exceptions on CallCommand.

Exceptions other than OSError are not surfaced.
This caused errors like this to be printed, but not block presubmit,
allowing bugs to sneak in.

Exception in thread Thread-8:
Traceback (most recent call last):
  File "C:\b\s\w\ir\cipd_bin_packages\cpython\bin\Lib\threading.py", line 801, in __bootstrap_inner
    self.run()
  File "C:\b\s\w\ir\cipd_bin_packages\cpython\bin\Lib\threading.py", line 754, in run
    self.__target(*self.__args, **self.__kwargs)
  File "C:\b\s\w\ir\kitchen-checkout\depot_tools\presubmit_support.py", line 199, in _WorkerFn
    result = self.CallCommand(test)
  File "C:\b\s\w\ir\kitchen-checkout\depot_tools\presubmit_support.py", line 170, in CallCommand
    p = subprocess.Popen(cmd, **test.kwargs)
  File "C:\b\s\w\ir\kitchen-checkout\depot_tools\subprocess2.py", line 143, in __init__
    super(Popen, self).__init__(args, **kwargs)
  File "C:\b\s\w\ir\cipd_bin_packages\cpython\bin\Lib\subprocess.py", line 390, in __init__
    errread, errwrite)
  File "C:\b\s\w\ir\cipd_bin_packages\cpython\bin\Lib\subprocess.py", line 640, in _execute_child
    startupinfo)
TypeError: environment can only contain strings

https://logs.chromium.org/logs/infra/buildbucket/cr-buildbucket.appspot.com/8898840708364523888/+/steps/presubmit/0/stdout

Change-Id: I34e65d8c0050eed7ed26fd782e0a5dc8616f30f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1877051
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
Reviewed-by: Anthony Polito <apolito@google.com>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>

presubmit_support.py

@@ -34,6 +34,7 @@ import tempfile  # Exposed through the API.
 import threading
 import time
 import types
+import traceback
 import unittest  # Exposed through the API.
 from warnings import warn
 
@@ -174,6 +175,12 @@ class ThreadPool(object):
       duration = time.time() - start
       return test.message(
           '%s exec failure (%4.2fs)\n   %s' % (test.name, duration, e))
+    except Exception as e:
+      duration = time.time() - start
+      return test.message(
+          '%s exec failure (%4.2fs)\n%s' % (
+              test.name, duration, traceback.format_exc()))
+
     if p.returncode != 0:
       return test.message(
           '%s (%4.2fs) failed\n%s' % (test.name, duration, stdout))

tests/presubmit_unittest.py

@@ -2892,6 +2892,47 @@ the current line as well!
     self.assertIsNone(commands[0].info)
 
 
+class ThreadPoolTest(unittest.TestCase):
+  def setUp(self):
+    super(ThreadPoolTest, self).setUp()
+    mock.patch('subprocess2.Popen').start()
+    mock.patch('presubmit_support.sigint_handler').start()
+    presubmit.sigint_handler.wait.return_value = ('stdout', '')
+    self.addCleanup(mock.patch.stopall)
+
+  def testSurfaceExceptions(self):
+    def FakePopen(cmd, **kwargs):
+      if cmd[0] == '3':
+        raise TypeError('TypeError')
+      if cmd[0] == '4':
+        raise OSError('OSError')
+      if cmd[0] == '5':
+        return mock.Mock(returncode=1)
+      return mock.Mock(returncode=0)
+    subprocess.Popen.side_effect = FakePopen
+
+    mock_tests = [
+        presubmit.CommandData(
+            name=str(i),
+            cmd=[str(i)],
+            kwargs={},
+            message=lambda x: x,
+        )
+        for i in range(10)
+    ]
+
+    t = presubmit.ThreadPool(1)
+    t.AddTests(mock_tests)
+    messages = sorted(t.RunAsync())
+
+    self.assertEqual(3, len(messages))
+    self.assertIn(
+      '3 exec failure (0.00s)\nTraceback (most recent call last):',
+      messages[0])
+    self.assertEqual('4 exec failure (0.00s)\n   OSError', messages[1])
+    self.assertEqual('5 (0.00s) failed\nstdout', messages[2])
+
+
 if __name__ == '__main__':
   import unittest
   unittest.main()