Merge pull request #95 from plotly/andrew/mac-subprocess-fix

Andrew/mac subprocess fix

devtools/browser.py

@@ -83,7 +83,7 @@ def __init__(
         if not path:
             path = default_path
         if path:
-            new_env["BROWSER_PATH"] = path
+            new_env["BROWSER_PATH"] = str(path)
         else:
             raise RuntimeError(
                 "Could not find an acceptable browser. Please set environmental variable BROWSER_PATH or pass `path=/path/to/browser` into the Browser() constructor."
@@ -205,6 +205,7 @@ async def _open_async(self):
         self.future_self.set_result(self)
 
     def _clean_temp(self):
+        name = self.temp_dir.name
         clean = False
         try:
             self.temp_dir.cleanup()
@@ -242,7 +243,7 @@ def remove_readonly(func, path, excinfo):
                         f"The temporary directory could not be deleted, execution will continue. {type(e)}: {e}", TempDirWarning
                 )
         if self.debug:
-            print(f"Tempfile still exists?: {bool(os.path.isfile(str(self.temp_dir.name)))}")
+            print(f"Tempfile still exists?: {bool(os.path.exists(str(name)))}")
 
     async def _is_closed_async(self, wait=0):
         waiter = self.subprocess.wait()
@@ -575,27 +576,55 @@ def write_json(self, obj):
             return key
 
 def diagnose():
-    print("*****************************************")
-    print("Please copy and paste all these results to an issue or to slack!")
-    print("Collecting information about the system:")
+    fail = []
+    print("*".center(50, "*"))
+    print("Collecting information about the system:".center(50, "*"))
     print(platform.system())
     print(platform.release())
     print(platform.version())
     print(platform.uname())
-    print("Looking for browser:")
-    print(which_browser())
-    print("Running a very simple test...")
+    print("Looking for browser:".center(50, "*"))
+    print(which_browser(debug=True))
     try:
+        print("Looking for version info:".center(50, "*"))
         import subprocess, sys # noqa
         print(subprocess.check_output([sys.executable, '-m', 'pip', 'freeze']))
         print(subprocess.check_output(["git", "describe", "--all", "--tags", "--long", "--always",]))
         print(sys.version)
         print(sys.version_info)
+    except BaseException as e:
+        fail.append(e)
     finally:
+        print("Done with version info.".center(50, "*"))
         pass
+    try:
+        print("Sync test".center(50, "*"))
+        import time
+        browser = Browser(debug=True, debug_browser=True)
+        time.sleep(2)
+        browser.close()
+    except BaseException as e:
+        fail.append(e)
+    finally:
+        print("Done with sync test".center(50, "*"))
     async def test():
         browser = await Browser(debug=True, debug_browser=True)
         await asyncio.sleep(2)
         await browser.close()
-    asyncio.run(test())
+    try:
+        print("Running Asyncio Test".center(50, "*"))
+        asyncio.run(test())
+    except BaseException as e:
+        fail.append(e)
+    finally:
+        print("Asyncio.run done".center(50, "*"))
+    print("")
+    sys.stdout.flush()
+    sys.stderr.flush()
+    if fail:
+        import traceback
+        for exception in fail:
+            if exception:
+                traceback.print_exception(exception)
+        raise BaseException("There was an exception, see above.")
     print("Thank you! Please share these results with us!")

devtools/pipe.py

@@ -66,6 +66,7 @@ def read_jsons(self, blocking=True, debug=None):
         except OSError as e:
             raise PipeClosedError() from e
         try:
+            raw_buffer = None # if we fail in read, we already defined
             raw_buffer = os.read(
                 self.read_from_chromium, 10000
             )  # 10MB buffer, nbd, doesn't matter w/ this
@@ -84,15 +85,21 @@ def read_jsons(self, blocking=True, debug=None):
         except OSError as e:
             if debug:
                 print(f"caught OSError in read() {str(e)}", file=sys.stderr)
-            if not raw_buffer:
+            if not raw_buffer or raw_buffer == b'{bye}\n':
                 raise PipeClosedError()
             # TODO this could be hard to test as it is a real OS corner case
             # but possibly raw_buffer is partial
             # and we don't check for partials
         decoded_buffer = raw_buffer.decode("utf-8")
+        if debug:
+            print(decoded_buffer, file=sys.stderr)
         for raw_message in decoded_buffer.split("\0"):
             if raw_message:
-                jsons.append(json.loads(raw_message))
+                try:
+                    jsons.append(json.loads(raw_message))
+                except BaseException as e:
+                    if debug:
+                        print(f"Problem with {raw_message} in json: {e}", file=sys.stderr)
                 if debug:
                     # This debug is kinda late but the jsons package helps with decoding, since JSON optionally
                     # allows escaping unicode characters, which chrome does (oof)

devtools/system.py

@@ -1,8 +1,9 @@
 import shutil
 import platform
 import os
+import sys
 
-chrome = ["chrome", "Chrome", "google-chrome", "google-chrome-stable", "chromium", "chromium-browser"]
+chrome = ["chrome", "Chrome", "google-chrome", "google-chrome-stable", "Chrome.app", "Google Chrome", "Google Chrome.app", "chromium", "chromium-browser"]
 chromium = ["chromium", "chromium-browser"]
 # firefox = // this needs to be tested
 # brave = // this needs to be tested
@@ -24,7 +25,7 @@
             ]
 else: # assume mac, or system == "Darwin"
     default_path_chrome = [
-            "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome"
+            "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome",
             ]
 
 def which_windows_chrome():
@@ -53,24 +54,28 @@ def _is_exe(path):
     finally:
         return res
 
-def which_browser(executable_name=chrome):
+def which_browser(executable_name=chrome, debug=False):
     path = None
     if isinstance(executable_name, str):
         executable_name = [executable_name]
     if platform.system() == "Windows":
         os.environ["NoDefaultCurrentDirectoryInExePath"] = "0"
     for exe in executable_name:
+        if debug: print(f"looking for {exe}", file=sys.stderr)
         if platform.system() == "Windows" and exe == "chrome":
             path = which_windows_chrome()
             if path and _is_exe(path):
                 return path
         path = shutil.which(exe)
+        if debug: print(f"looking for {path}", file=sys.stderr)
         if path and _is_exe(path):
             return path
     default_path = []
     if executable_name == chrome:
         default_path = default_path_chrome
     for candidate in default_path:
+        if debug: print(f"Looking at {candidate}", file=sys.stderr)
         if _is_exe(candidate):
-            return default_path
+            return candidate
+    if debug: print("Found nothing...", file=sys.stderr)
     return None