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", diff --git a/public/shims/sense_hat/sense_hat_blob.py b/public/shims/sense_hat/sense_hat_blob.py index 0ec207f71..aa0e85f07 100644 --- a/public/shims/sense_hat/sense_hat_blob.py +++ b/public/shims/sense_hat/sense_hat_blob.py @@ -1023,11 +1023,11 @@ def set_pixels(self, pixel_list): and 255 """ - if len(pixel_list) != 64: + if not hasattr(pixel_list, '__len__') or len(pixel_list) != 64: raise ValueError('Pixel lists must have 64 elements') for index, pix in enumerate(pixel_list): - if len(pix) != 3: + 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: @@ -1074,16 +1074,16 @@ 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') 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..5c3f9ff35 100644 --- a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx +++ b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx @@ -340,8 +340,23 @@ const SkulptRunner = ({ active, outputPanels = ["text", "visual"] }) => { .replace(/\[(.*?)\]/, "") .replace(/\.$/, ""); const errorType = err.tp$name || err.constructor.name; - const lineNumber = err.traceback[0].lineno; - const fileName = err.traceback[0].filename.replace(/^\.\//, ""); + let lineNumber = err.traceback[0].lineno; + let fileName = err.traceback[0].filename; + + // 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 && + fileName === "./sense_hat.py" && + ["RuntimeError", "ValueError"].includes(errorType) + ) { + lineNumber = err.traceback[1].lineno; + fileName = err.traceback[1].filename; + } + + 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`; diff --git a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js index 94c5c826f..5028077f7 100644 --- a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js +++ b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js @@ -224,6 +224,80 @@ 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 library is loaded + // via `fetch()` from a remote URL, which is hard to do in the test + // environment. + 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;