From 0ae5f13a1396874f8bf0fc2c4bcaf2f39ac9f997 Mon Sep 17 00:00:00 2001 From: Aayushdev18 Date: Mon, 5 Jan 2026 01:52:17 +0530 Subject: [PATCH 1/3] Fixes #8381 Prevents FES from throwing error when friendlyStack is null. When a ReferenceError occurs and processStack returns null for friendlyStack, accessing friendlyStack[0].lineNumber would throw a TypeError, causing a second error that masks the actual error. - Added null checks for friendlyStack before accessing properties - Added unit test to verify FES handles null friendlyStack gracefully All tests pass (1892/1892). --- src/core/friendly_errors/fes_core.js | 5 ++- test/unit/core/fes_error_monitoring.js | 57 ++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) create mode 100644 test/unit/core/fes_error_monitoring.js diff --git a/src/core/friendly_errors/fes_core.js b/src/core/friendly_errors/fes_core.js index 8962745918..0b8ac57391 100644 --- a/src/core/friendly_errors/fes_core.js +++ b/src/core/friendly_errors/fes_core.js @@ -724,7 +724,10 @@ function fesCore(p5, fn){ stacktrace && stacktrace[0].fileName && stacktrace[0].lineNumber && - stacktrace[0].columnNumber + stacktrace[0].columnNumber && + friendlyStack && + friendlyStack[0] && + friendlyStack[0].lineNumber ) { locationObj = { location: `${stacktrace[0].fileName}:${stacktrace[0].lineNumber}:${ diff --git a/test/unit/core/fes_error_monitoring.js b/test/unit/core/fes_error_monitoring.js new file mode 100644 index 0000000000..c7f43c90ac --- /dev/null +++ b/test/unit/core/fes_error_monitoring.js @@ -0,0 +1,57 @@ +import p5 from '../../../src/app.js'; + +suite('FES Error Monitoring', function () { + let myp5; + let logs = []; + let originalFesLogger; + + beforeEach(function () { + logs = []; + myp5 = new p5(function () {}); + // Use _fesLogger to capture FES error messages (proper way to test FES) + originalFesLogger = myp5._fesLogger; + myp5._fesLogger = function (...args) { + logs.push(args.join(' ')); + }; + }); + + afterEach(function () { + myp5._fesLogger = originalFesLogger; + if (myp5) { + myp5.remove(); + } + }); + + test('should handle ReferenceError without throwing when friendlyStack is null', function () { + // This test verifies the fix for issue #8381: FES should not throw an error + // when friendlyStack is null (which happens when processStack returns [false, null]) + // Before the fix, accessing friendlyStack[0].lineNumber would throw a TypeError, + // causing a second error that masks the actual ReferenceError + + let fesThrewError = false; + let errorMessage = ''; + + // Create a ReferenceError similar to what happens when a variable is undefined + const referenceError = new ReferenceError('s is not defined'); + + // Manually trigger FES error monitor + // This should not throw even if friendlyStack is null + try { + if (myp5 && myp5._fesErrorMonitor) { + myp5._fesErrorMonitor(referenceError); + } + } catch (e) { + fesThrewError = true; + errorMessage = e.message; + } + + // FES should not throw its own error + // This is the core fix: before the fix, accessing friendlyStack[0].lineNumber + // when friendlyStack is null would throw a TypeError, causing a second error + assert.isFalse( + fesThrewError, + `FES should not throw an error when handling ReferenceError. Error: ${errorMessage}` + ); + }); +}); + From 1716d8db387e215df6ef526a8960f128d5aff564 Mon Sep 17 00:00:00 2001 From: Aayushdev18 Date: Mon, 5 Jan 2026 17:04:28 +0530 Subject: [PATCH 2/3] Fixes #8381 Prevents FES from crashing when getSymbols accesses getters on prototype. After running the example sketch (https://editor.p5js.org/davepagurek/sketches/aVaqe0sNn) with dev tools open, I found the root cause: when getSymbols(fn) is called with fn (the p5 prototype), checking typeof obj[name] for properties like 'pixels' triggers the getter, which tries to access this._renderer.pixels, but this._renderer is undefined on the prototype, causing a TypeError. The fix uses Object.getOwnPropertyDescriptor to check if a property is a getter before accessing it, skipping getters to avoid triggering instance property access when obj is the prototype. - Added getter check in getSymbols to skip properties with getters - Added defensive null checks for friendlyStack before accessing properties - Updated test to use real ReferenceError scenario from example sketch Tested locally with the example sketch - now shows only the original ReferenceError, no FES TypeError. All tests pass. --- src/core/friendly_errors/fes_core.js | 15 +++- test/unit/core/fes_error_monitoring.js | 98 ++++++++++++++++++++------ 2 files changed, 89 insertions(+), 24 deletions(-) diff --git a/src/core/friendly_errors/fes_core.js b/src/core/friendly_errors/fes_core.js index 0b8ac57391..72f5618d97 100644 --- a/src/core/friendly_errors/fes_core.js +++ b/src/core/friendly_errors/fes_core.js @@ -1039,7 +1039,17 @@ function fesCore(p5, fn){ }) .map(name => { let type; - + + // Use getOwnPropertyDescriptor to avoid triggering getters + // that depend on instance state (like 'pixels' which accesses this._renderer) + const descriptor = Object.getOwnPropertyDescriptor(obj, name); + + if (descriptor && descriptor.get) { + // It's a getter, skip it to avoid accessing instance properties + // when obj is the prototype + return null; + } + if (typeof obj[name] === 'function') { type = 'function'; } else if (name === name.toUpperCase()) { @@ -1049,7 +1059,8 @@ function fesCore(p5, fn){ } return { name, type }; - }); + }) + .filter(symbol => symbol !== null); misusedAtTopLevelCode = [].concat( getSymbols(fn), diff --git a/test/unit/core/fes_error_monitoring.js b/test/unit/core/fes_error_monitoring.js index c7f43c90ac..b9f4d35477 100644 --- a/test/unit/core/fes_error_monitoring.js +++ b/test/unit/core/fes_error_monitoring.js @@ -22,36 +22,90 @@ suite('FES Error Monitoring', function () { } }); - test('should handle ReferenceError without throwing when friendlyStack is null', function () { + test('should handle ReferenceError without throwing when friendlyStack is null', function (done) { // This test verifies the fix for issue #8381: FES should not throw an error // when friendlyStack is null (which happens when processStack returns [false, null]) // Before the fix, accessing friendlyStack[0].lineNumber would throw a TypeError, // causing a second error that masks the actual ReferenceError + // + // This test reproduces the scenario from the example sketch: + // https://editor.p5js.org/davepagurek/sketches/aVaqe0sNn + // where accessing an undefined variable (like `s + 20`) triggers a ReferenceError + // that goes through handleMisspelling -> get pixels -> fesErrorMonitor + // Track if FES itself throws an error (the bug we're fixing) let fesThrewError = false; - let errorMessage = ''; + let referenceErrorCaught = false; + let errorMessages = []; + const originalErrorHandler = window.onerror; - // Create a ReferenceError similar to what happens when a variable is undefined - const referenceError = new ReferenceError('s is not defined'); - - // Manually trigger FES error monitor - // This should not throw even if friendlyStack is null - try { - if (myp5 && myp5._fesErrorMonitor) { - myp5._fesErrorMonitor(referenceError); + // Set up error handler to catch any errors + window.onerror = function(message, source, lineno, colno, error) { + errorMessages.push({ + message: message, + name: error ? error.name : 'Unknown', + errorMessage: error ? error.message : message + }); + + // Check if this is the specific TypeError from FES accessing friendlyStack[0].lineNumber + // This is the bug we're fixing: when friendlyStack is null, accessing friendlyStack[0].lineNumber + // at line 730 in fes_core.js throws "Cannot read property 'lineNumber' of undefined" + // We specifically check for errors about 'lineNumber' to avoid false positives from + // other TypeErrors (like the 'pixels' error from handleMisspelling) + if (error && error.name === 'TypeError' && error.message && + (error.message.includes('lineNumber') || + error.message.includes("can't access property 'lineNumber'"))) { + fesThrewError = true; } - } catch (e) { - fesThrewError = true; - errorMessage = e.message; - } - - // FES should not throw its own error - // This is the core fix: before the fix, accessing friendlyStack[0].lineNumber - // when friendlyStack is null would throw a TypeError, causing a second error - assert.isFalse( - fesThrewError, - `FES should not throw an error when handling ReferenceError. Error: ${errorMessage}` - ); + // Check if this is the original ReferenceError + if (error && error.name === 'ReferenceError') { + referenceErrorCaught = true; + } + return false; // Don't prevent default error handling + }; + + // Create a p5 instance that will trigger a ReferenceError naturally + // Similar to the example sketch: console.log(s + 20) where s is undefined + // This will trigger the same code path: ReferenceError -> handleMisspelling -> fesErrorMonitor + const testP5 = new p5(function(p) { + p.setup = function() { + p.createCanvas(100, 100); + }; + + p.draw = function() { + // Access undefined variable to trigger ReferenceError naturally + // This mimics the example sketch: console.log(s + 20) + // The error will propagate to the browser's error handler, which FES monitors + // This triggers the same code path as the example: handleMisspelling -> fesErrorMonitor + const result = s + 20; // eslint-disable-line no-undef + p.noLoop(); // Stop after first frame + }; + }); + + // Give FES time to process the error + setTimeout(function() { + window.onerror = originalErrorHandler; + + // FES should not throw its own error + // This is the core fix: before the fix, accessing friendlyStack[0].lineNumber + // when friendlyStack is null would throw a TypeError, causing a second error + assert.isFalse( + fesThrewError, + 'FES should not throw its own error when handling ReferenceError. ' + + 'If this fails, FES is throwing a TypeError when friendlyStack is null. ' + + 'Error messages: ' + JSON.stringify(errorMessages, null, 2) + ); + + // Should have caught the original ReferenceError + assert.isTrue( + referenceErrorCaught, + 'Should have caught the original ReferenceError from undefined variable. ' + + 'Error messages: ' + JSON.stringify(errorMessages, null, 2) + ); + + testP5.remove(); + done(); + }, 200); }); }); From 0ea6e64935cdedbfde25d989b7421edaf2e18c09 Mon Sep 17 00:00:00 2001 From: Aayushdev18 Date: Wed, 14 Jan 2026 11:53:56 +0530 Subject: [PATCH 3/3] Fix FES to use p5 instance instead of prototype for property checks Use p5.instance when available to check property types, allowing getters like 'width' and 'pixels' to work correctly. Falls back to descriptor approach when no instance is available. This prevents crashes while preserving misspelling detection for getter properties. --- src/core/friendly_errors/fes_core.js | 57 +++++++++++++++++----------- 1 file changed, 35 insertions(+), 22 deletions(-) diff --git a/src/core/friendly_errors/fes_core.js b/src/core/friendly_errors/fes_core.js index 72f5618d97..1bcfd81111 100644 --- a/src/core/friendly_errors/fes_core.js +++ b/src/core/friendly_errors/fes_core.js @@ -1022,9 +1022,12 @@ function fesCore(p5, fn){ */ defineMisusedAtTopLevelCode = () => { const uniqueNamesFound = {}; + const currentInstance = p5.instance || null; - const getSymbols = obj => - Object.getOwnPropertyNames(obj) + const getSymbols = obj => { + const instanceToUse = obj === fn ? currentInstance : null; + + return Object.getOwnPropertyNames(obj) .filter(name => { if (name[0] === '_') { return false; @@ -1040,33 +1043,43 @@ function fesCore(p5, fn){ .map(name => { let type; - // Use getOwnPropertyDescriptor to avoid triggering getters - // that depend on instance state (like 'pixels' which accesses this._renderer) - const descriptor = Object.getOwnPropertyDescriptor(obj, name); - - if (descriptor && descriptor.get) { - // It's a getter, skip it to avoid accessing instance properties - // when obj is the prototype - return null; - } - - if (typeof obj[name] === 'function') { - type = 'function'; - } else if (name === name.toUpperCase()) { - type = 'constant'; + if (instanceToUse) { + try { + const value = instanceToUse[name]; + if (typeof value === 'function') { + type = 'function'; + } else if (name === name.toUpperCase()) { + type = 'constant'; + } else { + type = 'variable'; + } + } catch (e) { + const descriptor = Object.getOwnPropertyDescriptor(obj, name); + if (descriptor && descriptor.value && typeof descriptor.value === 'function') { + type = 'function'; + } else if (name === name.toUpperCase()) { + type = 'constant'; + } else { + type = 'variable'; + } + } } else { - type = 'variable'; + const descriptor = Object.getOwnPropertyDescriptor(obj, name); + if (descriptor && descriptor.value && typeof descriptor.value === 'function') { + type = 'function'; + } else if (name === name.toUpperCase()) { + type = 'constant'; + } else { + type = 'variable'; + } } return { name, type }; - }) - .filter(symbol => symbol !== null); + }); + }; misusedAtTopLevelCode = [].concat( getSymbols(fn), - // At present, p5 only adds its constants to fn during - // construction, which may not have happened at the time a - // ReferenceError is thrown, so we'll manually add them to our list. getSymbols(contants) );