Skip to content

improve sketch error messages in the editor console #4092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Nixxx19 wants to merge 16 commits into
base: develop-codemirror-v6
Choose a base branch
from
+327 −97
Open

improve sketch error messages in the editor console #4092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
9fbda46
forward jshint syntax errors to the sketch console
Nixxx19 Apr 22, 2026
4279ff3
use stacktrace.fromerror for runtime error stacks
Nixxx19 Apr 22, 2026
7194832
send structured error meta alongside log strings
Nixxx19 Apr 22, 2026
f9a35f3
fix jshint noise and inline script line mapping
Nixxx19 Apr 22, 2026
36aa31e
suppress broken inline script to avoid duplicate parse error
Nixxx19 Apr 22, 2026
50dd5e4
dedupe jshint errors by file line and reason
Nixxx19 Apr 22, 2026
278b9c9
emit friendly p5 fes style message for jshint syntax errors
Nixxx19 Apr 22, 2026
3076ec8
Revert "emit friendly p5 fes style message for jshint syntax errors"
Nixxx19 Apr 22, 2026
6bbb338
render jshint errors inline with dynamic friendly hints
Nixxx19 Apr 22, 2026
20737cd
add mdn reference link to jshint friendly hint
Nixxx19 Apr 22, 2026
e808008
put mdn link on its own line in the friendly hint
Nixxx19 Apr 22, 2026
2db472d
emit mdn reference link only once per batch of jshint errors
Nixxx19 Apr 22, 2026
81aea2a
consolidate jshint friendly hints into single p5 says block
Nixxx19 Apr 22, 2026
70822d7
detect missing equals in let const var declarations
Nixxx19 Apr 22, 2026
e010707
decorate every error line in the selected file
Nixxx19 Apr 22, 2026
2e54aa0
fix html line offset mismatch and guard decoration range
Nixxx19 Apr 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions client/modules/IDE/components/Editor/consoleErrorDecoration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ const ERROR_DECORATION = Decoration.line({

// Add an error decoration to a specific line number
export function addErrorDecoration(view, lineNumber) {
if (!view || !lineNumber) return;
const totalLines = view.state.doc.lines;
if (lineNumber < 1 || lineNumber > totalLines) return;
const docLineNumber = view.state.doc.line(lineNumber);
view.dispatch({
effects: ADD_ERROR_DECORATION.of([
Expand All @@ -46,6 +49,7 @@ export function addErrorDecoration(view, lineNumber) {

// Remove all error decorations
export function removeErrorDecorations(view) {
if (!view) return;
view.dispatch({
effects: FILTER_ERROR_DECORATION.of(() => false)
});
Expand Down
58 changes: 39 additions & 19 deletions client/modules/IDE/components/Editor/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,27 +148,47 @@ function Editor({
useEffect(() => {
const consoleErrors = consoleEvents.filter((e) => e.method === 'error');

if (consoleErrors.length > 0) {
const firstError = consoleErrors[0];
const errorObj = { stack: firstError.data[0].toString() };
Comment on lines -152 to -153
Copy link
Copy Markdown

@skyash-dev skyash-dev May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using only consoleErrors[0] actually seems reasonable to me from a UX perspective.
reflects an implicit assumption that:

  • the first runtime error is usually the most actionable
  • one clear navigation target is often better than many competing ones
  • stability/readability matters more than exhaustiveness

maybe.

  • keep console output/navigation focused on the primary error only
  • but surface all resolved runtime errors through error decorations tooltips

that could reduce console noise while still making secondary issues discoverable contextually near the affected lines.

something similar is already done in editors/platforms like openprocessing, where the primary runtime issue is emphasized while additional diagnostics remain attached to the code locations themselves.

Copy link
Copy Markdown
Member Author

@Nixxx19 Nixxx19 May 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • the decorate-all part is already in the pr, targetFileId = pairs[0].file.id for navigation/focus and every error in that file gets addErrorDecoration so all the lines get a red gutter
  • where we differ is the console output, the pr surfaces all errors as text instead of hiding secondary ones behind hover tooltips. kept it that way because the editor audience is mostly beginners and hover-only info gets missed easily
  • tooltip widgets on decorations would also be new infra, probably better as a follow up, happy to open an issue to track it if you think its worth doing

Copy link
Copy Markdown

@skyash-dev skyash-dev May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo showing syntax errors in the line itself will be more intuitive for beginners.

StackTrace.fromError(errorObj).then((stackLines) => {
expandConsole();
const line = stackLines.find(
(l) => l.fileName && l.fileName.startsWith('/')
removeErrorDecorations(codemirrorView.current);

if (consoleErrors.length === 0) return;
Comment on lines +151 to +153
Copy link
Copy Markdown

@skyash-dev skyash-dev May 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think the decoration clearing may be better scoped to the empty-error case:

if (consoleErrors.length === 0) {
  removeErrorDecorations(codemirrorView.current);
  return;
}

readable + more stable

Copy link
Copy Markdown
Member Author

@Nixxx19 Nixxx19 May 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i thought about scoping it but kept the unconditional clear on purpose. if a user has an error on line 5, fixes it, then introduces a new error on line 10 on the next run, skipping the clear leaves the old gutter on line 5 even though its already fixed. clear-then-add guarantees the gutters always reflect just the latest run, happy to revisit if you can think of a case where the always-clear causes flicker or perf issues

Copy link
Copy Markdown

@skyash-dev skyash-dev May 27, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry i cant get it, can you clearly state the steps to reproduce what you are saying if we move removeErrorDecoration to if (consoleErrors.length === 0) {...}


const resolveFrameFromError = (errorEntry) => {
const metaStack = errorEntry.meta && errorEntry.meta.stack;
if (Array.isArray(metaStack) && metaStack.length > 0) {
return Promise.resolve(
metaStack.find((f) => f.fileName && f.fileName.startsWith('/')) ||
metaStack[0]
);
if (!line) return;
const fileNameArray = line.fileName.split('/');
const fileName = fileNameArray.slice(-1)[0];
const filePath = fileNameArray.slice(0, -1).join('/');
const fileWithError = files.find(
(f) => f.name === fileName && f.filePath === filePath
}
return StackTrace.fromError({
stack: errorEntry.data[0].toString()
}).then((stackLines) =>
stackLines.find((l) => l.fileName && l.fileName.startsWith('/'))
);
};

const matchFile = (frame) => {
if (!frame || !frame.fileName) return null;
const parts = frame.fileName.split('/');
const fileName = parts.slice(-1)[0];
const filePath = parts.slice(0, -1).join('/');
return files.find((f) => f.name === fileName && f.filePath === filePath);
};

Promise.all(consoleErrors.map(resolveFrameFromError)).then((frames) => {
const pairs = frames
.map((frame) => ({ frame, file: matchFile(frame) }))
.filter((p) => p.frame && p.file);
if (pairs.length === 0) return;
expandConsole();
const targetFileId = pairs[0].file.id;
setSelectedFile(targetFileId);
pairs
.filter((p) => p.file.id === targetFileId)
.forEach((p) =>
addErrorDecoration(codemirrorView.current, p.frame.lineNumber)
);
setSelectedFile(fileWithError.id);
addErrorDecoration(codemirrorView.current, line.lineNumber);
});
} else {
removeErrorDecorations(codemirrorView.current);
}
});
}, [consoleEvents]);

const editorSectionClass = classNames({
Expand Down
72 changes: 60 additions & 12 deletions client/modules/Preview/EmbedFrame.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import resolvePathsForElementsWithAttribute from '../../../server/utils/resolveU

let objectUrls = {};
let objectPaths = {};
let jshintErrors = [];
let jshintErrorKeys = new Set();

const Frame = styled.iframe`
min-height: 100%;
Expand Down Expand Up @@ -56,23 +58,49 @@ function resolveCSSLinksInString(content, files) {
return newContent;
}

function jsPreprocess(jsText) {
const JSHINT_OPTIONS = {
esversion: 11,
asi: true,
laxbreak: true,
laxcomma: true
};
Comment thread
Nixxx19 marked this conversation as resolved.

function jsPreprocess(jsText, fileName, lineOffset = 0) {
let newContent = jsText;
// check the code for js errors before sending it to strip comments
// or loops.
JSHINT(newContent);
JSHINT(newContent, JSHINT_OPTIONS);

const fatal = JSHINT.errors.filter(
(err) => err && (!err.code || err.code.startsWith('E'))
);

if (JSHINT.errors.length === 0) {
if (fatal.length === 0) {
newContent = decomment(newContent, {
ignore: /\/\/\s*noprotect/g,
space: true
});
newContent = loopProtect(newContent);
return newContent;
}
return newContent;
fatal.forEach((err) => {
const line = (err.line || 1) + lineOffset;
const key = `${fileName}:${line}:${err.reason}`;
if (jshintErrorKeys.has(key)) return;
jshintErrorKeys.add(key);
jshintErrors.push({
file: fileName,
line,
character: err.character,
reason: err.reason,
evidence: err.evidence,
code: err.code
});
});
return `/* p5 sketch suppressed due to syntax errors in ${fileName}, see console */`;
}

function resolveJSLinksInString(content, files) {
function resolveJSLinksInString(content, files, fileName, lineOffset = 0) {
let newContent = content;
let jsFileStrings = content.match(STRING_REGEX);
jsFileStrings = jsFileStrings || [];
Expand All @@ -98,10 +126,17 @@ function resolveJSLinksInString(content, files) {
}
});

return jsPreprocess(newContent);
return jsPreprocess(newContent, fileName, lineOffset);
}

function getInlineScriptLineOffset(htmlContent, scriptInner) {
if (!htmlContent || !scriptInner) return 0;
const ix = htmlContent.indexOf(scriptInner);
if (ix < 0) return 0;
return htmlContent.substring(0, ix).split('\n').length - 1;
}

function resolveScripts(sketchDoc, files) {
function resolveScripts(sketchDoc, files, htmlContent) {
const scriptsInHTML = sketchDoc.getElementsByTagName('script');
const scriptsInHTMLArray = Array.prototype.slice.call(scriptsInHTML);
scriptsInHTMLArray.forEach((script) => {
Expand Down Expand Up @@ -136,7 +171,12 @@ function resolveScripts(sketchDoc, files) {
) !== null
) {
script.setAttribute('crossorigin', '');
script.innerHTML = resolveJSLinksInString(script.innerHTML, files); // eslint-disable-line
const inlineOffset = getInlineScriptLineOffset(
htmlContent,
script.innerHTML
);
script.innerHTML = resolveJSLinksInString(script.innerHTML, files, 'index.html', inlineOffset); // eslint-disable-line
script.dataset.inlineOffset = String(inlineOffset);
}
});
}
Expand Down Expand Up @@ -175,7 +215,11 @@ function resolveJSAndCSSLinks(files) {
files.forEach((file) => {
const newFile = { ...file };
if (file.name.match(/.*\.js$/i)) {
newFile.content = resolveJSLinksInString(newFile.content, files);
newFile.content = resolveJSLinksInString(
newFile.content,
files,
file.name
);
} else if (file.name.match(/.*\.css$/i)) {
newFile.content = resolveCSSLinksInString(newFile.content, files);
}
Expand All @@ -188,7 +232,8 @@ function addLoopProtect(sketchDoc) {
const scriptsInHTML = sketchDoc.getElementsByTagName('script');
const scriptsInHTMLArray = Array.prototype.slice.call(scriptsInHTML);
scriptsInHTMLArray.forEach((script) => {
script.innerHTML = jsPreprocess(script.innerHTML); // eslint-disable-line
const inlineOffset = parseInt(script.dataset.inlineOffset || '0', 10);
script.innerHTML = jsPreprocess(script.innerHTML, 'index.html', inlineOffset); // eslint-disable-line
});
}

Expand All @@ -197,6 +242,8 @@ function injectLocalFiles(files, htmlFile, options) {
let scriptOffs = [];
objectUrls = {};
objectPaths = {};
jshintErrors = [];
jshintErrorKeys = new Set();
const resolvedFiles = resolveJSAndCSSLinks(files);
const parser = new DOMParser();
const sketchDoc = parser.parseFromString(htmlFile.content, 'text/html');
Expand All @@ -209,7 +256,7 @@ function injectLocalFiles(files, htmlFile, options) {
resolvePathsForElementsWithAttribute('href', sketchDoc, resolvedFiles);
// should also include background, data, poster, but these are used way less often

resolveScripts(sketchDoc, resolvedFiles);
resolveScripts(sketchDoc, resolvedFiles, htmlFile.content);
resolveStyles(sketchDoc, resolvedFiles);

if (textOutput || gridOutput) {
Expand Down Expand Up @@ -241,13 +288,14 @@ p5.prototype.registerMethod('afterSetup', p5.prototype.ensureAccessibleCanvas);`
const sketchDocString = `<!DOCTYPE HTML>\n${sketchDoc.documentElement.outerHTML}`;
scriptOffs = getAllScriptOffsets(sketchDocString);
const consoleErrorsScript = sketchDoc.createElement('script');
addLoopProtect(sketchDoc);
consoleErrorsScript.innerHTML = `
window.offs = ${JSON.stringify(scriptOffs)};
window.objectUrls = ${JSON.stringify(objectUrls)};
window.objectPaths = ${JSON.stringify(objectPaths)};
window.__jshintErrors = ${JSON.stringify(jshintErrors)};
window.editorOrigin = '${getConfig('EDITOR_URL')}';
`;
addLoopProtect(sketchDoc);
sketchDoc.head.prepend(consoleErrorsScript);

return `<!DOCTYPE HTML>\n${sketchDoc.documentElement.outerHTML}`;
Expand Down
Loading
Loading