[p5.js 2.0 Beta Bug Report]: Error in FES prevents message from being shown

[quinton-ashley]

Most appropriate sub-area of p5.js?

• Accessibility
• Color
• Core/Environment/Rendering
• Data
• DOM
• Events
• Image
• IO
• Math
• Typography
• Utilities
• WebGL
• Build process
• Unit testing
• Internationalization
• Friendly errors
• Other (specify if possible)

p5.js version

v2 beta 5

Web browser and version

134.0.6998.166 (Official Build) (arm64)

Operating system

macOS Version 15.3.2 (Build 24D81)

Steps to reproduce this

To reproduce run new p5().

Error log:

p5.js:74778 Uncaught TypeError: func.substring is not a function
    at mapToReference (p5.js:74778:34)
    at p5._report (p5.js:74821:19)
    at p5._friendlyError (p5.js:74859:12)
    at checkForUserDefinedFunctions (p5.js:74989:16)
mapToReference @ p5.js:74778
p5._report @ p5.js:74821
p5._friendlyError @ p5.js:74859
checkForUserDefinedFunctions @ p5.js:74989
p5.js:68523 Uncaught TypeError: Cannot read properties of undefined (reading 'pixels')
    at get pixels (p5.js:68523:29)
    at p5.js:75668:27
    at Array.map (<anonymous>)
    at getSymbols (p5.js:75665:12)
    at defineMisusedAtTopLevelCode (p5.js:75680:9)
    at helpForMisusedAtTopLevelCode (p5.js:75713:9)
get pixels @ p5.js:68523
(anonymous) @ p5.js:75668
getSymbols @ p5.js:75665
defineMisusedAtTopLevelCode @ p5.js:75680
helpForMisusedAtTopLevelCode @ p5.js:75713

p5.js:74778 Uncaught TypeError: func.substring is not a function
    at mapToReference (p5.js:74778:34)
    at p5._report (p5.js:74821:19)
    at p5._friendlyError (p5.js:74859:12)
    at checkForUserDefinedFunctions (p5.js:74989:16)
mapToReference @ p5.js:74778
p5._report @ p5.js:74821
p5._friendlyError @ p5.js:74859
checkForUserDefinedFunctions @ p5.js:74989
p5.js:68523 Uncaught TypeError: Cannot read properties of undefined (reading 'pixels')
    at get pixels (p5.js:68523:29)
    at p5.js:75668:27
    at Array.map (<anonymous>)
    at getSymbols (p5.js:75665:12)
    at defineMisusedAtTopLevelCode (p5.js:75680:9)
    at helpForMisusedAtTopLevelCode (p5.js:75713:9)
get pixels @ p5.js:68523
(anonymous) @ p5.js:75668
getSymbols @ p5.js:75665
defineMisusedAtTopLevelCode @ p5.js:75680
helpForMisusedAtTopLevelCode @ p5.js:75713

This error happens here in p5.js because context is the window and p5 overrides window.constructor p5's class constructor, which is why it's enumerated by Object.keys when it wouldn't otherwise.

fxns has a constructor property too because all JS Objects do, but ofc it doesn't store a string. Hence since func is not a string it lacks string methods.

for (const prop of Object.keys(context)) {
  const lowercase = prop.toLowerCase();

  // check if the lowercase property name has an entry in fxns, if the
  // actual name with correct capitalization doesnt exist in context,
  // and if the user-defined symbol is of the type function
  if (
    fxns[lowercase] &&
    !context[fxns[lowercase]] &&
    typeof context[prop] === 'function'
  ) {
    const msg = translator('fes.checkUserDefinedFns', {
      name: prop,
      actualName: fxns[lowercase]
    });

    p5._friendlyError(msg, fxns[lowercase]);
  }
}

[HughJacks]

Hi, I am wondering if anyone is working on this issue, and if i could be assigned to it.

Fair warning this is my first contribution. I would like to contribute as part of an assignment for coursework, but could not find any good first issues that were not either, trivial frontend code or already assigned.

[HughJacks]

when we do fxns[lowercase] with prop === 'constructor',
we get back the Object constructor function instead of a string (
since all JS objects inherit the constructor property from Object.prototype).

in src/core/friendly_errors/fes_core.js could we change:

for (const prop of Object.keys(context)) {
  
  const lowercase = prop.toLowerCase();
  if (
    fxns[lowercase] &&
    !context[fxns[lowercase]] &&
    typeof context[prop] === 'function'
  ) {
    

to:

if (
  fxns.hasOwnProperty(lowercase) &&
  !context[fxns[lowercase]] &&
  typeof context[prop] === 'function'
)

would using hasOwnProperty work in this case so that our check would only return true if the specified property is a direct property of fxns.

this way we could ignore Object.prototype.constructor which is inhereted.

please let me know any feedback about this approach.

Thanks

[quinton-ashley]

Good but also I think p5 shouldn't override window.constructor

[HughJacks]

It seems like this is where the window.constructor might get overridden. in core/main.js

const bindGlobal = (property) => {
  Object.defineProperty(window, property, {
    configurable: true,
    enumerable: true,
    get: () => {
      if(typeof this[property] === 'function'){
        return this[property].bind(this);
      }else{
        return this[property];
      }
    },
    set: (newValue) => {
      Object.defineProperty(window, property, {
        configurable: true,
        enumerable: true,
        value: newValue,
        writable: true
      });
      if (!p5.disableFriendlyErrors) {
        console.log(`You just changed the value of "${property}", which was a p5 global value. This could cause problems later if you're not careful.`);
      }
    }
  })
};

Would simply excluding constructor from this function or the later loops that call it be enough to fix this issue? That doesn't seem like the smartest way of fixing this issue.

Additionally are there any other default window properties that might get overridden we want to protect? Im not sure my idea is the most robust solution. Any feedback welcome!

[davepagurek]

Thanks @HughJacks! If you add a conditional breakpoint in there or something to catch when property === 'constructor', can you see what the call stack is to see how it got there? I think that bindGlobal is mostly intended for properties visible to the user (e.g. width, height, frameCount) so preventing it from being called in constructor could be the right move.

Additionally are there any other default window properties that might get overridden we want to protect? Im not sure my idea is the most robust solution. Any feedback welcome!

Once we see which loop it's coming from, we could maybe log all the things being bound and see if anything seems suspicious.

[HughJacks]

okay after trying this out I found that we are overriding in this loop

for (const p of Object.getOwnPropertyNames(p5.prototype)) {
        if(p[0] === '_') continue;
        bindGlobal(p);
      }

Just to check for other overrides I downloaded a text file with the output of this code which should have all the default window properties

function getAllProperties(obj) {
    const props = new Set();
    let current = obj;
    
    while (current) {
        Object.getOwnPropertyNames(current).forEach(prop => props.add(prop));
        current = Object.getPrototypeOf(current);
    }
    
    return Array.from(props).sort();
}

const windowProperties = getAllProperties(window);

then I compared it to a list i collect from bindGlobal

and found that we are potentially overriding

constructor, length, print

I guess a potential solution would just be to create a set of "protected" properties in bind global and skip them.