Create error or warning level banners and allow users to close it (#909)

pyscriptjs/src/consts.ts

@@ -0,0 +1 @@
+export const CLOSEBUTTON = `<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 16 16' fill="currentColor" width="12px"><path d='M.293.293a1 1 0 011.414 0L8 6.586 14.293.293a1 1 0 111.414 1.414L9.414 8l6.293 6.293a1 1 0 01-1.414 1.414L8 9.414l-6.293 6.293a1 1 0 01-1.414-1.414L6.586 8 .293 1.707a1 1 0 010-1.414z'/></svg>`

pyscriptjs/src/exceptions.ts

@@ -0,0 +1,61 @@
+import { CLOSEBUTTON } from "./consts"
+export class UserError extends Error {
+  constructor(message: string) {
+    super(message)
+    this.name = "UserError"
+  }
+}
+
+export function _createAlertBanner(message: string, level: "error" | "warning" = "error", logMessage = true) {
+  // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
+  switch (`log-${level}-${logMessage}`) {
+    case "log-error-true":
+      console.error(message);
+      break;
+    case "log-warning-true":
+      console.warn(message)
+      break;
+  }
+
+  const banner = document.createElement("div")
+  banner.className = `alert-banner py-${level}`
+  banner.innerHTML = message
+
+  if (level === "warning") {
+    const closeButton = document.createElement("button")
+
+    closeButton.id = "alert-close-button"
+    closeButton.addEventListener("click", () => {
+      banner.remove()
+    })
+    closeButton.innerHTML = CLOSEBUTTON
+
+    banner.appendChild(closeButton)
+  }
+
+  document.body.prepend(banner)
+}
+
+/*
+* This function is used to handle UserError, if we see an error of this
+* type, we will automatically create a banner on the page that will tell
+* the user what went wrong. Note that the error will still stop execution,
+* any other errors we will simply throw them and no banner will be shown.
+*/
+export function withUserErrorHandler(fn) {
+  try {
+    return fn();
+  } catch (error: unknown) {
+    if (error instanceof UserError) {
+      /*
+      *  Display a page-wide error message to show that something has gone wrong with
+      *  PyScript or Pyodide during loading. Probably not be used for issues that occur within
+      *  Python scripts, since stderr can be routed to somewhere in the DOM
+      */
+      _createAlertBanner(error.message)
+    }
+    else {
+      throw error
+    }
+  }
+}

pyscriptjs/src/main.ts

@@ -7,9 +7,10 @@ import { make_PyScript, initHandlers, mountElements } from './components/pyscrip
 import { PyLoader } from './components/pyloader';
 import { PyodideRuntime } from './pyodide';
 import { getLogger } from './logger';
-import { handleFetchError, showError, globalExport } from './utils';
+import { handleFetchError, showWarning, globalExport } from './utils';
 import { calculatePaths } from './plugins/fetch';
 import { createCustomElements } from './components/elements';
+import { UserError, withUserErrorHandler } from "./exceptions"
 
 type ImportType = { [key: string]: unknown };
 type ImportMapType = {
@@ -78,9 +79,9 @@ class PyScriptApp {
             // errors" and stop the computation, but currently our life cycle
             // is too messy to implement it reliably. We might want to revisit
             // this once it's in a better shape.
-            showError(
+            showWarning(
                 'Multiple <py-config> tags detected. Only the first is ' +
-                    'going to be parsed, all the others will be ignored',
+                'going to be parsed, all the others will be ignored',
             );
         }
         this.config = loadConfigFromElement(el);
@@ -100,12 +101,11 @@ class PyScriptApp {
     loadRuntime() {
         logger.info('Initializing runtime');
         if (this.config.runtimes.length == 0) {
-            showError('Fatal error: config.runtimes is empty');
-            return;
+            throw new UserError('Fatal error: config.runtimes is empty');
         }
 
         if (this.config.runtimes.length > 1) {
-            showError('Multiple runtimes are not supported yet. ' + 'Only the first will be used');
+            showWarning('Multiple runtimes are not supported yet.<br />Only the first will be used');
         }
         const runtime_cfg = this.config.runtimes[0];
         this.runtime = new PyodideRuntime(this.config, runtime_cfg.src, runtime_cfg.name, runtime_cfg.lang);
@@ -179,6 +179,8 @@ class PyScriptApp {
             try {
                 await runtime.loadFromFile(paths[i], fetchPaths[i]);
             } catch (e) {
+                // Remove the loader so users can see the banner better
+                this.loader.remove()
                 //Should we still export full error contents to console?
                 handleFetchError(<Error>e, fetchPaths[i]);
             }
@@ -243,6 +245,6 @@ globalExport('pyscript_get_config', pyscript_get_config);
 
 // main entry point of execution
 const globalApp = new PyScriptApp();
-globalApp.main();
+withUserErrorHandler(globalApp.main.bind(globalApp));
 
 export const runtime = globalApp.runtime;

pyscriptjs/src/pyconfig.ts

@@ -1,7 +1,8 @@
 import toml from '../src/toml';
 import { getLogger } from './logger';
 import { version } from './runtime';
-import { getAttribute, readTextFromPath, showError } from './utils';
+import { getAttribute, readTextFromPath } from './utils';
+import { UserError } from "./exceptions"
 
 const logger = getLogger('py-config');
 
@@ -147,34 +148,22 @@ function parseConfig(configText: string, configType = 'toml') {
         try {
             // TOML parser is soft and can parse even JSON strings, this additional check prevents it.
             if (configText.trim()[0] === '{') {
-                const errMessage = `config supplied: ${configText} is an invalid TOML and cannot be parsed`;
-                showError(`<p>${errMessage}</p>`);
-                throw Error(errMessage);
+                throw new UserError(`The config supplied: ${configText} is an invalid TOML and cannot be parsed`);
             }
             config = toml.parse(configText);
         } catch (err) {
             const errMessage: string = err.toString();
-            showError(`<p>config supplied: ${configText} is an invalid TOML and cannot be parsed: ${errMessage}</p>`);
-            // we cannot easily just "throw err" here, because for some reason
-            // playwright gets confused by it and cannot print it
-            // correctly. It is just displayed as an empty error.
-            // If you print err in JS, you get something like this:
-            //     n {message: '...', offset: 19, line: 2, column: 19}
-            // I think that 'n' is the minified name?
-            // The workaround is to re-wrap the message into SyntaxError(), so that
-            // it's correctly handled by playwright.
-            throw SyntaxError(errMessage);
+            throw new UserError(`The config supplied: ${configText} is an invalid TOML and cannot be parsed: ${errMessage}`);
         }
     } else if (configType === 'json') {
         try {
             config = JSON.parse(configText);
         } catch (err) {
             const errMessage: string = err.toString();
-            showError(`<p>config supplied: ${configText} is an invalid JSON and cannot be parsed: ${errMessage}</p>`);
-            throw err;
+            throw new UserError(`The config supplied: ${configText} is an invalid JSON and cannot be parsed: ${errMessage}`);
         }
     } else {
-        showError(`<p>type of config supplied is: ${configType}, supported values are ["toml", "json"].</p>`);
+        throw new UserError(`<p>The type of config supplied'${configType}' is not supported, supported values are ["toml", "json"].</p>`);
     }
     return config;
 }

pyscriptjs/src/styles/pyscript_base.css

@@ -109,12 +109,46 @@ color: #0f172a;
 }
 
 /* Pop-up second layer end */
+.alert-banner {
+    position: relative;
+    width: 99%;
+    padding: .5rem;
+    margin: 5px 0;
+}
+
+.alert-banner p {
+    margin: 0;
+}
 
 .py-error{
-    background-color: rgb(254 226 226);
+    background-color: #FFE9E8;
     border: solid;
-    border-color: #fca5a5;
-    color: #ff0000;
+    border-color: #f0625f;
+    color: #9d041c;
+}
+
+.py-warning {
+    background-color: rgb(255, 244, 229);
+    border: solid;
+    border-color: #ffa016;
+    color: #794700;
+}
+
+.alert-banner.py-error>#alert-close-button {
+    color: #9d041c;
+}
+
+.alert-banner.py-warning>#alert-close-button {
+    color: #794700
+}
+
+#alert-close-button {
+position: absolute;
+right: .5rem;
+top: .5rem;
+cursor: pointer;
+background: transparent;
+border: none;
 }
 
 .py-box{

pyscriptjs/src/utils.ts

@@ -1,3 +1,5 @@
+import { _createAlertBanner, UserError } from "./exceptions"
+
 export function addClasses(element: HTMLElement, classes: string[]) {
     for (const entry of classes) {
         element.classList.add(entry);
@@ -39,21 +41,8 @@ export function ensureUniqueId(el: HTMLElement) {
     if (el.id === '') el.id = `py-internal-${_uniqueIdCounter++}`;
 }
 
-/*
- *  Display a page-wide error message to show that something has gone wrong with
- *  PyScript or Pyodide during loading. Probably not be used for issues that occur within
- *  Python scripts, since stderr can be routed to somewhere in the DOM
- */
-export function showError(msg: string): void {
-    const warning = document.createElement('div');
-    // XXX: the style should go to css instead of here probably
-    warning.className = 'py-error';
-    warning.style.backgroundColor = 'LightCoral';
-    warning.style.alignContent = 'center';
-    warning.style.margin = '4px';
-    warning.style.padding = '4px';
-    warning.innerHTML = msg;
-    document.body.prepend(warning);
+export function showWarning(msg: string): void {
+    _createAlertBanner(msg, "warning")
 }
 
 export function handleFetchError(e: Error, singleFile: string) {
@@ -75,9 +64,13 @@ export function handleFetchError(e: Error, singleFile: string) {
             singleFile +
             `</u> failed with error 404 (File not Found). Are your filename and path are correct?</p>`;
     } else {
-        errorContent = '<p>PyScript encountered an error while loading from file: ' + e.message + '</p>';
+        errorContent = `<p>PyScript encountered an error while loading from file: ${e.message} </p>`;
     }
-    showError(errorContent);
+    // We need to create the banner because `handleFetchError` is called before we
+    // use withUserErrorHandler in main.js we are also disabling the log message
+    // because it will be logged by the uncaught exception in promise.
+    _createAlertBanner(errorContent, "error", false)
+    throw new UserError(errorContent)
 }
 
 export function readTextFromPath(path: string) {

pyscriptjs/tests/integration/test_01_basic.py

@@ -81,7 +81,6 @@ class TestBasic(PyScriptTest):
                 # which are built and distributed with pyodide
                 packages = ["asciitree"]
             </py-config>
-
             <py-script>
                 import js
                 import asciitree

pyscriptjs/tests/integration/test_py_config.py

@@ -5,7 +5,7 @@ import tempfile
 import pytest
 import requests
 
-from .support import PyScriptTest
+from .support import JsErrors, PyScriptTest
 
 URL = "https://github.com/pyodide/pyodide/releases/download/0.20.0/pyodide-build-0.20.0.tar.bz2"
 TAR_NAME = "pyodide-build-0.20.0.tar.bz2"
@@ -115,8 +115,13 @@ class TestConfig(PyScriptTest):
             """,
             wait_for_pyscript=False,
         )
-        self.page.wait_for_selector(".py-error")
-        self.check_js_errors("Unexpected end of JSON input")
+        banner = self.page.wait_for_selector(".py-error")
+        assert "SyntaxError: Unexpected end of JSON input" in self.console.error.text
+        expected = (
+            "The config supplied: [[ is an invalid JSON and cannot be "
+            "parsed: SyntaxError: Unexpected end of JSON input"
+        )
+        assert banner.inner_text() == expected
 
     def test_invalid_toml_config(self):
         # we need wait_for_pyscript=False because we bail out very soon,
@@ -129,8 +134,14 @@ class TestConfig(PyScriptTest):
             """,
             wait_for_pyscript=False,
         )
-        self.page.wait_for_selector(".py-error")
-        self.check_js_errors("SyntaxError: Expected DoubleQuote")
+        banner = self.page.wait_for_selector(".py-error")
+        assert "SyntaxError: Expected DoubleQuote" in self.console.error.text
+        expected = (
+            "The config supplied: [[ is an invalid TOML and cannot be parsed: "
+            "SyntaxError: Expected DoubleQuote, Whitespace, or [a-z], [A-Z], "
+            '[0-9], "-", "_" but "\\n" found.'
+        )
+        assert banner.inner_text() == expected
 
     def test_multiple_py_config(self):
         self.pyscript_run(
@@ -150,12 +161,12 @@ class TestConfig(PyScriptTest):
             </py-script>
             """
         )
-        div = self.page.wait_for_selector(".py-error")
+        banner = self.page.wait_for_selector(".py-warning")
         expected = (
             "Multiple <py-config> tags detected. Only the first "
             "is going to be parsed, all the others will be ignored"
         )
-        assert div.text_content() == expected
+        assert banner.text_content() == expected
 
     def test_no_runtimes(self):
         snippet = """
@@ -194,11 +205,9 @@ class TestConfig(PyScriptTest):
             </py-script>
         """
         self.pyscript_run(snippet)
-        div = self.page.wait_for_selector(".py-error")
-        expected = (
-            "Multiple runtimes are not supported yet. Only the first will be used"
-        )
-        assert div.text_content() == expected
+        banner = self.page.wait_for_selector(".py-warning")
+        expected = "Multiple runtimes are not supported yet.Only the first will be used"
+        assert banner.text_content() == expected
         assert self.console.log.lines[-1] == "hello world"
 
     def test_paths(self):
@@ -232,13 +241,9 @@ class TestConfig(PyScriptTest):
                 [[fetch]]
                 files = ["./f.py"]
             </py-config>
-            """
+            """,
+            wait_for_pyscript=False,
         )
-        assert self.console.error.lines == ["Failed to load resource: net::ERR_FAILED"]
-        assert self.console.warning.lines == [
-            "Caught an error in fetchPaths:\r\n TypeError: Failed to fetch"
-        ]
-
         errorContent = """PyScript: Access to local files
         (using "Paths:" in &lt;py-config&gt;)
         is not available when directly opening a HTML file;
@@ -246,6 +251,15 @@ class TestConfig(PyScriptTest):
 
         inner_html = self.page.locator(".py-error").inner_html()
         assert errorContent in inner_html
+        assert "Failed to load resource: net::ERR_FAILED" in self.console.error.lines
+        assert (
+            "Caught an error in fetchPaths:\r\n TypeError: Failed to fetch"
+            in self.console.warning.lines
+        )
+        with pytest.raises(JsErrors) as exc:
+            self.check_js_errors()
+
+        assert errorContent in str(exc.value)
 
     def test_paths_from_packages(self):
         self.writefile("utils/__init__.py", "")

pyscriptjs/tests/unit/exceptions.test.ts

@@ -0,0 +1,117 @@
+import { jest } from "@jest/globals"
+import { _createAlertBanner, withUserErrorHandler, UserError } from "../../src/exceptions"
+
+describe("Test _createAlertBanner", () => {
+
+  afterEach(() => {
+    // Ensure we always have a clean body
+    document.body.innerHTML = `<div>Hello World</div>`
+  })
+
+
+  it("error level shouldn't contain close button", async () => {
+    _createAlertBanner("Something went wrong!", "error")
+
+    const banner = document.getElementsByClassName("alert-banner")
+    const closeButton = document.getElementById("alert-close-button")
+    expect(banner.length).toBe(1)
+    expect(banner[0].innerHTML).toBe("Something went wrong!")
+    expect(closeButton).toBeNull()
+  })
+
+  it("warning level should contain close button", async () => {
+    _createAlertBanner("This is a warning", "warning")
+
+    const banner = document.getElementsByClassName("alert-banner")
+    const closeButton = document.getElementById("alert-close-button")
+    expect(banner.length).toBe(1)
+    expect(banner[0].innerHTML).toContain("This is a warning")
+    expect(closeButton).not.toBeNull()
+  })
+
+  it("error level banner should log to console", async () => {
+    const logSpy = jest.spyOn(console, "error")
+
+    _createAlertBanner("Something went wrong!")
+
+    expect(logSpy).toHaveBeenCalledWith("Something went wrong!")
+
+  })
+
+  it("warning level banner should log to console", async () => {
+    const logSpy = jest.spyOn(console, "warn")
+
+    _createAlertBanner("This warning", "warning")
+
+    expect(logSpy).toHaveBeenCalledWith("This warning")
+  })
+
+  it("close button should remove element from page", async () => {
+    let banner = document.getElementsByClassName("alert-banner")
+    expect(banner.length).toBe(0)
+
+    _createAlertBanner("Warning!", "warning")
+
+    // Just a sanity check
+    banner = document.getElementsByClassName("alert-banner")
+    expect(banner.length).toBe(1)
+
+    const closeButton = document.getElementById("alert-close-button")
+    if(closeButton) {
+      closeButton.click()
+      // Confirm that clicking the close button, removes the element
+      banner = document.getElementsByClassName("alert-banner")
+      expect(banner.length).toBe(0)
+    } else {
+      fail("Unable to find close button on the page, but should exist")
+    }
+
+  })
+
+  it("toggling logging off on error alert shouldn't log to console", async () => {
+    const errorLogSpy = jest.spyOn(console, "error")
+
+    _createAlertBanner("Test error", "error", false)
+    expect(errorLogSpy).not.toHaveBeenCalledWith("Test error")
+  })
+
+  it("toggling logging off on warning alert shouldn't log to console", async () => {
+    const warnLogSpy = jest.spyOn(console, "warn")
+    _createAlertBanner("Test warning", "warning", false)
+    expect(warnLogSpy).not.toHaveBeenCalledWith("Test warning")
+  })
+})
+
+
+describe("Test withUserErrorHandler", () => {
+
+  afterEach(() => {
+    // Ensure we always have a clean body
+    document.body.innerHTML = `<div>Hello World</div>`
+  })
+
+  it("userError doesn't stop execution", async () => {
+    function exception() {
+      throw new UserError("Computer says no")
+    }
+
+    function func() {
+      withUserErrorHandler(exception)
+      return "Hello, world"
+    }
+
+    const returnValue = func()
+    const banners = document.getElementsByClassName("alert-banner")
+    expect(banners.length).toBe(1)
+    expect(banners[0].innerHTML).toBe("Computer says no")
+    expect(returnValue).toBe("Hello, world")
+  })
+
+  it("any other exception should stop execution and raise", async () => {
+    function exception() {
+      throw new Error("Explosions!")
+    }
+
+    expect(() => withUserErrorHandler(exception)).toThrow(new Error("Explosions!"))
+  })
+})

pyscriptjs/tests/unit/pyconfig.test.ts

@@ -1,6 +1,7 @@
 import { jest } from '@jest/globals';
 import { loadConfigFromElement, defaultConfig } from '../../src/pyconfig';
 import { version } from '../../src/runtime';
+import { UserError } from '../../src/exceptions'
 
 // inspired by trump typos
 const covfefeConfig = {
@@ -128,7 +129,7 @@ describe('loadConfigFromElement', () => {
     it('should NOT be able to load an inline config in TOML format with type as JSON', () => {
         const el = make_config_element({ type: 'json' });
         el.innerHTML = covfefeConfigToml;
-        expect(()=>loadConfigFromElement(el)).toThrow(SyntaxError);
+        expect(()=>loadConfigFromElement(el)).toThrow(UserError);
     });
 
     it('should NOT be able to load an inline TOML config with a JSON config from src with type as toml', () => {
@@ -140,19 +141,19 @@ describe('loadConfigFromElement', () => {
     it('should NOT be able to load an inline TOML config with a JSON config from src with type as json', () => {
         const el = make_config_element({ type: 'json', src: '/covfefe.json' });
         el.innerHTML = covfefeConfigToml;
-        expect(()=>loadConfigFromElement(el)).toThrow(SyntaxError);
+        expect(()=>loadConfigFromElement(el)).toThrow(UserError);
     });
 
     it('should error out when passing an invalid JSON', () => {
         const el = make_config_element({ type: 'json' });
         el.innerHTML = '[[';
-        expect(()=>loadConfigFromElement(el)).toThrow(SyntaxError);
+        expect(()=>loadConfigFromElement(el)).toThrow(UserError);
     });
 
     it('should error out when passing an invalid TOML', () => {
         const el = make_config_element({});
         el.innerHTML = '[[';
-        expect(()=>loadConfigFromElement(el)).toThrow(SyntaxError);
+        expect(()=>loadConfigFromElement(el)).toThrow(UserError);
     });
 
 });

pyscriptjs/tests/unit/utils.test.ts

@@ -1,5 +1,6 @@
-import { ensureUniqueId, joinPaths } from '../../src/utils';
-import { expect } from "@jest/globals";
+import { expect } from "@jest/globals"
+import { ensureUniqueId, joinPaths } from "../../src/utils"
+
 
 describe("Utils", () => {