Clear and restore problematic NODE environment variables before calling orca (#1378)

* Add orca_env context manage to clear and restore environment variables
that interfere with the correct functioning of orca.

* Increase the orca request retry duration from 8s to 30s. 8 seconds was arbitrary, and we've received reports that it is sometimes not long enough. So increasing to 30s.

Author: jonmmease

plotly/io/_orca.py

@@ -9,6 +9,7 @@
 import threading
 import warnings
 from copy import copy
+from contextlib import contextmanager
 
 import requests
 import retrying
@@ -836,6 +837,36 @@ def __repr__(self):
 del OrcaStatus
 
 
+@contextmanager
+def orca_env():
+    """
+    Context manager to clear and restore environment variables that are
+    problematic for orca to function properly
+
+    NODE_OPTIONS: When this variable is set, orca <v1.2 will have a
+    segmentation fault due to an electron bug.
+    See: https://github.com/electron/electron/issues/12695
+
+    ELECTRON_RUN_AS_NODE: When this environment variable is set the call
+    to orca is transformed into a call to nodejs.
+    See https://github.com/plotly/orca/issues/149#issuecomment-443506732
+    """
+    clear_env_vars = ['NODE_OPTIONS', 'ELECTRON_RUN_AS_NODE']
+    orig_env_vars = {}
+
+    try:
+        # Clear and save
+        orig_env_vars.update({
+            var: os.environ.pop(var)
+            for var in clear_env_vars
+            if var in os.environ})
+        yield
+    finally:
+        # Restore
+        for var, val in orig_env_vars.items():
+            os.environ[var] = val
+
+
 # Public orca server interaction functions
 # ----------------------------------------
 def validate_executable():
@@ -918,13 +949,6 @@ def validate_executable():
             formatted_path=formatted_path,
             instructions=install_location_instructions))
 
-    # Clear NODE_OPTIONS environment variable
-    # ---------------------------------------
-    # When this variable is set, orca <v1.2 will have a segmentation fault
-    # due to an electron bug.
-    # See: https://github.com/electron/electron/issues/12695
-    os.environ.pop('NODE_OPTIONS', None)
-
     # Run executable with --help and see if it's our orca
     # ---------------------------------------------------
     invalid_executable_msg = """
@@ -938,12 +962,13 @@ def validate_executable():
         instructions=install_location_instructions)
 
     # ### Run with Popen so we get access to stdout and stderr
-    p = subprocess.Popen(
-        [executable, '--help'],
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE)
+    with orca_env():
+        p = subprocess.Popen(
+            [executable, '--help'],
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE)
 
-    help_result, help_error = p.communicate()
+        help_result, help_error = p.communicate()
 
     if p.returncode != 0:
         err_msg = invalid_executable_msg + """
@@ -986,12 +1011,13 @@ def validate_executable():
     # Get orca version
     # ----------------
     # ### Run with Popen so we get access to stdout and stderr
-    p = subprocess.Popen(
-        [executable, '--version'],
-        stdout=subprocess.PIPE,
-        stderr=subprocess.PIPE)
+    with orca_env():
+        p = subprocess.Popen(
+            [executable, '--version'],
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE)
 
-    version_result, version_error = p.communicate()
+        version_result, version_error = p.communicate()
 
     if p.returncode != 0:
         raise ValueError(invalid_executable_msg + """
@@ -1171,8 +1197,9 @@ def ensure_server():
             # Create subprocess that launches the orca server on the
             # specified port.
             DEVNULL = open(os.devnull, 'wb')
-            orca_state['proc'] = subprocess.Popen(cmd_list,
-                                                  stdout=DEVNULL)
+            with orca_env():
+                orca_state['proc'] = subprocess.Popen(cmd_list,
+                                                      stdout=DEVNULL)
 
             # Update orca.status so the user has an accurate view
             # of the state of the orca server
@@ -1191,7 +1218,7 @@ def ensure_server():
             orca_state['shutdown_timer'] = t
 
 
-@retrying.retry(wait_random_min=5, wait_random_max=10, stop_max_delay=8000)
+@retrying.retry(wait_random_min=5, wait_random_max=10, stop_max_delay=30000)
 def request_image_with_retrying(**kwargs):
     """
     Helper method to perform an image request to a running orca server process

plotly/tests/test_orca/test_orca_server.py

@@ -13,6 +13,10 @@
 # --------
 @pytest.fixture()
 def setup():
+    # Set problematic environment variables
+    os.environ['NODE_OPTIONS'] = '--max-old-space-size=4096'
+    os.environ['ELECTRON_RUN_AS_NODE'] = '1'
+
     # Reset orca state
     pio.orca.reset_status()
     pio.orca.config.restore_defaults()

plotly/tests/test_orca/test_to_image.py

@@ -283,3 +283,40 @@ def test_topojson_fig_to_image(topofig, format):
 def test_latex_fig_to_image(latexfig, format):
     img_bytes = pio.to_image(latexfig, format=format, width=700, height=500)
     assert_image_bytes(img_bytes, 'latexfig.' + format)
+
+
+# Environmnet variables
+# ---------------------
+def test_problematic_environment_variables(fig1, format):
+    pio.orca.config.restore_defaults(reset_server=True)
+
+    os.environ['NODE_OPTIONS'] = '--max-old-space-size=4096'
+    os.environ['ELECTRON_RUN_AS_NODE'] = '1'
+
+    # Do image export
+    img_bytes = pio.to_image(fig1, format=format, width=700, height=500)
+    assert_image_bytes(img_bytes, 'fig1.' + format)
+
+    # Check that environment variables were restored
+    assert os.environ['NODE_OPTIONS'] == '--max-old-space-size=4096'
+    assert os.environ['ELECTRON_RUN_AS_NODE'] == '1'
+
+
+# Invalid figure json
+# -------------------
+def test_invalid_figure_json():
+    # Do image export
+    bad_fig = {'foo': 'bar'}
+    with pytest.raises(ValueError) as err:
+        pio.to_image(bad_fig, format='png')
+
+    assert "Invalid value of type" in str(err.value)
+
+    with pytest.raises(ValueError) as err:
+        pio.to_image(bad_fig, format='png', validate=False)
+
+    assert ('The image request was rejected by the orca conversion utility'
+            in str(err.value))
+
+    assert ('400: invalid or malformed request syntax'
+            in str(err.value))