tools/android/loading: Lets some execption pass through chrome controller's Open

tools/android/loading/controller.py

 # Copyright 2016 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Controller objects that control the context in which chrome runs.
 
 This is responsible for the setup necessary for launching chrome, and for
 creating a DevToolsConnection. There are remote device and local
 desktop-specific versions.
 """
@@ skipping 64 matching lines @@
 
 class ChromeControllerError(Exception):
   """Chrome error with detailed log.
 
   Note:
     Some of these errors might be known intermittent errors that can usually be
     retried by the caller after re-doing any specific setup again.
   """
   _INTERMITTENT_WHITE_LIST = {websocket.WebSocketTimeoutException,
                               devtools_monitor.DevToolsConnectionTargetCrashed}
+  _PASSTHROUGH_WHITE_LIST = (MemoryError, SyntaxError)

[pasko, 2016/06/01 12:47:17]

wow, interesting, did you actually hit a memory error?
are there other unrecoverable errors we want to stop on?

[gabadie, 2016/06/01 12:54:43]

It is not obvious in this CL but there is exceptions such as SystemExit or
KeyboardInterupt are not in subclasses of Exception but ExceptionBase. But the
except: implictly does a except ExceptionBase, the reason why we change below
from except: to except Exception:.

The issue is that there is some exception such as the one listed here that are
still subclasses of Exception, the reason we are doing this
_PASSTHROUGH_WHITE_LIST.

[pasko, 2016/06/01 13:42:22]

thanks, it is indeed not obvious

 
   def __init__(self, log):
     """Constructor
 
     Args:
       log: String containing the log of the running Chrome instance that was
           running. It will be interleaved with any other running Android
           package.
     """
     self.error_type, self.error_value, self.error_traceback = sys.exc_info()
@@ skipping 252 matching lines @@
               continue
             yield connection
             if self._slow_death:
               self._device.adb.Shell('am start com.google.android.launcher')
               time.sleep(self.TIME_TO_IDLE_SECONDS)
             break
         else:
           raise ChromeControllerInternalError(
               'Failed to connect to Chrome devtools after {} '
               'attempts.'.format(self.DEVTOOLS_CONNECTION_ATTEMPTS))
-      except:
+      except ChromeControllerError._PASSTHROUGH_WHITE_LIST:
+        raise
+      except Exception:
         logcat = ''.join([l + '\n' for l in self._device.adb.Logcat(dump=True)])
         raise ChromeControllerError(log=logcat)
       finally:
         self._device.ForceStop(package_info.package)
 
   def ResetBrowserState(self):
     """Override for chrome state reseting."""
     logging.info('Reset chrome\'s profile')
     package_info = OPTIONS.ChromePackage()
     # We assume all the browser is in the Default user profile directory.
@@ skipping 145 matching lines @@
         raise ChromeControllerInternalError(
             'Failed to connect to Chrome devtools after {} '
             'attempts.'.format(self.DEVTOOLS_CONNECTION_ATTEMPTS))
       # Start and yield the devtool connection.
       self._StartConnection(connection)
       yield connection
       if self._slow_death:
         connection.Close()
         chrome_process.wait()
         chrome_process = None
-    except:
+    except ChromeControllerError._PASSTHROUGH_WHITE_LIST:
+      raise
+    except Exception:
       raise ChromeControllerError(log=open(tmp_log.name).read())
     finally:
       if OPTIONS.local_noisy:
         sys.stderr.write(open(tmp_log.name).read())
       del tmp_log
       if chrome_process:
         try:
           chrome_process.kill()
         except OSError:
           pass  # Chrome is already dead.
@@ skipping 47 matching lines @@
     if (not os.path.isdir(self._profile_dir) or
         os.listdir(self._profile_dir) == []):
       # Launch chrome so that it populates the profile directory.
       with self.Open():
         pass
     assert os.path.isdir(self._profile_dir)
     assert os.path.isdir(os.path.dirname(self._GetCacheDirectoryPath()))
 
   def _GetCacheDirectoryPath(self):
     return os.path.join(self._profile_dir, 'Default', 'Cache')