diff --git a/.yarnrc.yml b/.yarnrc.yml index 579a78a56..7ee9a5b14 100644 --- a/.yarnrc.yml +++ b/.yarnrc.yml @@ -18,4 +18,5 @@ npmMinimalAgeGate: 7d npmPreapprovedPackages: - "@raspberrypifoundation/design-system-react" - "@raspberrypifoundation/design-system-core" - - "@RaspberryPiFoundation/scratch-gui" \ No newline at end of file + - "@RaspberryPiFoundation/scratch-gui" + - "@raspberrypifoundation/python-friendly-error-messages" \ No newline at end of file diff --git a/cypress/e2e/spec-wc-pyodide.cy.js b/cypress/e2e/spec-wc-pyodide.cy.js index 371900470..ac45af0ec 100644 --- a/cypress/e2e/spec-wc-pyodide.cy.js +++ b/cypress/e2e/spec-wc-pyodide.cy.js @@ -1,6 +1,7 @@ import { getEditorShadow, getErrorMessage, + getFriendlyErrorMessage, getFileButtonByName, getProgramInput, getPyodideOutput, @@ -253,3 +254,41 @@ print(text_out) ); }); }); + +describe("When friendly errors enabled with pyodide", () => { + beforeEach(() => { + cy.intercept("GET", "**/python-error-copydecks/**", { + fixture: "copydeck.json", + }).as("copydeck"); + + const params = new URLSearchParams(); + params.set("friendly_errors_enabled", "true"); + + cy.visit({ + url: `${origin}?${params.toString()}`, + headers: { + "Cross-Origin-Opener-Policy": "same-origin", + "Cross-Origin-Embedder-Policy": "require-corp", + }, + }); + cy.window().then((win) => { + Object.defineProperty(win, "crossOriginIsolated", { + value: true, + configurable: true, + }); + }); + cy.wait("@copydeck"); + }); + + it("shows a friendly error message when an error occurs", () => { + runCode("print(kitten)"); + getErrorMessage().should( + "contain", + "NameError: name 'kitten' is not defined on line 1 of main.py", + ); + getFriendlyErrorMessage().should( + "contain", + "This variable doesn't exist here", + ); + }); +}); diff --git a/cypress/e2e/spec-wc-skulpt.cy.js b/cypress/e2e/spec-wc-skulpt.cy.js index 4535072da..d2fe74f18 100644 --- a/cypress/e2e/spec-wc-skulpt.cy.js +++ b/cypress/e2e/spec-wc-skulpt.cy.js @@ -1,5 +1,6 @@ import { getErrorMessage, + getFriendlyErrorMessage, getP5Canvas, getPythonConsoleOutput, getSkulptRunner, @@ -149,3 +150,35 @@ describe("Running the code with skulpt", () => { ); }); }); + +describe("When friendly errors enabled with skulpt", () => { + beforeEach(() => { + cy.intercept("GET", "**/python-error-copydecks/**", { + fixture: "copydeck.json", + }).as("copydeck"); + + const params = new URLSearchParams(); + params.set("friendly_errors_enabled", "true"); + + cy.visit(`${origin}?${params.toString()}`); + cy.window().then((win) => { + Object.defineProperty(win, "crossOriginIsolated", { + value: false, + configurable: true, + }); + }); + cy.wait("@copydeck"); + }); + + it("shows a friendly error message when an error occurs", () => { + runCode("import turtle\nprint(kitten)"); + getErrorMessage().should( + "contain", + "NameError: name 'kitten' is not defined on line 2 of main.py", + ); + getFriendlyErrorMessage().should( + "contain", + "This variable doesn't exist here", + ); + }); +}); diff --git a/cypress/fixtures/copydeck.json b/cypress/fixtures/copydeck.json new file mode 100644 index 000000000..8e385eaa3 --- /dev/null +++ b/cypress/fixtures/copydeck.json @@ -0,0 +1,46 @@ +{ + "meta": { + "language": "en", + "version": 1 + }, + + "errors": { + "NameError": { + "variants": [ + { + "if": { + "not_message": ["is not defined"] + }, + "title": "This variable doesn't exist yet", + "summary": "Your code uses the variable {{name}}, but it hasn't been created yet. Check {{loc}}. If you meant to print the text {{name}}, put it in double quotes.", + "why": "Without speech marks Python treats {{name}} as a variable, and this variable does not exist yet.", + "steps": [ + "If it is meant to be text, put speech marks around {{name}}.", + "If it is meant to be a variable, make it first (for example: {{name}} = 0).", + "Check spelling and capital letters." + ], + "_placeholders": { + "name": "The undefined variable name, e.g. kittens", + "loc": "Where it was used, e.g. line 2 in main.py" + } + }, + { + "if": { + "match_message": ["is not defined"] + }, + "title": "This variable doesn't exist here", + "summary": "{{name}} might be created somewhere else, but you're using it at {{loc}}. If you meant the text {{name}}, put it in double quotes.", + "why": "A variable created in another place might not be available here.", + "steps": [ + "Move the line that makes it to above where you use it.", + "Or set it here just before you use it." + ], + "_placeholders": { + "name": "The variable name used out of scope, e.g. total", + "loc": "Where it was referenced, e.g. line 8 in main.py" + } + } + ] + } + } +} \ No newline at end of file diff --git a/cypress/helpers/editor.js b/cypress/helpers/editor.js index 5d7b9d1fc..ee1853049 100644 --- a/cypress/helpers/editor.js +++ b/cypress/helpers/editor.js @@ -57,7 +57,10 @@ export const getP5Canvas = () => getEditorShadow().find(".p5Canvas"); export const getTurtleOutput = () => getEditorShadow().find("#turtleOutput"); export const getErrorMessage = () => - getEditorShadow().find(".error-message__content"); + getEditorShadow().find(".error-message__python"); + +export const getFriendlyErrorMessage = () => + getEditorShadow().find(".friendly-error-message"); export const getTextOutputTab = () => getPyodideOutput().findByLabelText("Text output"); diff --git a/package.json b/package.json index 1880722eb..5309fca46 100644 --- a/package.json +++ b/package.json @@ -15,7 +15,7 @@ "@hello-pangea/dnd": "^16.2.0", "@juggle/resize-observer": "^3.3.1", "@raspberrypifoundation/design-system-react": "^2.7.0", - "@raspberrypifoundation/python-friendly-error-messages": "0.1.6", + "@raspberrypifoundation/python-friendly-error-messages": "0.7.0", "@react-three/drei": "9.114.3", "@react-three/fiber": "^8.0.13", "@reduxjs/toolkit": "^1.6.2", diff --git a/src/PyodideWorker.js b/src/PyodideWorker.js index 561b5d995..42d8fe27a 100644 --- a/src/PyodideWorker.js +++ b/src/PyodideWorker.js @@ -534,7 +534,7 @@ const PyodideWorker = () => { const line = match ? parseInt(match[2], 10) : ""; - return { file, line, mistake, type, info }; + return { file, line, mistake, type, info, rawTraceback: error.message }; }; initialisePyodide(); diff --git a/src/assets/error_file.svg b/src/assets/error_file.svg new file mode 100644 index 000000000..ee9ea4c60 --- /dev/null +++ b/src/assets/error_file.svg @@ -0,0 +1,3 @@ + + + diff --git a/src/assets/icons/cancel_FILL.svg b/src/assets/icons/cancel_FILL.svg new file mode 100644 index 000000000..5ef0d2fec --- /dev/null +++ b/src/assets/icons/cancel_FILL.svg @@ -0,0 +1,3 @@ + + + diff --git a/src/assets/stylesheets/EditorPanel.scss b/src/assets/stylesheets/EditorPanel.scss index f29850738..2a147f49a 100644 --- a/src/assets/stylesheets/EditorPanel.scss +++ b/src/assets/stylesheets/EditorPanel.scss @@ -1,5 +1,6 @@ -@forward '@raspberrypifoundation/design-system-core/scss/properties/spacing'; -@use '@raspberrypifoundation/design-system-core/scss/mixins/typography' as typography; +@forward "@raspberrypifoundation/design-system-core/scss/properties/spacing"; +@use "@raspberrypifoundation/design-system-core/scss/mixins/typography" as + typography; // Scrollbar-width is needed from scrollbar to show in Chrome .editor-wrapper { @@ -31,7 +32,6 @@ overscroll-behavior-x: none; font-family: var(--wc-font-family-monospace); - .cm-content { flex: 1; padding-block-start: var(--space-0-5); @@ -60,5 +60,3 @@ display: none; } } - - diff --git a/src/assets/stylesheets/ErrorMessage.scss b/src/assets/stylesheets/ErrorMessage.scss index 33a3a0af7..232711e00 100644 --- a/src/assets/stylesheets/ErrorMessage.scss +++ b/src/assets/stylesheets/ErrorMessage.scss @@ -1,34 +1,34 @@ -@use "./rpf_design_system/font-size" as *; -@use "./rpf_design_system/spacing" as *; +@use "@raspberrypifoundation/design-system-core/scss/mixins/typography" as + typography; .error-message { - color: #7e0305; - background-color: #fde2e1; - padding: $space-0-75 $space-1-25; + color: var(--rpf-red-900); + background-color: var(--rpf-red-100); + border-block-start: 1px solid var(--editor-color-outline); + padding: var(--space-1); overflow-y: auto; + scrollbar-width: thin; + max-block-size: 30%; - &__content { + &__error { padding: 0; margin: 0; white-space: pre-wrap; overflow-wrap: break-word; + font-weight: bold; } - &--medium { - @include font-size-1-5(regular); - } - - &--large { - @include font-size-2(regular); - } + &__python { + display: flex; + gap: var(--space-1); + @include typography.style-1; - // Only displaying title and summary of friendlyError - &__friendly { - .pfem__why, - .pfem__steps, - .pfem__patch, - .pfem__details { - display: none; + &--medium { + @include typography.style-1; + } + + &--large { + @include typography.style-2; } } } diff --git a/src/assets/stylesheets/FriendlyErrorMessage.scss b/src/assets/stylesheets/FriendlyErrorMessage.scss new file mode 100644 index 000000000..3f6eef237 --- /dev/null +++ b/src/assets/stylesheets/FriendlyErrorMessage.scss @@ -0,0 +1,58 @@ +.friendly-error-message { + background-color: var(--rpf-white); + padding: var(--space-1) var(--space-1-5); + color: var(--rpf-text-primary); + border-radius: var(--space-1); + margin-block-start: var(--space-1); + + &__content { + line-height: 1.75rem; + + .pfem__title { + font-weight: 600; + display: inline; + } + + .pfem__summary { + display: inline; + + &::before { + content: " - "; + } + } + + .pfem__var, .pfem__code, .pfem__file, .pfem__line { + font-family: var(--wc-font-family-monospace); + border-radius: 0.25rem; + padding: 0.2rem var(--space-0-5); + } + + .pfem__var { + background-color: var(--rpf-green-100); + } + + .pfem__code { + background-color: var(--rpf-grey-100); + } + + .pfem__file { + white-space: nowrap; + background-color: var(--rpf-yellow-100); + + &::before { + content: ""; + display: inline-block; + inline-size: 1em; + block-size: 1em; + background: url("../error_file.svg") no-repeat center; + background-size: contain; + margin-inline-end: 0.12rem; + vertical-align: middle; + } + } + + .pfem__line { + background-color: var(--rpf-blue-100); + } + } +} diff --git a/src/assets/stylesheets/InternalStyles.scss b/src/assets/stylesheets/InternalStyles.scss index 01f136b70..9f36bf398 100644 --- a/src/assets/stylesheets/InternalStyles.scss +++ b/src/assets/stylesheets/InternalStyles.scss @@ -43,6 +43,7 @@ @use "./EditorFinalStep.scss" as *; @use "./Loader.scss" as *; @use "./LoadFailed.scss" as *; +@use "./FriendlyErrorMessage" as *; @use "./settings/fonts" as fonts; @@ -158,7 +159,9 @@ button:focus-visible { // Primary button --rpf-button-primary-background-color: var(--editor-color-theme); --rpf-button-primary-background-color-focus: var(--editor-color-theme); - --rpf-button-primary-background-color-hover: var(--editor-color-theme-secondary); + --rpf-button-primary-background-color-hover: var( + --editor-color-theme-secondary + ); --rpf-button-primary-background-color-active: var(--rpf-navy-600); --rpf-button-primary-background-color-disabled: var(--rpf-navy-200); --rpf-button-primary-color-disabled: var(--rpf-grey-600); @@ -167,10 +170,16 @@ button:focus-visible { // Secondary button --rpf-button-secondary-background-color: var(--editor-color-theme); --rpf-button-secondary-background-color-focus: var(--rpf-brand-raspberry); - --rpf-button-secondary-background-color-hover: var(--editor-color-theme-tertiary); + --rpf-button-secondary-background-color-hover: var( + --editor-color-theme-tertiary + ); --rpf-button-secondary-border-color: var(--editor-color-theme); - --rpf-button-secondary-border-color-hover: var(--editor-color-theme-secondary); - --rpf-button-secondary-background-color-active: var(--editor-color-theme-secondary); + --rpf-button-secondary-border-color-hover: var( + --editor-color-theme-secondary + ); + --rpf-button-secondary-background-color-active: var( + --editor-color-theme-secondary + ); --rpf-button-secondary-background-color-disabled: var(--rpf-grey-50); --rpf-button-secondary-text-color: var(--editor-color-theme); @@ -230,7 +239,9 @@ button:focus-visible { --rpf-button-secondary-text-color: var(--rpf-white); --rpf-button-secondary-background-color: var(--editor-color-layer-2); --rpf-button-secondary-background-color-active: var(--rpf-navy-200); - --rpf-button-secondary-color-disabled-background: var(--editor-color-layer-3); + --rpf-button-secondary-color-disabled-background: var( + --editor-color-layer-3 + ); --rpf-button-secondary-background-color-hover: var(--editor-color-outline); --rpf-button-secondary-border-color: var(--editor-color-theme); --rpf-button-secondary-border-color-hover: var(--editor-color-theme); @@ -240,9 +251,17 @@ button:focus-visible { // Tertiary button --rpf-button-tertiary-text-color-hover: var(--rpf-grey-200); --rpf-button-tertiary-danger-text-color: var(--rpf-red-600); - --rpf-button-tertiary-danger-background-color-hover: rgba(255, 255, 255, 0.1); - --rpf-button-tertiary-danger-background-color-active: rgba(255, 255, 255, 0.15); + --rpf-button-tertiary-danger-background-color-hover: rgba( + 255, + 255, + 255, + 0.1 + ); + --rpf-button-tertiary-danger-background-color-active: rgba( + 255, + 255, + 255, + 0.15 + ); } } - - diff --git a/src/components/Editor/EditorInput/EditorInput.jsx b/src/components/Editor/EditorInput/EditorInput.jsx index ece8db7ff..98de64507 100644 --- a/src/components/Editor/EditorInput/EditorInput.jsx +++ b/src/components/Editor/EditorInput/EditorInput.jsx @@ -15,6 +15,7 @@ import EditorPanel from "../EditorPanel/EditorPanel"; import DraggableTab from "../DraggableTabs/DraggableTab"; import DroppableTabList from "../DraggableTabs/DroppableTabList"; import RunBar from "../../RunButton/RunBar"; +import ErrorMessage from "../ErrorMessage/ErrorMessage"; import "../../../assets/stylesheets/EditorInput.scss"; import RunnerControls from "../../RunButton/RunnerControls"; @@ -186,6 +187,7 @@ const EditorInput = () => { /> ))} + {isMobile ? null : } ))} diff --git a/src/components/Editor/EditorInput/EditorInput.test.js b/src/components/Editor/EditorInput/EditorInput.test.js index 84b2426aa..30d2b4ab5 100644 --- a/src/components/Editor/EditorInput/EditorInput.test.js +++ b/src/components/Editor/EditorInput/EditorInput.test.js @@ -188,3 +188,15 @@ describe("When read only", () => { expect(screen.queryByText("editorPanel.viewOnly")).toBeInTheDocument(); }); }); + +describe("When there is an error", () => { + beforeEach(() => { + renderEditorInput({ + editor: { ...initialState.editor, error: "An error occurred" }, + }); + }); + + test("The error message is displayed", () => { + expect(screen.getByText("An error occurred")).toBeInTheDocument(); + }); +}); diff --git a/src/components/Editor/ErrorMessage/ErrorMessage.jsx b/src/components/Editor/ErrorMessage/ErrorMessage.jsx index fe1516cec..1030f6e86 100644 --- a/src/components/Editor/ErrorMessage/ErrorMessage.jsx +++ b/src/components/Editor/ErrorMessage/ErrorMessage.jsx @@ -3,30 +3,25 @@ import "../../../assets/stylesheets/ErrorMessage.scss"; import { useSelector } from "react-redux"; import DOMPurify from "dompurify"; import { SettingsContext } from "../../../utils/settings"; +import CancelFillIcon from "../../../assets/icons/cancel_FILL.svg"; +import FriendlyErrorMessage from "../FriendlyErrorMessage/FriendlyErrorMessage"; const ErrorMessage = () => { const error = useSelector((state) => state.editor.error); - const friendlyError = useSelector((state) => state.editor.friendlyError); const settings = useContext(SettingsContext); - const errorHtml = DOMPurify.sanitize(error); - - const friendlyErrorHtml = friendlyError?.html - ? DOMPurify.sanitize(friendlyError.html) - : null; - return error ? ( -
-
-      {friendlyErrorHtml && (
-        
+
+ +
-      )}
+      
+
) : null; }; diff --git a/src/components/Editor/ErrorMessage/ErrorMessage.test.js b/src/components/Editor/ErrorMessage/ErrorMessage.test.js index f9fe32a43..439b70155 100644 --- a/src/components/Editor/ErrorMessage/ErrorMessage.test.js +++ b/src/components/Editor/ErrorMessage/ErrorMessage.test.js @@ -33,7 +33,7 @@ describe("When error is set", () => { test("Font size class is set correctly", () => { const errorMessage = screen.queryByText("Oops").parentElement; - expect(errorMessage).toHaveClass("error-message--myFontSize"); + expect(errorMessage).toHaveClass("error-message__python--myFontSize"); }); test("Does not display friendly error elements", () => { diff --git a/src/components/Editor/FriendlyErrorMessage/FriendlyErrorMessage.jsx b/src/components/Editor/FriendlyErrorMessage/FriendlyErrorMessage.jsx new file mode 100644 index 000000000..3c074917b --- /dev/null +++ b/src/components/Editor/FriendlyErrorMessage/FriendlyErrorMessage.jsx @@ -0,0 +1,23 @@ +import React from "react"; +import { useSelector } from "react-redux"; +import "../../../assets/stylesheets/FriendlyErrorMessage.scss"; +import DOMPurify from "dompurify"; + +const FriendlyErrorMessage = () => { + const friendlyError = useSelector((state) => state.editor.friendlyError); + + const friendlyErrorHtml = friendlyError?.html + ? DOMPurify.sanitize(friendlyError.html) + : null; + + return friendlyErrorHtml ? ( +
+
+
+ ) : null; +}; + +export default FriendlyErrorMessage; diff --git a/src/components/Editor/FriendlyErrorMessage/FriendlyErrorMessage.test.js b/src/components/Editor/FriendlyErrorMessage/FriendlyErrorMessage.test.js new file mode 100644 index 000000000..745e82374 --- /dev/null +++ b/src/components/Editor/FriendlyErrorMessage/FriendlyErrorMessage.test.js @@ -0,0 +1,85 @@ +import configureStore from "redux-mock-store"; +import { Provider } from "react-redux"; +import { render, screen } from "@testing-library/react"; +import FriendlyErrorMessage from "./FriendlyErrorMessage"; + +const middlewares = []; +const mockStore = configureStore(middlewares); + +describe("When friendlyError is set", () => { + beforeEach(() => { + const initialState = { + editor: { + error: "An error occurred", + friendlyError: { + html: '
Friendly Error Title
This is a more user-friendly explanation of the error.
A variable created in another place might not be available here.
  • Move the line that makes it to above where you use it.
  • Or set it here just before you use it.
kettle = 0
Original error
An error occurred
', + }, + }, + }; + const store = mockStore(initialState); + render( + + + , + ); + }); + + test("Friendly error title and summary display", () => { + expect(screen.getByText("Friendly Error Title")).toBeInTheDocument(); + expect( + screen.getByText( + "This is a more user-friendly explanation of the error.", + ), + ).toBeInTheDocument(); + }); + + it("strips unsafe scripts", () => { + const initialState = { + editor: { + error: "An error occurred", + friendlyError: { + html: ` +
Friendly Error Title
+ + + Bad link + `, + }, + }, + }; + const store = mockStore(initialState); + const { container } = render( + + + , + ); + + expect(container.querySelector("script")).not.toBeInTheDocument(); + expect(container.querySelector("[onerror]")).not.toBeInTheDocument(); + expect( + container.querySelector('a[href^="javascript:"]'), + ).not.toBeInTheDocument(); + }); +}); + +describe("When friendlyError is not set", () => { + beforeEach(() => { + const initialState = { + editor: { + error: "Oops", + }, + }; + const store = mockStore(initialState); + render( + + + , + ); + }); + + test("Does not display friendly error elements", () => { + expect( + document.querySelector(".friendly-error-message"), + ).not.toBeInTheDocument(); + }); +}); diff --git a/src/components/Editor/Runners/PythonRunner/PyodideRunner/PyodideRunner.jsx b/src/components/Editor/Runners/PythonRunner/PyodideRunner/PyodideRunner.jsx index ebdcf013c..4a75f3e89 100644 --- a/src/components/Editor/Runners/PythonRunner/PyodideRunner/PyodideRunner.jsx +++ b/src/components/Editor/Runners/PythonRunner/PyodideRunner/PyodideRunner.jsx @@ -15,7 +15,7 @@ import { import { loadCopydeckFor, registerAdapter, - pyodideAdapter, + cpythonAdapter, friendlyExplain, } from "@raspberrypifoundation/python-friendly-error-messages"; import { Tab, Tabs, TabList, TabPanel } from "react-tabs"; @@ -49,10 +49,15 @@ const PyodideRunner = ({ useEffect(() => { if (friendlyErrorsEnabled) { - loadCopydeckFor(navigator.language, { - base: `${process.env.PUBLIC_URL}/python-error-copydecks/`, - }); - registerAdapter("pyodide", pyodideAdapter); + try { + loadCopydeckFor(navigator.language || "en", { + base: `${process.env.PUBLIC_URL}/python-error-copydecks/`, + }); + registerAdapter("pyodide", cpythonAdapter); + } catch { + console.error("Could not load friendly error copydeck"); + dispatch(setFriendlyError(null)); + } } }, [friendlyErrorsEnabled]); @@ -81,6 +86,7 @@ const PyodideRunner = ({ const userId = user?.profile?.user; const isSplitView = useSelector((s) => s.editor.isSplitView); const isEmbedded = useSelector((s) => s.editor.isEmbedded); + const isOutputOnly = useSelector((s) => s.editor.isOutputOnly); const reactAppApiEndpoint = useSelector((s) => s.editor.reactAppApiEndpoint); const codeRunTriggered = useSelector((s) => s.editor.codeRunTriggered); const codeRunStopped = useSelector((s) => s.editor.codeRunStopped); @@ -121,6 +127,7 @@ const PyodideRunner = ({ data.mistake, data.type, data.info, + data.rawTraceback, ); break; case "handleFileWrite": @@ -210,7 +217,7 @@ const PyodideRunner = ({ node.scrollTop = node.scrollHeight; }; - const handleError = (file, line, mistake, type, info) => { + const handleError = (file, line, mistake, type, info, rawTraceback) => { let errorMessage; if (type === "KeyboardInterrupt") { @@ -233,14 +240,20 @@ const PyodideRunner = ({ projectCode?.find((c) => c.name === "main" && c.extension === "py") ?.content ?? ""; - const friendlyError = friendlyExplain({ - error: errorMessage, - code: inputCode, - runtime: "pyodide", - }); + try { + const friendlyError = friendlyExplain({ + error: rawTraceback, + file: file, + code: inputCode, + runtime: "pyodide", + sections: ["title", "summary"], + }); - if (friendlyError?.html) { - dispatch(setFriendlyError({ html: friendlyError.html })); + if (friendlyError?.html) { + dispatch(setFriendlyError({ html: friendlyError.html })); + } + } catch { + console.error("Could not parse friendly error"); } } } @@ -460,7 +473,7 @@ const PyodideRunner = ({ )}
- + {isOutputOnly && }
}
             {!isEmbedded && isMobile && }
           
- + {isOutputOnly && } {hasVisual && ( diff --git a/src/components/Editor/Runners/PythonRunner/PyodideRunner/PyodideRunner.test.js b/src/components/Editor/Runners/PythonRunner/PyodideRunner/PyodideRunner.test.js index 582abdcd4..65cf479b0 100644 --- a/src/components/Editor/Runners/PythonRunner/PyodideRunner/PyodideRunner.test.js +++ b/src/components/Editor/Runners/PythonRunner/PyodideRunner/PyodideRunner.test.js @@ -21,6 +21,7 @@ import { openFile, setFocussedFileIndex, setFriendlyError, + setIsOutputOnly, } from "../../../../../redux/EditorSlice.js"; import store from "../../../../../app/store"; @@ -41,10 +42,6 @@ const project = { window.crossOriginIsolated = true; process.env.PUBLIC_URL = "."; -const friendlyErrorHtml = - '
Friendly error title
' + - '
A friendly summary of the error
'; - const updateRunner = ({ project = {}, codeRunTriggered = false }) => { act(() => { if (project) { @@ -431,13 +428,40 @@ describe("When an error is received", () => { file: "main.py", type: "SyntaxError", info: "something's wrong", + mistake: "if score = 10:\n ^^^^^^^^^^", }); }); test("it dispatches action to set the error with correct message", () => { expect(dispatchSpy).toHaveBeenCalledWith({ type: "editor/setError", - payload: "SyntaxError: something's wrong on line 2 of main.py", + payload: + "SyntaxError: something's wrong on line 2 of main.py:\nif score = 10:\n ^^^^^^^^^^", + }); + }); + + describe("When output-only is enabled", () => { + beforeEach(() => { + act(() => { + store.dispatch(setIsOutputOnly(true)); + }); + + const worker = PyodideWorker.getLastInstance(); + worker.postMessageFromWorker({ + method: "handleError", + line: 2, + file: "main.py", + type: "SyntaxError", + info: "something's wrong", + }); + }); + + test("it displays the error message", () => { + expect( + screen.queryByText( + "SyntaxError: something's wrong on line 2 of main.py", + ), + ).toBeInTheDocument(); }); }); @@ -446,6 +470,19 @@ describe("When an error is received", () => { let registerAdapter; let friendlyExplain; + const friendlyErrorHtml = + '
Friendly error title
' + + '
A friendly summary of the error
'; + + const errorWorkerMessage = { + method: "handleError", + line: 2, + file: "main.py", + type: "SyntaxError", + info: "something's wrong", + mistake: "if score = 10:\n ^^^^^^^^^^", + }; + beforeEach(() => { ({ // Using the global mock in setupTests.js to track calls to these functions @@ -453,41 +490,108 @@ describe("When an error is received", () => { registerAdapter, friendlyExplain, } = require("@raspberrypifoundation/python-friendly-error-messages")); + }); - friendlyExplain.mockReturnValue({ - html: friendlyErrorHtml, + describe("on mount", () => { + beforeEach(() => { + render( + + + , + ); }); - render( - - - , - ); + test("loadCopydeckFor is called", () => { + expect(loadCopydeckFor).toHaveBeenCalled(); + }); - const worker = PyodideWorker.getLastInstance(); - worker.postMessageFromWorker({ - method: "handleError", - line: 2, - file: "main.py", - type: "SyntaxError", - info: "something's wrong", + test("registerAdapter is called for pyodide", () => { + expect(registerAdapter).toHaveBeenCalledWith("pyodide", {}); }); }); - test("loadCopydeckFor is called", () => { - expect(loadCopydeckFor).toHaveBeenCalled(); + describe("when an error occurs and friendlyExplain returns HTML", () => { + beforeEach(() => { + friendlyExplain.mockReturnValue({ html: friendlyErrorHtml }); + + render( + + + , + ); + + PyodideWorker.getLastInstance().postMessageFromWorker( + errorWorkerMessage, + ); + }); + + test("dispatches setFriendlyError", () => { + expect(dispatchSpy).toHaveBeenCalledWith( + setFriendlyError({ html: friendlyErrorHtml }), + ); + }); }); - test("registerAdapter is called for pyodide", () => { - expect(registerAdapter).toHaveBeenCalledWith("pyodide", {}); + describe("when an error occurs and friendlyExplain returns no match", () => { + beforeEach(() => { + friendlyExplain.mockReturnValue(null); + + render( + + + , + ); + + PyodideWorker.getLastInstance().postMessageFromWorker( + errorWorkerMessage, + ); + }); + + test("does not dispatch setFriendlyError", () => { + expect(dispatchSpy).not.toHaveBeenCalledWith( + expect.objectContaining({ type: "editor/setFriendlyError" }), + ); + }); + + test("still dispatches setError with the original message", () => { + expect(dispatchSpy).toHaveBeenCalledWith( + setError( + "SyntaxError: something's wrong on line 2 of main.py:\nif score = 10:\n ^^^^^^^^^^", + ), + ); + }); }); - test("dispatches setFriendlyError", () => { - expect(dispatchSpy).toHaveBeenCalledWith( - setFriendlyError({ - html: friendlyErrorHtml, - }), - ); + describe("when an error occurs and friendlyExplain throws", () => { + beforeEach(() => { + friendlyExplain.mockImplementation(() => { + throw new Error("Could not parse friendly error"); + }); + + render( + + + , + ); + + PyodideWorker.getLastInstance().postMessageFromWorker( + errorWorkerMessage, + ); + }); + + test("does not dispatch setFriendlyError", () => { + expect(dispatchSpy).not.toHaveBeenCalledWith( + expect.objectContaining({ type: "editor/setFriendlyError" }), + ); + }); + + test("still dispatches setError with the original message", () => { + expect(dispatchSpy).toHaveBeenCalledWith( + setError( + "SyntaxError: something's wrong on line 2 of main.py:\nif score = 10:\n ^^^^^^^^^^", + ), + ); + }); }); }); }); diff --git a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx index 19b78d0e6..8fb832960 100644 --- a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx +++ b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.jsx @@ -20,10 +20,9 @@ import { import { loadCopydeckFor, registerAdapter, - skulptAdapter, + cpythonAdapter, friendlyExplain, } from "@raspberrypifoundation/python-friendly-error-messages"; -import ErrorMessage from "../../../ErrorMessage/ErrorMessage"; import ApiCallHandler from "../../../../../utils/apiCallHandler"; import store from "../../../../../redux/stores/WebComponentStore"; import VisualOutputPane from "../VisualOutputPane"; @@ -90,7 +89,6 @@ const SkulptRunner = ({ const user = useSelector((state) => state.auth.user); const isSplitView = useSelector((state) => state.editor.isSplitView); const isEmbedded = useSelector((state) => state.editor.isEmbedded); - const isOutputOnly = useSelector((state) => state.editor.isOutputOnly); const codeRunTriggered = useSelector( (state) => state.editor.codeRunTriggered, ); @@ -176,10 +174,15 @@ const SkulptRunner = ({ useEffect(() => { if (friendlyErrorsEnabled) { - loadCopydeckFor(navigator.language, { - base: `${process.env.PUBLIC_URL}/python-error-copydecks/`, - }); - registerAdapter("skulpt", skulptAdapter); + try { + loadCopydeckFor(navigator.language || "en", { + base: `${process.env.PUBLIC_URL}/python-error-copydecks/`, + }); + registerAdapter("skulpt", cpythonAdapter); + } catch { + console.error("Could not load friendly error copydeck"); + dispatch(setFriendlyError(null)); + } } }, [friendlyErrorsEnabled]); @@ -449,14 +452,20 @@ const SkulptRunner = ({ const inputCode = projectCode?.find((c) => c.name === "main" && c.extension === "py") ?.content ?? ""; - const friendlyError = friendlyExplain({ - error: errorMessage, - code: inputCode, - runtime: "skulpt", - }); - if (friendlyError?.html) { - dispatch(setFriendlyError({ html: friendlyError.html })); + try { + const friendlyError = friendlyExplain({ + error: errorMessage, + code: inputCode, + runtime: "skulpt", + sections: ["title", "summary"], + }); + + if (friendlyError?.html) { + dispatch(setFriendlyError({ html: friendlyError.html })); + } + } catch { + console.error("Could not parse friendly error"); } } } @@ -630,7 +639,6 @@ const SkulptRunner = ({ )} -
}
             {!isEmbedded && isMobile && }
           
-          {!isOutputOnly && }
           
             
           
diff --git a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js
index f5f5bf5b1..5e13d5121 100644
--- a/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js
+++ b/src/components/Editor/Runners/PythonRunner/SkulptRunner/SkulptRunner.test.js
@@ -346,61 +346,6 @@ describe("When an error originates in the sense_hat shim", () => {
   });
 });
 
-describe("When an error has occurred", () => {
-  let mockStore;
-  let store;
-  let initialState;
-
-  beforeEach(() => {
-    const middlewares = [];
-    mockStore = configureStore(middlewares);
-    initialState = {
-      editor: {
-        project: {
-          components: [
-            {
-              name: "main",
-              extension: "py",
-              content: "boom!",
-            },
-          ],
-          image_list: [],
-        },
-        error: "SyntaxError: bad token T_OP on line 1 of main.py",
-      },
-      auth: {
-        user,
-      },
-    };
-  });
-
-  test("Displays error message", () => {
-    store = mockStore(initialState);
-    render(
-      
-        
-      ,
-    );
-
-    expect(
-      screen.getByText("SyntaxError: bad token T_OP on line 1 of main.py"),
-    ).toBeVisible();
-  });
-
-  test("Does not display error message when isOutputOnly state is true", () => {
-    initialState.editor.isOutputOnly = true;
-    store = mockStore(initialState);
-    render(
-      
-        
-      ,
-    );
-    expect(
-      screen.queryByText("SyntaxError: bad token T_OP on line 1 of main.py"),
-    ).not.toBeInTheDocument();
-  });
-});
-
 describe("When there is an import error and the site is cross-origin isolated", () => {
   let store;
   beforeEach(() => {
diff --git a/src/components/Mobile/MobileProject/MobileProject.jsx b/src/components/Mobile/MobileProject/MobileProject.jsx
index 959dbfcd5..72193d9a4 100644
--- a/src/components/Mobile/MobileProject/MobileProject.jsx
+++ b/src/components/Mobile/MobileProject/MobileProject.jsx
@@ -25,6 +25,7 @@ const MobileProject = ({
   const codeRunTriggered = useSelector(
     (state) => state.editor.codeRunTriggered,
   );
+  const error = useSelector((state) => state.editor.error);
   const includesInstructions = sidebarOptions.includes("instructions");
 
   const [selectedTab, setSelectedTab] = useState(1);
@@ -41,6 +42,12 @@ const MobileProject = ({
     }
   }, [codeRunTriggered, sidebarShowing, withSidebar]);
 
+  useEffect(() => {
+    if (!codeRunTriggered && error && projectType === "python") {
+      setSelectedTab(withSidebar ? 1 : 0);
+    }
+  }, [error, codeRunTriggered, projectType, withSidebar]);
+
   return (
     
{ expect(screen.queryByTitle("sidebar.expand")).not.toBeInTheDocument(); }); }); + +describe("When there is an error after code has been run", () => { + beforeEach(() => { + const initialState = { + editor: { + project: { + project_type: "python", + components: [ + { + name: "main", + extension: "py", + content: "print('hello')", + }, + ], + image_list: [], + user_id: user.profile.user, + }, + codeRunTriggered: false, + openFiles: [["main.py"]], + focussedFileIndices: [0], + error: "An error occurred", + }, + auth: { + user: user, + }, + }; + const store = mockStore(initialState); + render( + + + , + ); + }); + + test("The input tab is selected", () => { + const inputTab = screen.getByText("mobile.code").parentElement; + expect(inputTab).toHaveClass("react-tabs__tab--selected"); + }); + + test("The error message is displayed", () => { + expect(screen.getByText("An error occurred")).toBeInTheDocument(); + }); +}); diff --git a/src/utils/setupTests.js b/src/utils/setupTests.js index c17846e7e..6ccf273b1 100644 --- a/src/utils/setupTests.js +++ b/src/utils/setupTests.js @@ -55,8 +55,7 @@ jest.mock("../assets/markdown/demoInstructions.md", () => { jest.mock("@raspberrypifoundation/python-friendly-error-messages", () => ({ loadCopydeckFor: jest.fn(), registerAdapter: jest.fn(), - pyodideAdapter: {}, - skulptAdapter: {}, + cpythonAdapter: {}, friendlyExplain: jest.fn(), })); diff --git a/src/web-component.html b/src/web-component.html index 0611083eb..82846220f 100644 --- a/src/web-component.html +++ b/src/web-component.html @@ -1,4 +1,4 @@ - +