Check args more thoroghly in sense_hat.py and mangle traceback for sense_hat.py errors

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",

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')

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`;

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(
+      <Provider store={store}>
+        <SkulptRunner active={true} />
+      </Provider>,
+    );
+  });
+
+  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;