GSOC 2019 : Improve Friendly Error System (FES) and documentation

Hello everyone ! I am Ajay Neethi Kannan , and I am studying Computer Science in Indian Institute of Technology, Roorkee. I am extremely interested in working with the processing foundation this summer.

I have been contributing to p5.js and the p5.js-website since January, and it has been and is an amazing experience. I have developed a good understanding of the library during this time, and I am confident that I can implement the features suggested below.

I have explored the projects of this year, and I have become interested in the Friendly Error System. The Friendly Error System
is designed to help beginners to identify the errors in their code easily and rectify them .I am very inspired by it’s one of a kind nature and its logic,
and would love to make it better.

Currently, the components of the Friendly Error System are ,

  • validateParameters : helps in checking the parameters passed to functions in the library, and printing easily understandable messages to debug them.
  • friendlyFileLoadError : helps when a file fails to load correctly.
  • friendlyGlobalFunctionBinder : prints friendly messages when an user attempts to override any global functions defined by the library unknowingly.

I would like to improve upon its functionality , here are some of my ideas :

(The implementation details given below do not represent the final code changes , and I will be able to provide more insight on the implementation part with your feedback and help in the proposal)

IMPROVEMENTS TO EXISTING FES:

1. Improve the readability of validateParameters function :
In the present state, whenever the validateParameters function detects that the arguments passed to the function do not match any of the formats
in which the function takes the arguments (overloads) , it raises a message like

p5.js says: color() was expecting Number for parameter #0 (zero-based index),
received string instead at (file_location)

It does not mention the name of the parameters in the message, which makes it generic and tougher to understand. It would be nice to format the message as,

p5.js says: color() was expecting Number for gray (parameter #0, zero-based index),
received string with value : 'wrong_type' instead at (file_location)

So the information which would be shown is:

  • The name of the parameters , and the type of variable expected.
  • The actual value of the argument passed.
  • The overload which the user intended to use, guessed by FES.

2.Performance and readability improvement to lookupParamDoc() :

Basically the validateParameters() function works in the following way,

a) Given a function, get the data associated with it from the data.json file, which contains the reference generated from the inline documentation.
b) Parse the data to extract only the useful information from it , and store it in a convenient form consisting of maximum parameter size, different overloads, type of variables used for each parameter of every overlod, minimum parameters for every overload.
c) Use the extracted data to compare the arguments passed to a function , and check which overload it suits the most.
d) If no errors, it returns succesfully, otherwise the closest approximated overload is used to calculate the overload errors, using getOverloadErrors().

The problems associated with this approach are :

  • The entire data.json is stored in the final p5.js file, and occupies nearly one fourth of the entire library size (20,000 lines)
  • The different fields such as description, examples which occupy most of the space will not be used , and can be checked in the p5.js-website easily,
    which already has i18n support too.
  • Since the data is huge and the algorithmic complexity used to get the data of a specific function is O(n), it took upto even 0.8s when searching for the
    function data for the first time (as it is cached subsequently).
  • The logic used to achieve the parsing the required function data and storing it in a convenient data structure is amazing, but I found it extremely tough
    to understand.

So the suggestion for improvement is :

  • Move the parsing logic away from the library and generate a new file , similar to data.json, which contains only the necassary information about the different functions. This file called FES.json, will be generated from data.json, and the data for each function will be stored by using the
    logic from lookupParamDoc, by parsing them and storing in the convenient retrievable form.
  • This process will be done outside the library, and all the function data will be ready to use in the library, by simply ‘requiring’ them.
  • Add documentation explaining the data structure used in representing the function data, so that people can visualize it , while exploring error_helpers.js

Advantages of this approach :

  • The file size can be reduced , as we shall remove the unnecessary data.
  • The readability of the code improves drastically.
  • Performance improves a lot as now data retrieval is O(1) complexity , as the function data is already present in required format.
  • If i18n is to be implemented here , we can store only the necessary fields here such as a field’s description, while not storing information like the desctiption of the entire method in different languages, the examples, as we can use the p5.js-website for that.
  • Suppose in the future, we need additional logic for some functions , which will be used by the FES, we can store it in the new format easily.

3.Caching the console messages :

We have to store an amount of console messages without affecting the performance much , so that we can prevent the same friendly error messages to keep on printing in the console. In the case , where the validateParameters() detects a mismatch in the arguments, but the code continues to execute without throwing any error, the same function is called during every loop of the draw function , thus flooding the console with the same comments, hiding any other message printed in the console along the way .
For eg :

function draw()
{
    arc(1, 1, 10.5, 10);
}
setTimeout(() => console.log('I will be hidden.'), 2000);

The suggestion for the improvement is:

  • If a similar console message has been printed by the FES within a said interval , prevent it from being printed again in the console , or bring the attention
    of the user to it .

Advantages :

  • More debuggable console messages.

4.(Re)introducing formatting and colors into the console :

It would be great to not only have color for the console messages, but also good formatting grouping error messages of same function, text alignment, clear distinction
for each type of error, etc.

Some of the features which could be implemented are :

  • Improving visibility and distinction between different error types, with visibility being the important concern.
  • Presently if a function’s arguments fail for more than one argument, we get all the errors stacked one below the other , wihout indentation. So it would be nice to indent and group error messages of the same type – improving the friendlyParamError() function
  • Language sensitive formatting : With the implementation of i18n, it would be important to include text align properties for right-to-left languages, and left-to-right languages.
  • Making the errors welcoming and friendly through colors, ascii art with help from the community.

5.Improve accuracy of validateParameters():

Consider this sketch:

function setup()
{
    let a, b, c, d;
    background(a, b, c, d);
}

This gives the error :

p5.js says: color() was expecting at least 1 arguments, but received only 0 at file:///home/aj_ryuk/muyarchi/p5.js/lib/p5.js:51575:35. [reference | p5.js]

There are two problems with this :

  • The user would be confused to see that he has passed 4 arguments , but the message says 0 arguments. (This is dealt at 6)

  • Second, the message says ‘color() was expecting’ , and not ‘background() was expecting’
    These can be solved partially by having stronger checking for the background() function, but still the arguments are passed to color() function,
    internally through the background() function , and now both errors for background() and color() function show up .
    So when one public facing function calls another public facing function internally , care must be taken that the error messages of the internal functions
    do not show up.
    This can be implemented by maintaing a variable called ‘depth’ , which is incremented when we call a public facing function, and decremented when we exit
    the public facing function. It is reset to zero when an error is thrown. Only when the value of the variable is less than or equal to 1, the FES displays the message.
    (The detailed implementation has not been finalised yet, but just a rough idea).

6. The minor and side changes which would be incorporated :

a) Separating ‘load’ and ‘loadX’ logic from the report() function to make it more general and

b) There seem to be few issues with CORS headers in loading of files, especially images , so those changes can be incorporated into p5._friendlyFileLoadError()

c) In p5._friendlyParamError() function , whenever an EMPTY_VAR, error is thrown , it may due to uninitialized variables , but the current function doesn’t acknowledge this.
For example consider ,

        function setup()
        {
            let a , b = 10, c , d;
            background(a ,b, c, d);
        }

Even though the intention of the user may be to use the four parameter overload of the background function , the current FES, removes the trailing undefined variables , and applies the logic for two parameter overload (alpha, gray) of this function. So , the error message for UNITIALIZED_VAR should be included, along with EMPTY_VAR, WRONT_TYPE, TOO_FEW_ARGUMENTS, etc.

d) Updating FES to ES6 standards.

e) i18n for number to string functions using the Intl API. (different languages have different number formats)

7.Improve the documentation of the friendly error system :

When I started contributing to the p5.js library, I found the FES to be the most daunting to understand , because I was unable to visualize the flow of the system.

So I think as part of improving it’s documentation, it would be nice to,

  • update the developer docs for FES
  • improve inline documentation of the FES.
  • explain the flow and order in which the functions execute.
  • provide a visual representation of the FES , to explain its flow, the data structures used and so on.

ADDITION OF NEW FEATURES :

Some of the new features I am excited about are ,

8.Sound and visual cues support :

Cue train whistle here 
 !

Inspired from the p5.js-web-editor, we can have different types of friendly sounds and visual cues to indicate that there may be an error in the code,
showing successful execution, etc.
The aim is to raise different , distinct , friendly cues for different errors, warnings ,etc . so that the user is made aware that there is some error, before they
open the console.
If this feature can be implemented, care would be taken to prevent the cues from interfering with the functionality of the acutal sketch.

9.Global error handling along wih i18n:

The aim is to catch the errors thrown in the console , and match them up with friendly comments, as mentioned in the developer documentation.
The issues associated are :

  • Every browser has a different error message description for different errors.
  • i18n support needs to be added.
  • It may be daunting for a beginner to see a red error box (or yellow warning box) with a cryptic message, so we can format it and provide suggestions to debug it.

The suggestions for the basic implementation are :

We will create a list of suggestions for possible errors thrown while using the library - These suggestions will support i18n.

The first step would be catch the global error thrown using window.onerror event listener.

Next step would be analyse the error message and get details about where the error occurred , variable names, line number , column number , file ,etc . and store it for formatting

Next we will analyse the stack trace, identifying the p5 functions which were present along the way , to suggest the path along which they should attempt to debug the error. (omitting the non public facing API of p5).

Next we will use a string matching algorithm ( such as levenshtein’s algorithm, trigram comparison) and identify which suggestion best suits the error message.
(We will have to experiment with different algorithms to identify which best suits our purpose)
Then we will add i18n to the suggestion which most closely explains the error message.

Combining all the above , we will provide
a) a nicely formatted message,
b) which will provide the type of the error,
c) location of the error,
d) suggest the path to take to resolve the error (order in which to check the files , functions, etc)
e) suggestion on how to solve the error , which supports i18n,
f) and the error object below.

I request the community and the mentors to provide their views on the proposed ideas.
Any help towards new ideas and methods to implement these features would be greatly appreciated.

Writing tests for FES and i18n:

All the implemented methods will be followed by writing of tests for them as well as for methods whose tests are yet to be written.




Some of the doubts I have are,

1.One of the major features to be included in FES is to provide an i18n framework for the library itself, which will be used by the console messages and FES. I have some ideas to implement them but I am not sure about the requirements of the implementation.

  • Do we have to work on providing an interface to i18n in the library using pre existing libraries like i18next, or
  • Do we have to implement the i18n support from scratch , building our own i18n framework ?

If we chose to use a framework like i18next, in my humble opinion, few of the benefits are :

  • Abstraction from i18n implementation and the codebase will remain clean, which helps the library to continue to stay beginner friendly for contributors.
  • i18next is an amazing framework which is extremely mature , and has support above normal interpolation, formatting,etc . like language detection, graceful fallbacks, nesting translations, translating arrays, objects, etc.
  • It also allows a standard format for i18n to be implemented in the library, as it is pretty stable and error tested. This will also help people interested in translating to get started quicker because of the standardised format.

I could be wrong about this, I request the community to provide their thoughts on this , so that I can propose my ideas for implementing i18n support in the library.

2.I would like to know how deeply should I explain the implementation details of the above proposed ideas, should I discuss this with the assigned mentor ?

Also , @outofambit , I would be overjoyed if you could provide your feedback on this ,
I would start writing my proposal form once my doubts have been cleared.

Thank you all for your time !

3 Likes

Hey everyone !
I have made the first draft of my proposal with respect to the features mentioned above.
Here is the link to my proposal

Please do have a look and provide your opinions on it. All suggestions are welcome !

I am sorry about the length of the proposal, I tried to explain the features in a detailed way, making the proposal long. I am willing to shorten it in case needed.

I would like to thank you all profusely, fot the past two months with the community has been a blast! I have learnt so many things along the way and contributed to this amazing library, with your help.

I have some doubts regarding the proposal :

  • How is i18n going to be implemented in the library ? @limzykenneth explained well that it will be mostly implemented as an optional library. Please provide your thoughts on how I should include it in my proposal. If it forms a part of this project, I would remove the less important features of my current proposal to make way for this important one. I would also be happy to co operate in case it is implemented outside this project, and change the features proposed in this project accordingly.

@lmccart @outofambit @limzykenneth I would be most grateful if you could clear my doubts and review my proposal.

Thank you so much everyone ! :heart::heart::heart:

@Ajayneethikannan this is looking great. One suggestion I’d have for the readability section: I think the language itself could be simplified and made clearer. For example “parameter #0, zero based index” could be confusing if they don’t know what zero based index is. More clearly this could just say “the first parameter”. I suspect there may be other small modifications like this that could make a big impact.

Thank you @lmccart for your suggestions ! I will examine the FES in detail , and update the proposal accordingly.

Hey everyone! I have nearly finished my proposal and I have some doubts regarding the implementation of i18n in the library. One of the important ideas for improving the FES is to support i18n for the console and FES messages. I tried to think about how to implement it, and this is what I came up with, ( do correct my mistakes )

The parts of FES which handle generating messages for the console are p5._friendlyParamError( ), p5.prototype._createFriendlyGlobalFunctionBinder( ), helpForMisusedAtTopLevelCode( ), friendlyWelcome( ), p5._friendlyFileLoadError( ), etc.
Since these methods also log messages to the console with few well defined statements and do not depend on the data.json file in any way,
implementing i18n for FES would be the same as for other functions in the library.

The parts required for supporting i18n in the library are :

Translation functionality:

This will provide the framework to translate the messages. It will comprise of the i18n features,

  • interpolation( for dynamic values in translation )
  • formatting( formatting numbers, strings before translating them )
  • nesting( use other keys inside a translation ) and
  • plurals( if necessary )

Language resources:

i18n works by translating keys. We need to provide the values in different languages to produce the final translated message. One way to organise the different values is to store the messages( values for the console messages in different languages ) separately for each function. For example, the resource for the Spanish language may look like,

let esTranslationResource = {
    //for function getSum
    getSum : {
      The_parameters_are_not_numbers: "Los parĂĄmetros a: {{a, number}} y b: {{b}} no son nĂșmeros.",
    },

    //for function compareArrays
    compareArrays : {
      Array_size_not_equal: "Las matrices {{a, number}} (longitud: {{a.length, number}}) y {{b, number}} (longitud: {{b.length, number}}) no tienen la misma longitud"
    },

    //common translations
    Welcome: "Hola {{name, uppercase}} !"
}

Example:

Considering the above translation values for the Spanish language, a rough implementation can be:

function compareArrays(a, b)
{
  ...  
  if(a.length !== b.length)print('compareArrays', "Array size not equal", {a: a, b: b} ); //the short error message acts as a key (after some modification) as well as a fallback message.
  ...
}

with the output as :

Las matrices [1, 2] (longitud : 2) y [1] (longitud : 1) no tienen la misma longitud

Here the print function internally translates the message. It’s arguments are:

  • Function name
  • Key: The key will be a short message in english, without any dynamic values, which will be printed as a last resort in case there is some error.
  • parameters: An object giving the dynamic values to be printed in the message as well as options.


Implementation in the library:


Translation functionality:

This can be implemented by using an external library, such as i18next or a custom implementation.( What are the community’s thoughts on this ? )

The translation functionality would be present in both the minified as well as the unminified versions of the library.

The reasons for this are:

  • Other p5.js libraries can easily implement i18n in their own console messages, by only loading in their own translations( resources ).
  • If the translation functionality is present only in unminified version, then it would be repetition of work to print the message using a key and without a key in different versions.

The doubts faced are:

  • Is it fine to use an external framework such as i18next for example, to implement this ?

Language resources:

The resource of the primary language ( here, english ) would be present in both the unminified and minified versions. The resources for other languages would be present only in the unminified library,
and ability to add extra language resources would be present in both versions of the library.

Additional p5.js libraries can load their language resources in both the minified and unminified versions of the library.

Doubts:

Some of the doubts faced by me are:

  • What are your thoughts on using an external library to create the i18n functionality?
  • I noticed that currently there is a pending PR for implementing i18n for console messages, if it is to be implemented independently, how should I add it in my proposal ? I would be extremely happy to co operate with other contributors outside this project too.

I request @lmccart , @outofambit, @limzykenneth and the community to provide their thoughts on this. If it is accepted, I will be replacing the low priority feature suggestions in my proposal with
implementing i18n in the library.

Thank you very much !

Hey everyone !

I have completed the final draft of my proposal, it would be extremely helpful, if you could review it and provide suggestions to improve it.

Here is the link to my proposal
Thank you very much !

Hey ! I managed to submit my proposal for improving the Friendly Error System as well one for improving the unit tests of the library ( could not get it reviewed though :cry: ).
Thank you everyone :grin::grin::grin: