From 1b0b5ea1952d1cce1f7c7191263146018ef436a3 Mon Sep 17 00:00:00 2001 From: Patrick Cherry Date: Thu, 26 Feb 2026 17:47:17 +0000 Subject: [PATCH 01/11] Specifically mangle traceback for `sense_hat.py` errors This commit specifically fixes the issue `TypeError: object of type 'int' has no len() on line 1030 of sense_hat.py` by a more explicit check to see if the pixel object has a `__len__` attribute. Also, the I mangled traceback when the filename is `./sense_hat.py` to skip that first file, hopefully pointing to the line in the user's code instead. --- public/shims/sense_hat/sense_hat_blob.py | 13 ++++++++----- .../PythonRunner/SkulptRunner/SkulptRunner.jsx | 9 +++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/public/shims/sense_hat/sense_hat_blob.py b/public/shims/sense_hat/sense_hat_blob.py index 0ec207f71..ebd491506 100644 --- a/public/shims/sense_hat/sense_hat_blob.py +++ b/public/shims/sense_hat/sense_hat_blob.py @@ -1074,16 +1074,19 @@ def set_pixel(self, x, y, *args): ap.set_pixel(x, y, pixel) """ - pixel_error = 'Pixel arguments must be given as (r, g, b) or r, g, b' - if len(args) == 1: pixel = args[0] - if len(pixel) != 3: - raise ValueError(pixel_error) elif len(args) == 3: pixel = args else: - raise ValueError(pixel_error) + pixel = None + + # Check if pixel responds to len, and if it has 3 elements (r, g, b) + if not hasattr(pixel, '__len__') or len(pixel) != 3: + raise ValueError( + 'Pixel arguments must be given as (r, g, b) or r, g, b. ' + 'Did you forget the x, y coordinates?' + ) if x > 7 or x < 0: raise ValueError('X position must be between 0 and 7') diff --git a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx index 0e19c63c7..032a08b37 100644 --- a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx +++ b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx @@ -340,6 +340,15 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => { .replace(/\[(.*?)\]/, "") .replace(/\.$/, ""); const errorType = err.tp$name || err.constructor.name; + + // If this is an error in the sense_hat.py shim, remove the first line of + // the traceback as this will be the line in the shim which we don't want + // to show to users, so that the error message will instead point to the + // line in the user's code which caused the error. + if (err.traceback[0].filename === "./sense_hat.py") { + err.traceback.shift(); + } + const lineNumber = err.traceback[0].lineno; const fileName = err.traceback[0].filename.replace(/^\.\//, ""); From 3d206a475ad183a2a2ef404b837cdc023d513680 Mon Sep 17 00:00:00 2001 From: Patrick Cherry Date: Thu, 26 Feb 2026 18:08:58 +0000 Subject: [PATCH 02/11] Fix pixel length checks in set_pixels too --- public/shims/sense_hat/sense_hat_blob.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/public/shims/sense_hat/sense_hat_blob.py b/public/shims/sense_hat/sense_hat_blob.py index ebd491506..ef4b9f389 100644 --- a/public/shims/sense_hat/sense_hat_blob.py +++ b/public/shims/sense_hat/sense_hat_blob.py @@ -1023,12 +1023,15 @@ def set_pixels(self, pixel_list): and 255 """ - if len(pixel_list) != 64: - raise ValueError('Pixel lists must have 64 elements') + if not hasattr(pixel_list, '__len__') or len(pixel_list) != 64: + raise ValueError('Pixel lists must be a list of 64 elements') for index, pix in enumerate(pixel_list): - if len(pix) != 3: - raise ValueError('Pixel at index %d is invalid. Pixels must contain 3 elements: Red, Green and Blue' % index) + if not hasattr(pix, '__len__') or len(pix) != 3: + raise ValueError( + 'Pixel at index %d is invalid. ' + 'Pixels must contain 3 elements: Red, Green and Blue' % index + ) for element in pix: if element > 255 or element < 0: From d55dea37d1e95842c7ddb02e6979a37da77cd704 Mon Sep 17 00:00:00 2001 From: Patrick Cherry Date: Thu, 26 Feb 2026 18:12:28 +0000 Subject: [PATCH 03/11] Add safe navigation for the traceback --- public/shims/sense_hat/sense_hat_blob.py | 6 +++--- .../Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/public/shims/sense_hat/sense_hat_blob.py b/public/shims/sense_hat/sense_hat_blob.py index ef4b9f389..7367ec9d3 100644 --- a/public/shims/sense_hat/sense_hat_blob.py +++ b/public/shims/sense_hat/sense_hat_blob.py @@ -1029,9 +1029,9 @@ def set_pixels(self, pixel_list): for index, pix in enumerate(pixel_list): if not hasattr(pix, '__len__') or len(pix) != 3: raise ValueError( - 'Pixel at index %d is invalid. ' - 'Pixels must contain 3 elements: Red, Green and Blue' % index - ) + f"Pixel at index {index} is invalid. " + 'Pixels must contain 3 elements: Red, Green and Blue' + ) for element in pix: if element > 255 or element < 0: diff --git a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx index 032a08b37..7ef6c998a 100644 --- a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx +++ b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx @@ -345,7 +345,7 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => { // the traceback as this will be the line in the shim which we don't want // to show to users, so that the error message will instead point to the // line in the user's code which caused the error. - if (err.traceback[0].filename === "./sense_hat.py") { + if (err.traceback[0]?.filename === "./sense_hat.py") { err.traceback.shift(); } From 164ee52da2c786ad7431251e8c13ae4f96be52cb Mon Sep 17 00:00:00 2001 From: Patrick Cherry Date: Thu, 26 Feb 2026 21:43:54 +0000 Subject: [PATCH 04/11] Add safety to the check; add tests --- .../SkulptRunner/SkulptRunner.jsx | 2 +- .../SkulptRunner/SkulptRunner.test.js | 61 +++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx index 7ef6c998a..fcf6db07e 100644 --- a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx +++ b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx @@ -345,7 +345,7 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => { // the traceback as this will be the line in the shim which we don't want // to show to users, so that the error message will instead point to the // line in the user's code which caused the error. - if (err.traceback[0]?.filename === "./sense_hat.py") { + if (err.traceback.length > 1 && err.traceback[0].filename === "./sense_hat.py") { err.traceback.shift(); } diff --git a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js index 94c5c826f..ac5f11c7f 100644 --- a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js +++ b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js @@ -224,6 +224,67 @@ describe("When an error occurs", () => { }); }); +describe("When an error originates in the sense_hat shim", () => { + let store; + + beforeEach(() => { + const middlewares = []; + const mockStore = configureStore(middlewares); + const initialState = { + editor: { + project: { + components: [ + { + name: "main", + extension: "py", + content: + "from sense_hat import set_pixel\nset_pixel(255, 0, 0, 0, 0)", + }, + { + name: "sense_hat", + extension: "py", + content: + "def set_pixel(x, y, r, g, b):\n" + + " if x > 7 or x < 0:\n" + + " raise ValueError('X position must be between 0 and 7')\n", + }, + ], + image_list: [], + }, + codeRunTriggered: true, + }, + auth: { user }, + }; + store = mockStore(initialState); + render( + + + , + ); + }); + + test("reports the error at the user's line, not the shim's line", () => { + expect(store.getActions()).toContainEqual( + setError( + "ValueError: X position must be between 0 and 7 on line 2 of main.py", + ), + ); + }); + + test("sets errorDetails pointing to the user's code, not the shim", () => { + expect(store.getActions()).toContainEqual( + setErrorDetails({ + type: "ValueError", + line: 2, + file: "main.py", + description: "X position must be between 0 and 7", + message: + "ValueError: X position must be between 0 and 7 on line 2 of main.py", + }), + ); + }); +}); + describe("When an error has occurred", () => { let mockStore; let store; From bb5895d448f54d52e2b8e89cb951fbc560a9a49b Mon Sep 17 00:00:00 2001 From: Patrick Cherry Date: Thu, 26 Feb 2026 21:55:15 +0000 Subject: [PATCH 05/11] Lint --- .../Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx index fcf6db07e..756d74405 100644 --- a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx +++ b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx @@ -345,7 +345,10 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => { // the traceback as this will be the line in the shim which we don't want // to show to users, so that the error message will instead point to the // line in the user's code which caused the error. - if (err.traceback.length > 1 && err.traceback[0].filename === "./sense_hat.py") { + if ( + err.traceback.length > 1 && + err.traceback[0].filename === "./sense_hat.py" + ) { err.traceback.shift(); } From 6df69268be4bfe4be7ab4992c808af559ec86012 Mon Sep 17 00:00:00 2001 From: Patrick Cherry Date: Thu, 26 Feb 2026 22:05:55 +0000 Subject: [PATCH 06/11] Add a big comment --- .../PythonRunner/SkulptRunner/SkulptRunner.test.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js index ac5f11c7f..c53ad1eb8 100644 --- a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js +++ b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js @@ -227,6 +227,19 @@ describe("When an error occurs", () => { describe("When an error originates in the sense_hat shim", () => { let store; + // This initialState sets up a file `sense_hat.py` which contains code that + // will raise an error if the set_pixel function is called with an x + // value greater than 7 or less than 0. The `main.py` file then calls this + // function with an x value of 255, which will cause the error to be raised. + // + // This file matches the name looked for in the SkulptRunner + // (`./sense_hat.py`), so the SkulptRunner should attribute the error to the + // user's `main.py` file and not the `sense_hat.py` shim file, and set the + // errorDetails accordingly. + // + // This test has to be run this way because the sense hat libray is loaded + // via `fetch()` from a remote URL, which is hard to do in the test + // environment. beforeEach(() => { const middlewares = []; const mockStore = configureStore(middlewares); From fc02afaaf09baa9422ff76493d3d571134de8e41 Mon Sep 17 00:00:00 2001 From: Patrick Cherry Date: Fri, 27 Feb 2026 07:57:38 +0000 Subject: [PATCH 07/11] Don't mutate the traceback array --- .../PythonRunner/SkulptRunner/SkulptRunner.jsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx index 756d74405..5c3f9ff35 100644 --- a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx +++ b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx @@ -340,20 +340,23 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => { .replace(/\[(.*?)\]/, "") .replace(/\.$/, ""); const errorType = err.tp$name || err.constructor.name; + let lineNumber = err.traceback[0].lineno; + let fileName = err.traceback[0].filename; - // If this is an error in the sense_hat.py shim, remove the first line of + // If this is an error in the sense_hat.py shim, use the next entry in // the traceback as this will be the line in the shim which we don't want // to show to users, so that the error message will instead point to the // line in the user's code which caused the error. if ( err.traceback.length > 1 && - err.traceback[0].filename === "./sense_hat.py" + fileName === "./sense_hat.py" && + ["RuntimeError", "ValueError"].includes(errorType) ) { - err.traceback.shift(); + lineNumber = err.traceback[1].lineno; + fileName = err.traceback[1].filename; } - const lineNumber = err.traceback[0].lineno; - const fileName = err.traceback[0].filename.replace(/^\.\//, ""); + fileName = fileName.replace(/^\.\//, ""); if (errorType === "ImportError" && window.crossOriginIsolated) { const articleLink = `https://help.editor.raspberrypi.org/hc/en-us/articles/30841379339924-What-Python-libraries-are-available-in-the-Code-Editor`; From 1d20deb813450545d6ff17759374242a7cfeec62 Mon Sep 17 00:00:00 2001 From: Patrick Cherry Date: Fri, 27 Feb 2026 08:02:38 +0000 Subject: [PATCH 08/11] Tune error handling --- public/shims/sense_hat/sense_hat_blob.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/public/shims/sense_hat/sense_hat_blob.py b/public/shims/sense_hat/sense_hat_blob.py index 7367ec9d3..37d63b73c 100644 --- a/public/shims/sense_hat/sense_hat_blob.py +++ b/public/shims/sense_hat/sense_hat_blob.py @@ -1024,7 +1024,7 @@ def set_pixels(self, pixel_list): """ if not hasattr(pixel_list, '__len__') or len(pixel_list) != 64: - raise ValueError('Pixel lists must be a list of 64 elements') + raise ValueError('Pixel lists must be a sequence of 64 elements') for index, pix in enumerate(pixel_list): if not hasattr(pix, '__len__') or len(pix) != 3: @@ -1086,10 +1086,7 @@ def set_pixel(self, x, y, *args): # Check if pixel responds to len, and if it has 3 elements (r, g, b) if not hasattr(pixel, '__len__') or len(pixel) != 3: - raise ValueError( - 'Pixel arguments must be given as (r, g, b) or r, g, b. ' - 'Did you forget the x, y coordinates?' - ) + raise ValueError('Pixel arguments must be given as (r, g, b) or r, g, b.') if x > 7 or x < 0: raise ValueError('X position must be between 0 and 7') From 5e8a6554aa769e08fb04a9a583454f8e30f166c4 Mon Sep 17 00:00:00 2001 From: Patrick Cherry Date: Fri, 27 Feb 2026 08:19:55 +0000 Subject: [PATCH 09/11] Reduce noise in error messages I've opened a PR in the sense_hat library to match the changes I've made here, and to reduce noise there, I've kept the error messages the same as they were. --- public/shims/sense_hat/sense_hat_blob.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/public/shims/sense_hat/sense_hat_blob.py b/public/shims/sense_hat/sense_hat_blob.py index 37d63b73c..aa0e85f07 100644 --- a/public/shims/sense_hat/sense_hat_blob.py +++ b/public/shims/sense_hat/sense_hat_blob.py @@ -1024,14 +1024,11 @@ def set_pixels(self, pixel_list): """ if not hasattr(pixel_list, '__len__') or len(pixel_list) != 64: - raise ValueError('Pixel lists must be a sequence of 64 elements') + raise ValueError('Pixel lists must have 64 elements') for index, pix in enumerate(pixel_list): if not hasattr(pix, '__len__') or len(pix) != 3: - raise ValueError( - f"Pixel at index {index} is invalid. " - 'Pixels must contain 3 elements: Red, Green and Blue' - ) + raise ValueError('Pixel at index %d is invalid. Pixels must contain 3 elements: Red, Green and Blue' % index) for element in pix: if element > 255 or element < 0: @@ -1086,7 +1083,7 @@ def set_pixel(self, x, y, *args): # Check if pixel responds to len, and if it has 3 elements (r, g, b) if not hasattr(pixel, '__len__') or len(pixel) != 3: - raise ValueError('Pixel arguments must be given as (r, g, b) or r, g, b.') + raise ValueError('Pixel arguments must be given as (r, g, b) or r, g, b') if x > 7 or x < 0: raise ValueError('X position must be between 0 and 7') From 7579ab0b22be2d0fe6ceffc7319bfb49eea40938 Mon Sep 17 00:00:00 2001 From: Patrick Cherry Date: Fri, 27 Feb 2026 08:30:29 +0000 Subject: [PATCH 10/11] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js index c53ad1eb8..5028077f7 100644 --- a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js +++ b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js @@ -237,7 +237,7 @@ describe("When an error originates in the sense_hat shim", () => { // user's `main.py` file and not the `sense_hat.py` shim file, and set the // errorDetails accordingly. // - // This test has to be run this way because the sense hat libray is loaded + // This test has to be run this way because the sense hat library is loaded // via `fetch()` from a remote URL, which is hard to do in the test // environment. beforeEach(() => { From 22066bf15c32dd6c50a051f6cb92cee8a561fa69 Mon Sep 17 00:00:00 2001 From: Patrick Cherry Date: Fri, 27 Feb 2026 08:44:52 +0000 Subject: [PATCH 11/11] Update package version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 86cf599bc..41e5218f8 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@raspberrypifoundation/editor-ui", - "version": "0.34.6", + "version": "0.34.7", "private": true, "dependencies": { "@apollo/client": "^3.7.8",