P5.js: FES: Disable parameter validation for internal calls

Created on 28 May 2020  路  5Comments  路  Source: processing/p5.js

Most appropriate sub-area of p5.js?

  • [x] Other (Friendly Errors)

Feature enhancement details:

In #4571, I mentioned a number of problems that must also be addressed before addressing the main issue. In essence, I was talking about p5 functions that pass forward arguments which they didn't even receive in the first place. Quoting myself from there:

Fixing this would also involve changing the code inside p5.proptotype.save here.
The problem is that when we call save() to save a JSON like this save(myJSON, 'my.json'), a call is made internally to saveJSON with the arguments (args[0], args[1], args[2]). However, args[2] is optional and is undefined in this case.

loadModel documentation, example 1.
ValidationError: loadStrings() was expecting Function for parameter #2 (zero-based index), received an empty variable instead.
We are not passing any callbacks in the example and that's okay because those are optional parameters. But the code here passes the failureCallback argument to loadStrings without checking if it exists. And so we see a validation error in loadStrings. This can also be fixed via a simple check.

But these are just the visible ones that would result in a test failure. I do not know how many other instances there are in the source where something similar happens. So instead of addressing these directly, I thought they could be addressed alongside another known issue of the FES. Suppose we run the sketch

function setup() {
 createCanvas(400, 400);
 let data = {message: 'hi'}, file;
 /* some code */
 saveJSON(data, file);
}

We get 馃尭 p5.js says: saveJSON() was expecting at least 2 arguments, but received only 1
and also
馃尭 p5.js says: saveStrings() was expecting String for parameter #1 (zero-based index), received an empty variable instead
The error for saveStrings is confusing here because it was never called by the user.

I suggest that the FES shouldn't display parameter validation messages when a function is called internally by another p5 function. This would also indirectly "solve" the problems I mentioned in #4571

enhancement

Most helpful comment

My idea for this to see the stack trace here and get the functions in it.
For a p5 function called from the user's sketch we would see a stack
| |
| -----------------------|
| _friendlyParamError |
| validateParameters |
| some p5 functon |
| some non p5 function |

We can display a validation message in this case.

For a p5 function called internally, we would see a stack like
| |
| -----------------------|
| _friendlyParamError |
| validateParameters |
| some p5 functon |
| some p5 function |

We should not display a validation message in this case

All 5 comments

My idea for this to see the stack trace here and get the functions in it.
For a p5 function called from the user's sketch we would see a stack
| |
| -----------------------|
| _friendlyParamError |
| validateParameters |
| some p5 functon |
| some non p5 function |

We can display a validation message in this case.

For a p5 function called internally, we would see a stack like
| |
| -----------------------|
| _friendlyParamError |
| validateParameters |
| some p5 functon |
| some p5 function |

We should not display a validation message in this case

We spoke about this before but just to restate publicly, I think this is a great idea.

Perhaps you should break up error_helpers.js into multiple files before working on this since it might add a significant amount of code?

Perhaps you should break up error_helpers.js into multiple files before working on this since it might add a significant amount of code?

Yes. I was just wondering if I could bundle up the breaking up of the file and the fix for this issue into one PR or should they go into different ones

You can bundle into one PR.

Resolved in #4590. I used the word "Addresses" there. That's why this didn't close automatically. Closing now.

Was this page helpful?
0 / 5 - 0 ratings