telemetry: Report error correctly on connection timeout

When we hit the 30s timeout, we would continue to run the test, then report a success, then attempt to dump the stdio and stacktrace.

Instead re-raise the exception and remove the finally block to stop the test from going through its paces

BUG=424024

Review URL: https://codereview.chromium.org/694893004

Cr-Commit-Position: refs/heads/master@{#302577}

tools/telemetry/telemetry/page/page_runner.py

@@ -290,12 +290,13 @@ def _RunPageAndRetryRunIfNeeded(test, page_set, expectations, finder_options,
     except exceptions.BrowserGoneException as e:
       state.StopBrowser()
       if attempt_num == max_attempts:
-        logging.error('Aborting after too many retries')
-        raise
-      if test.is_multi_tab_test:
+        results.AddValue(failure.FailureValue.FromMessage(
+            page, 'Failed to connect to browser after too many retries.'))
+      elif test.is_multi_tab_test:
         logging.error('Aborting multi-tab test after browser crashed')
         raise
-      logging.warning(str(e))
+      else:
+        logging.warning(str(e))
 
 
 @decorators.Cache

tools/telemetry/telemetry/page/page_runner_unittest.py

@@ -155,6 +155,67 @@ class PageRunnerTests(unittest.TestCase):
     self.assertEquals(1, len(GetSuccessfulPageRuns(results)))
     self.assertEquals(1, len(results.failures))
 
+  def testRaiseBrowserGoneExceptionFromValidatePage(self):
+    self.SuppressExceptionFormatting()
+    ps = page_set.PageSet()
+    expectations = test_expectations.TestExpectations()
+    ps.pages.append(page_module.Page(
+        'file://blank.html', ps, base_dir=util.GetUnittestDataDir()))
+    ps.pages.append(page_module.Page(
+        'file://blank.html', ps, base_dir=util.GetUnittestDataDir()))
+
+    class Test(page_test.PageTest):
+      def __init__(self, *args):
+        super(Test, self).__init__(*args)
+        self.run_count = 0
+      def ValidatePage(self, *_):
+        old_run_count = self.run_count
+        self.run_count += 1
+        if old_run_count == 0:
+          raise exceptions.BrowserGoneException()
+
+    options = options_for_unittests.GetCopy()
+    options.output_formats = ['none']
+    options.suppress_gtest_report = True
+    test = Test()
+    SetUpPageRunnerArguments(options)
+    results = results_options.CreateResults(EmptyMetadataForTest(), options)
+    page_runner.Run(test, ps, expectations, options, results)
+    self.assertEquals(2, test.run_count)
+    self.assertEquals(1, len(GetSuccessfulPageRuns(results)))
+    self.assertEquals(1, len(results.failures))
+
+  def testRaiseBrowserGoneExceptionFromRestartBrowserBeforeEachPage(self):
+    self.SuppressExceptionFormatting()
+    ps = page_set.PageSet()
+    expectations = test_expectations.TestExpectations()
+    ps.pages.append(page_module.Page(
+        'file://blank.html', ps, base_dir=util.GetUnittestDataDir()))
+    ps.pages.append(page_module.Page(
+        'file://blank.html', ps, base_dir=util.GetUnittestDataDir()))
+
+    class Test(page_test.PageTest):
+      def __init__(self, *args):
+        super(Test, self).__init__(*args)
+        self.run_count = 0
+      def RestartBrowserBeforeEachPage(self):
+        old_run_count = self.run_count
+        self.run_count += 1
+        if old_run_count == 0:
+          raise exceptions.BrowserGoneException(None)
+        return self._needs_browser_restart_after_each_page
+
+    options = options_for_unittests.GetCopy()
+    options.output_formats = ['none']
+    options.suppress_gtest_report = True
+    test = Test()
+    SetUpPageRunnerArguments(options)
+    results = results_options.CreateResults(EmptyMetadataForTest(), options)
+    page_runner.Run(test, ps, expectations, options, results)
+    self.assertEquals(2, test.run_count)
+    self.assertEquals(1, len(GetSuccessfulPageRuns(results)))
+    self.assertEquals(1, len(results.failures))
+
   def testHandlingOfCrashedTabWithExpectedFailure(self):
     self.SuppressExceptionFormatting()
     ps = page_set.PageSet()