From 6b1330d28a9918c01de6343313ecd05a9c6bce8d Mon Sep 17 00:00:00 2001 From: Andrea Giammarchi Date: Thu, 17 Oct 2024 16:18:05 +0200 Subject: [PATCH] Fix #2220 - Avoid DOM notifications on errors (#2226) * Fix #2220 - Avoid DOM notifications on errors --- core/package-lock.json | 47 +++++++++---------------- core/package.json | 8 ++--- core/src/config.js | 2 ++ core/src/plugins/error.js | 13 +++++-- core/tests/index.html | 2 +- core/tests/javascript/mpy-error.html | 14 ++++++++ core/tests/javascript/mpy-no-error.html | 14 ++++++++ core/tests/javascript/mpy-no-error.toml | 1 + core/tests/js_tests.spec.js | 14 ++++++++ core/tests/manual/error/index.html | 39 ++++++++++++++++++++ core/tests/manual/error/pyscript.toml | 1 + core/types/plugins/error.d.ts | 1 + 12 files changed, 118 insertions(+), 38 deletions(-) create mode 100644 core/tests/javascript/mpy-error.html create mode 100644 core/tests/javascript/mpy-no-error.html create mode 100644 core/tests/javascript/mpy-no-error.toml create mode 100644 core/tests/manual/error/index.html create mode 100644 core/tests/manual/error/pyscript.toml diff --git a/core/package-lock.json b/core/package-lock.json index 88970054..eba5bdb3 100644 --- a/core/package-lock.json +++ b/core/package-lock.json @@ -1,19 +1,19 @@ { "name": "@pyscript/core", - "version": "0.6.6", + "version": "0.6.7", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@pyscript/core", - "version": "0.6.6", + "version": "0.6.7", "license": "APACHE-2.0", "dependencies": { "@ungap/with-resolvers": "^0.1.0", "@webreflection/idb-map": "^0.3.2", - "add-promise-listener": "^0.1.1", + "add-promise-listener": "^0.1.3", "basic-devtools": "^0.1.6", - "polyscript": "^0.16.2", + "polyscript": "^0.16.3", "sabayon": "^0.5.2", "sticky-module": "^0.1.1", "to-json-callback": "^0.1.1", @@ -26,7 +26,7 @@ "@codemirror/state": "^6.4.1", "@codemirror/view": "^6.34.1", "@playwright/test": "1.45.3", - "@rollup/plugin-commonjs": "^28.0.0", + "@rollup/plugin-commonjs": "^28.0.1", "@rollup/plugin-node-resolve": "^15.3.0", "@rollup/plugin-terser": "^0.4.4", "@webreflection/toml-j0.4": "^1.1.3", @@ -560,19 +560,19 @@ } }, "node_modules/@rollup/plugin-commonjs": { - "version": "28.0.0", - "resolved": "https://registry.npmjs.org/@rollup/plugin-commonjs/-/plugin-commonjs-28.0.0.tgz", - "integrity": "sha512-BJcu+a+Mpq476DMXG+hevgPSl56bkUoi88dKT8t3RyUp8kGuOh+2bU8Gs7zXDlu+fyZggnJ+iOBGrb/O1SorYg==", + "version": "28.0.1", + "resolved": "https://registry.npmjs.org/@rollup/plugin-commonjs/-/plugin-commonjs-28.0.1.tgz", + "integrity": "sha512-+tNWdlWKbpB3WgBN7ijjYkq9X5uhjmcvyjEght4NmH5fAU++zfQzAJ6wumLS+dNcvwEZhKx2Z+skY8m7v0wGSA==", "dev": true, "license": "MIT", "dependencies": { "@rollup/pluginutils": "^5.0.1", "commondir": "^1.0.1", "estree-walker": "^2.0.2", - "fdir": "^6.1.1", + "fdir": "^6.2.0", "is-reference": "1.2.1", "magic-string": "^0.30.3", - "picomatch": "^2.3.1" + "picomatch": "^4.0.2" }, "engines": { "node": ">=16.0.0 || 14 >= 14.17" @@ -586,19 +586,6 @@ } } }, - "node_modules/@rollup/plugin-commonjs/node_modules/picomatch": { - "version": "2.3.1", - "resolved": "https://registry.npmjs.org/picomatch/-/picomatch-2.3.1.tgz", - "integrity": "sha512-JU3teHTNjmE2VCGFzuY8EXzCDVwEqB2a8fsIvwaStHhAWJEeVd1o1QD80CU6+ZdEXXSLbSsuLwJjkCBWqRQUVA==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">=8.6" - }, - "funding": { - "url": "https://github.com/sponsors/jonschlinkert" - } - }, "node_modules/@rollup/plugin-node-resolve": { "version": "15.3.0", "resolved": "https://registry.npmjs.org/@rollup/plugin-node-resolve/-/plugin-node-resolve-15.3.0.tgz", @@ -1020,9 +1007,9 @@ } }, "node_modules/add-promise-listener": { - "version": "0.1.1", - "resolved": "https://registry.npmjs.org/add-promise-listener/-/add-promise-listener-0.1.1.tgz", - "integrity": "sha512-b3DQJ4VBQ1e4bjVPd0mqHkgFt4MYD8jYTEcfN9Qx+bGYs+WRLxPDnX7fRztGRm1k4CZzeXWvvYif6L1q4TToqg==", + "version": "0.1.3", + "resolved": "https://registry.npmjs.org/add-promise-listener/-/add-promise-listener-0.1.3.tgz", + "integrity": "sha512-hQ6IgGJ7NvvlPYbwdekhdVwPb4QzEptNZ5v7B4XRKz7FukUPDuF/v+R5EFHArWmhmq4d+xv0G4/B5bu2GSiz9Q==", "license": "MIT" }, "node_modules/ajv": { @@ -2588,8 +2575,6 @@ "integrity": "sha512-M7BAV6Rlcy5u+m6oPhAPFgJTzAioX/6B0DxyvDlo9l8+T3nLKbrczg2WLUyzd45L8RqfUMyGPzekbMvX2Ldkwg==", "dev": true, "license": "MIT", - "optional": true, - "peer": true, "engines": { "node": ">=12" }, @@ -2649,9 +2634,9 @@ } }, "node_modules/polyscript": { - "version": "0.16.2", - "resolved": "https://registry.npmjs.org/polyscript/-/polyscript-0.16.2.tgz", - "integrity": "sha512-S+RHW3ogB4nFlajp+Pd+YWuoEJMRBNuFwcke3H/lxlq8ZkX2lEZhvvda+QcwAsQXitmWg3Wi0Oz8fuwXyIKZeg==", + "version": "0.16.3", + "resolved": "https://registry.npmjs.org/polyscript/-/polyscript-0.16.3.tgz", + "integrity": "sha512-I3kHxt62FMRAX2iVl24iCEtG4UnUInMSbv/LnwevkmjOErLPAQtES4CNzU/fgKRpXYCqp0WWQaRvRYkJhpMIbA==", "license": "APACHE-2.0", "dependencies": { "@ungap/structured-clone": "^1.2.0", diff --git a/core/package.json b/core/package.json index 26c13526..3c3994d0 100644 --- a/core/package.json +++ b/core/package.json @@ -1,6 +1,6 @@ { "name": "@pyscript/core", - "version": "0.6.6", + "version": "0.6.7", "type": "module", "description": "PyScript", "module": "./index.js", @@ -60,9 +60,9 @@ "dependencies": { "@ungap/with-resolvers": "^0.1.0", "@webreflection/idb-map": "^0.3.2", - "add-promise-listener": "^0.1.1", + "add-promise-listener": "^0.1.3", "basic-devtools": "^0.1.6", - "polyscript": "^0.16.2", + "polyscript": "^0.16.3", "sabayon": "^0.5.2", "sticky-module": "^0.1.1", "to-json-callback": "^0.1.1", @@ -75,7 +75,7 @@ "@codemirror/state": "^6.4.1", "@codemirror/view": "^6.34.1", "@playwright/test": "1.45.3", - "@rollup/plugin-commonjs": "^28.0.0", + "@rollup/plugin-commonjs": "^28.0.1", "@rollup/plugin-node-resolve": "^15.3.0", "@rollup/plugin-terser": "^0.4.4", "@webreflection/toml-j0.4": "^1.1.3", diff --git a/core/src/config.js b/core/src/config.js index 040bbbe5..c6a32c88 100644 --- a/core/src/config.js +++ b/core/src/config.js @@ -146,6 +146,8 @@ for (const [TYPE] of TYPES) { } } else if (!parsed?.plugins?.includes(`!${key}`)) { toBeAwaited.push(value().then(({ default: p }) => p)); + } else if (key === "error") { + toBeAwaited.push(value().then(({ notOnDOM }) => notOnDOM())); } } diff --git a/core/src/plugins/error.js b/core/src/plugins/error.js index 7eb3dde2..2ff35313 100644 --- a/core/src/plugins/error.js +++ b/core/src/plugins/error.js @@ -1,6 +1,12 @@ // PyScript Error Plugin +import { buffered } from "polyscript/exports"; import { hooks } from "../core.js"; +let dontBotherDOM = false; +export function notOnDOM() { + dontBotherDOM = true; +} + hooks.main.onReady.add(function override(pyScript) { // be sure this override happens only once hooks.main.onReady.delete(override); @@ -8,13 +14,15 @@ hooks.main.onReady.add(function override(pyScript) { // trap generic `stderr` to propagate to it regardless const { stderr } = pyScript.io; - // override it with our own logic - pyScript.io.stderr = (error, ...rest) => { + const cb = (error, ...rest) => { notify(error.message || error); // let other plugins or stderr hook, if any, do the rest return stderr(error, ...rest); }; + // override it with our own logic + pyScript.io.stderr = pyScript.type === "py" ? cb : buffered(cb); + // be sure uncaught Python errors are also visible addEventListener("error", ({ message }) => { if (message.startsWith("Uncaught PythonError")) notify(message); @@ -30,6 +38,7 @@ hooks.main.onReady.add(function override(pyScript) { * @param {string} message */ export function notify(message) { + if (dontBotherDOM) return; const div = document.createElement("div"); div.className = "py-error"; div.textContent = message; diff --git a/core/tests/index.html b/core/tests/index.html index 7a09479e..43971239 100644 --- a/core/tests/index.html +++ b/core/tests/index.html @@ -14,5 +14,5 @@ a:hover { opacity: 1; } - + diff --git a/core/tests/javascript/mpy-error.html b/core/tests/javascript/mpy-error.html new file mode 100644 index 00000000..a8c62206 --- /dev/null +++ b/core/tests/javascript/mpy-error.html @@ -0,0 +1,14 @@ + + + + + + + + + diff --git a/core/tests/javascript/mpy-no-error.html b/core/tests/javascript/mpy-no-error.html new file mode 100644 index 00000000..6084dcc2 --- /dev/null +++ b/core/tests/javascript/mpy-no-error.html @@ -0,0 +1,14 @@ + + + + + + + + + diff --git a/core/tests/javascript/mpy-no-error.toml b/core/tests/javascript/mpy-no-error.toml new file mode 100644 index 00000000..4e7f1f7e --- /dev/null +++ b/core/tests/javascript/mpy-no-error.toml @@ -0,0 +1 @@ +plugins = ["!error"] diff --git a/core/tests/js_tests.spec.js b/core/tests/js_tests.spec.js index aff590e0..0fa09b99 100644 --- a/core/tests/js_tests.spec.js +++ b/core/tests/js_tests.spec.js @@ -138,3 +138,17 @@ test('Pyodide lockFileURL vs CDN', async ({ page }) => { const body = await page.evaluate(() => document.body.textContent); await expect(body).toBe('OK'); }); + +test('MicroPython buffered error', async ({ page }) => { + await page.goto('http://localhost:8080/tests/javascript/mpy-error.html'); + await page.waitForSelector('html.ok'); + const body = await page.evaluate(() => document.body.textContent.trim()); + await expect(body).toBe('This is an error'); +}); + +test('MicroPython buffered NO error', async ({ page }) => { + await page.goto('http://localhost:8080/tests/javascript/mpy-no-error.html'); + await page.waitForSelector('html.ok'); + const body = await page.evaluate(() => document.body.textContent.trim()); + await expect(body).toBe(''); +}); diff --git a/core/tests/manual/error/index.html b/core/tests/manual/error/index.html new file mode 100644 index 00000000..04b50f10 --- /dev/null +++ b/core/tests/manual/error/index.html @@ -0,0 +1,39 @@ + + + + + + PyScript Error Bug? + + + + + + plugins = ["!error"] + + + + + +
+ + diff --git a/core/tests/manual/error/pyscript.toml b/core/tests/manual/error/pyscript.toml new file mode 100644 index 00000000..4e7f1f7e --- /dev/null +++ b/core/tests/manual/error/pyscript.toml @@ -0,0 +1 @@ +plugins = ["!error"] diff --git a/core/types/plugins/error.d.ts b/core/types/plugins/error.d.ts index bf9a6788..40bdfd0d 100644 --- a/core/types/plugins/error.d.ts +++ b/core/types/plugins/error.d.ts @@ -1,3 +1,4 @@ +export function notOnDOM(): void; /** * Add a banner to the top of the page, notifying the user of an error * @param {string} message