From e5de0890b0063cb743af18ae0d6945f10442ffcb Mon Sep 17 00:00:00 2001
From: Hood Chatham <hood@mit.edu>
Date: Sun, 18 Apr 2021 10:28:21 -0400
Subject: [PATCH] Improve console error handling, add console.html tests
 (#1480)

---
 .circleci/config.yml              | 13 +++--
 Makefile                          |  5 ++
 conftest.py                       | 13 +++--
 src/pyodide-py/pyodide/console.py | 21 ++++++--
 src/pyodide.js                    |  3 ++
 src/templates/console.html        | 66 ++++++++++++++++++++------
 src/tests/test_console.py         | 79 ++++++++++++++++++++++++++++++-
 7 files changed, 168 insertions(+), 32 deletions(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index f5a183a87..8b8f99ebe 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -7,7 +7,6 @@ defaults: &defaults
   environment:
     - EMSDK_NUM_CORES: 4
       EMCC_CORES: 4
-      PYODIDE_BASE_URL: https://cdn.jsdelivr.net/pyodide/dev/full/
 
 jobs:
   lint:
@@ -95,9 +94,6 @@ jobs:
           paths:
             - .
 
-      - run:
-          name: Prepare CircleCI artifacts
-          command: PYODIDE_BASE_URL="./" make build/console.html
 
       - store_artifacts:
           path: /root/repo/build/
@@ -136,9 +132,6 @@ jobs:
           paths:
             - ./packages/.artifacts
             - ./build
-      - run:
-          name: Prepare CircleCI artifacts
-          command: PYODIDE_BASE_URL="./" make build/console.html
 
       - store_artifacts:
           path: /root/repo/build/
@@ -239,6 +232,9 @@ jobs:
             tar cjf pyodide-build-${CIRCLE_TAG}.tar.bz2  pyodide/
             ghr -t "${GITHUB_TOKEN}" -u "${CIRCLE_PROJECT_USERNAME}" -r "${CIRCLE_PROJECT_REPONAME}" -c "${CIRCLE_SHA1}" -delete  "${CIRCLE_TAG}" ./pyodide-build-${CIRCLE_TAG}.tar.bz2
 
+      - run:
+          name: Set PYODIDE_BASE_URL
+          command: PYODIDE_BASE_URL="https://cdn.jsdelivr.net/pyodide/dev/full/" make update_base_url
       - run:
           name: Deploy to pyodide-cdn2.iodide.io
           command: |
@@ -258,6 +254,9 @@ jobs:
       - checkout
       - attach_workspace:
           at: .
+      - run:
+          name: Set PYODIDE_BASE_URL
+          command: PYODIDE_BASE_URL="https://cdn.jsdelivr.net/pyodide/dev/full/" make update_base_url
       - run:
           name: Install requirements
           command: |
diff --git a/Makefile b/Makefile
index 559ea109f..986aac01f 100644
--- a/Makefile
+++ b/Makefile
@@ -95,6 +95,11 @@ build/webworker_dev.js: src/webworker.js
 	cp $< $@
 	sed -i -e 's#{{ PYODIDE_BASE_URL }}#./#g' $@
 
+update_base_url: \
+	build/console.html \
+	build/pyodide.js \
+	build/webworker.js \
+	docs/_build/html/console.html
 
 test: all
 	pytest src emsdk/tests packages/*/test* pyodide_build -v
diff --git a/conftest.py b/conftest.py
index 16c2c7583..b4947e54b 100644
--- a/conftest.py
+++ b/conftest.py
@@ -109,6 +109,14 @@ class SeleniumWrapper:
                 f"{(build_dir / 'test.html').resolve()} " f"does not exist!"
             )
         self.driver.get(f"http://{server_hostname}:{server_port}/test.html")
+        self.javascript_setup()
+        if load_pyodide:
+            self.run_js("await loadPyodide({ indexURL : './'});")
+            self.save_state()
+        self.script_timeout = script_timeout
+        self.driver.set_script_timeout(script_timeout)
+
+    def javascript_setup(self):
         self.run_js("Error.stackTraceLimit = Infinity;", pyodide_checks=False)
         self.run_js(
             """
@@ -155,11 +163,6 @@ class SeleniumWrapper:
             """,
             pyodide_checks=False,
         )
-        if load_pyodide:
-            self.run_js("await loadPyodide({ indexURL : './'});")
-            self.save_state()
-        self.script_timeout = script_timeout
-        self.driver.set_script_timeout(script_timeout)
 
     @property
     def logs(self):
diff --git a/src/pyodide-py/pyodide/console.py b/src/pyodide-py/pyodide/console.py
index 37ad9ca69..6ef85f78e 100644
--- a/src/pyodide-py/pyodide/console.py
+++ b/src/pyodide-py/pyodide/console.py
@@ -1,3 +1,4 @@
+import traceback
 from typing import Optional, Callable, Any, List, Tuple
 import code
 import io
@@ -221,6 +222,17 @@ class InteractiveConsole(code.InteractiveConsole):
             self.load_packages_and_run(self.run_complete, source)
         )
 
+    def num_frames_to_keep(self, tb):
+        keep_frames = False
+        kept_frames = 0
+        # Try to trim out stack frames inside our code
+        for (frame, _) in traceback.walk_tb(tb):
+            keep_frames = keep_frames or frame.f_code.co_filename == "<console>"
+            keep_frames = keep_frames or frame.f_code.co_filename == "<exec>"
+            if keep_frames:
+                kept_frames += 1
+        return kept_frames
+
     async def load_packages_and_run(self, run_complete, source):
         try:
             await run_complete
@@ -230,11 +242,12 @@ class InteractiveConsole(code.InteractiveConsole):
         with self.stdstreams_redirections():
             await _load_packages_from_imports(source)
             try:
-                result = await eval_code_async(source, self.locals)
+                result = await eval_code_async(
+                    source, self.locals, filename="<console>"
+                )
             except BaseException as e:
-                from traceback import print_exception
-
-                print_exception(type(e), e, e.__traceback__)
+                nframes = self.num_frames_to_keep(e.__traceback__)
+                traceback.print_exception(type(e), e, e.__traceback__, -nframes)
                 raise e
             else:
                 self.display(result)
diff --git a/src/pyodide.js b/src/pyodide.js
index bdb6ca84e..c7713e0cf 100644
--- a/src/pyodide.js
+++ b/src/pyodide.js
@@ -448,6 +448,9 @@ globalThis.loadPyodide = async function(config = {}) {
           }
         });
       }
+      if (Module.on_fatal) {
+        Module.on_fatal(e);
+      }
     } catch (e) {
       console.error("Another error occurred while handling the fatal error:");
       console.error(e);
diff --git a/src/templates/console.html b/src/templates/console.html
index 500072953..4f511ca70 100644
--- a/src/templates/console.html
+++ b/src/templates/console.html
@@ -3,9 +3,8 @@
   <head>
     <meta charset="UTF-8">
     <script src="https://cdn.jsdelivr.net/npm/jquery"></script>
-    <script src="https://cdn.jsdelivr.net/npm/jquery.terminal/js/jquery.terminal.min.js"></script>
-    <script src="https://cdn.jsdelivr.net/npm/jquery.terminal/js/echo_newline.js"></script>
-    <link href="https://cdn.jsdelivr.net/npm/jquery.terminal/css/jquery.terminal.min.css" rel="stylesheet"/>
+    <script src="https://cdn.jsdelivr.net/npm/jquery.terminal@2.23.0/js/jquery.terminal.min.js"></script>
+    <link href="https://cdn.jsdelivr.net/npm/jquery.terminal@2.23.0/css/jquery.terminal.min.css" rel="stylesheet"/>
     <script src="{{ PYODIDE_BASE_URL }}pyodide.js"></script>
     <style>
       .terminal { --size: 1.5; }
@@ -13,6 +12,9 @@
   </head>
   <body>
     <script>
+      function sleep(s){
+        return new Promise(resolve => setTimeout(resolve, s));
+      }
       async function main() {
         await loadPyodide({ indexURL : '{{ PYODIDE_BASE_URL }}' });
         let namespace = pyodide.globals.get("dict")();
@@ -33,26 +35,47 @@
                     return f"Welcome to the Pyodide terminal emulator 🐍\\n{super().banner()}"
 
 
-            js.window.pyconsole = PyConsole()
+            js.pyconsole = PyConsole()
         `, namespace);
         namespace.destroy();
 
         let ps1 = '>>> ', ps2 = '... ';
 
+        async function lock(){
+          let resolve;
+          let ready = term.ready;
+          term.ready = new Promise(res => resolve = res);
+          await ready;
+          return resolve;
+        }
+
         async function interpreter(command) {
-            // multiline should be splitted (usefull when pasting)
+          let unlock = await lock();
+          try {
             term.pause();
+            // multiline should be splitted (useful when pasting)
             for( const c of command.split('\n') ) {
-                const prompt = pyconsole.push(c) ? ps2 : ps1;
-                term.set_prompt(prompt);
-                let run_complete = pyconsole.run_complete;
-                try {
+              let run_complete = pyconsole.run_complete;
+              try {
+                  const incomplete = pyconsole.push(c);
+                  term.set_prompt(incomplete ? ps2 : ps1);
                   let r = await run_complete;
-                  r.destroy();
-                } catch(_){ }
+                  if(pyodide.isPyProxy(r)){
+                    r.destroy();
+                  }
+                } catch(e){
+                  if(e.name !== "PythonError"){
+                    term.error(e);
+                    throw e;
+                  }
+                }
                 run_complete.destroy();
             }
+          } finally {
             term.resume();
+            await sleep(10);
+            unlock();
+          }
         }
 
         let term = $('body').terminal(
@@ -66,11 +89,24 @@
             }
           }
         );
-
-        pyconsole.stdout_callback = (s) => term.echo(s, {newline : false});
-        pyconsole.stderr_callback = (s) => term.echo(`[[;red;]${$.terminal.escape_brackets(s)}\u200B]`, {newline : false});
+        window.term = term;
+        pyconsole.stdout_callback = s => term.echo(s, {newline : false});
+        pyconsole.stderr_callback = s => {
+            term.error(s.trimEnd());
+        }
+        term.ready = Promise.resolve();
+        pyodide._module.on_fatal = async (e) => {
+          term.error("Pyodide has suffered a fatal error. Please report this to the Pyodide maintainers.");
+          term.error("The cause of the fatal error was:");
+          term.error(e);
+          term.error("Look in the browser console for more details.");
+          await term.ready;
+          term.pause();
+          await sleep(15);
+          term.pause();
+        };
       }
-      main();
+      window.console_ready = main();
     </script>
   </body>
 </html>
diff --git a/src/tests/test_console.py b/src/tests/test_console.py
index 28c9f7401..2ea4c8d42 100644
--- a/src/tests/test_console.py
+++ b/src/tests/test_console.py
@@ -2,6 +2,8 @@ import pytest
 from pathlib import Path
 import sys
 
+from conftest import selenium_common
+
 sys.path.append(str(Path(__file__).parents[2] / "src" / "pyodide-py"))
 
 from pyodide import console  # noqa: E402
@@ -75,7 +77,10 @@ def test_interactive_console_streams(safe_sys_redirections):
 
     my_stderr = ""
     shell.push("raise Exception('hi')")
-    assert my_stderr.endswith("Exception: hi\n")
+    assert (
+        my_stderr
+        == 'Traceback (most recent call last):\n  File "<console>", line 1, in <module>\nException: hi\n'
+    )
     assert shell.run_complete.exception() is not None
     my_stderr = ""
     shell.push("1+1")
@@ -262,3 +267,75 @@ def test_interactive_console_top_level_await(selenium, safe_selenium_sys_redirec
     selenium.run("shell.push('from js import fetch')")
     selenium.run("shell.push('await (await fetch(`packages.json`)).json()')")
     assert selenium.run("result") == None
+
+
+@pytest.fixture(params=["firefox", "chrome"], scope="function")
+def console_html_fixture(request, web_server_main):
+    with selenium_common(request, web_server_main, False) as selenium:
+        selenium.driver.get(
+            f"http://{selenium.server_hostname}:{selenium.server_port}/console.html"
+        )
+        selenium.javascript_setup()
+        try:
+            yield selenium
+        finally:
+            print(selenium.logs)
+
+
+def test_console_html(console_html_fixture):
+    selenium = console_html_fixture
+    selenium.run_js(
+        """
+        await window.console_ready;
+        """
+    )
+    result = selenium.run_js(
+        r"""
+        let result = [];
+        assert(() => term.get_output().startsWith("Welcome to the Pyodide terminal emulator 🐍"))
+
+        term.clear();
+        term.exec("1+1");
+        await term.ready;
+        assert(() => term.get_output().trim() === ">>> 1+1\n2", term.get_output().trim());
+
+
+        term.clear();
+        term.exec("1+");
+        await term.ready;
+        result.push([term.get_output(),
+`>>> 1+
+[[;;;terminal-error]  File "<console>", line 1
+    1+
+     ^
+SyntaxError: invalid syntax]`
+        ]);
+
+        term.clear();
+        term.exec("raise Exception('hi')");
+        await term.ready;
+        result.push([term.get_output(),
+`>>> raise Exception('hi')
+[[;;;terminal-error]Traceback (most recent call last):]
+[[;;;terminal-error]  File "<console>", line 1, in <module>]
+[[;;;terminal-error]Exception: hi]`
+        ]);
+
+        term.clear();
+        term.exec("from _pyodide_core import trigger_fatal_error; trigger_fatal_error()");
+        await term.ready;
+        result.push([term.get_output(),
+`>>> from _pyodide_core import trigger_fatal_error; trigger_fatal_error()
+[[;;;terminal-error]Pyodide has suffered a fatal error. Please report this to the Pyodide maintainers.]
+[[;;;terminal-error]The cause of the fatal error was:]
+[[;;;terminal-error]Error: intentionally triggered fatal error!]
+[[;;;terminal-error]Look in the browser console for more details.]`
+        ]);
+
+        await sleep(30);
+        assert(() => term.paused());
+        return result;
+        """
+    )
+    for [x, y] in result:
+        assert x == y