Skip to content
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.

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

Unable to build library using grunt. Fatal error: Protocol error (Runtime.callFunctionOn): Object reference chain is too long. #5367

Closed
1 of 17 tasks
malviys opened this issue Jul 29, 2021 · 12 comments · Fixed by #5469
Labels

Comments

@malviys
Copy link
Contributor

malviys commented Jul 29, 2021

Most appropriate sub-area of p5.js?

  • Accessibility (Web Accessibility)
  • Build tools and processes
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Friendly error system
  • Image
  • IO (Input/Output)
  • Localization
  • Math
  • Unit Testing
  • Typography
  • Utilities
  • WebGL
  • Other (specify if possible)

Details about the bug:

Unable to build library using grunt

  • p5.js version: 1.4.0
  • Web browser and version: Google Chrome Version 92.0.4515.107 (Official Build) (64-bit)
  • Operating System: Windows 10 Build 19043.1110
  • Node version: 16.5.0
  • Steps to reproduce this:
npm run grunt

Console error:

Fatal error: Protocol error (Runtime.callFunctionOn): Object reference chain is too long
@malviys malviys added the Bug label Jul 29, 2021
@welcome
Copy link

welcome bot commented Jul 29, 2021

Welcome! 👋 Thanks for opening your first issue here! And to ensure the community is able to respond to your issue, be sure to follow the issue template if you haven't already.

@limzykenneth
Copy link
Member

@malviys Can you share a more complete console output?

@malviys
Copy link
Contributor Author

malviys commented Aug 18, 2021

@limzykenneth It is very long output Grunt builds and runs test all test cases. Don't know how much code would be sufficient because it tries to run test cases and then fails.

@limzykenneth
Copy link
Member

Basically we need to know which task failed and if it's the test, which test is causing the issue. I know it can be difficult to tell where the issue occur at first glance so if you could share a more complete output I can diagnose it better. You can put it on a gist or pastebin as those are a bit more suitable for long text than github issue comments.

@malviys
Copy link
Contributor Author

malviys commented Aug 24, 2021

@limzykenneth here is the pastebin link https://pastebin.com/FdULWrF2 for the error details

@limzykenneth
Copy link
Member

It seems to be a puppeteer problem. You can change your node.js version to 14.x and npm version to 6.x and you should be able to run grunt fully. I'll need to see what kind of fix is available for npm version 7.x.

@limzykenneth
Copy link
Member

I couldn't do a straightforward update to puppeteer 10.2.0 (latest version) because of a few failed unit tests that I'm not too familiar with. @outofambit Can you perhaps have a look?

  1) Global Error Handling
       identifies TypeError 'readFromNull':

      AssertionError: expected 0 to equal 1
      + expected - actual

      -0
      +1
      
      at Function.assert.strictEqual (http://localhost:9001/node_modules/chai/chai.js:2329:32)
      at http://localhost:9001/test/unit/core/error_helpers.js:774:14

  2) Global Error Handling
       identifies TypeError 'readFromUndefined':

      AssertionError: expected 0 to equal 1
      + expected - actual

      -0
      +1
      
      at Function.assert.strictEqual (http://localhost:9001/node_modules/chai/chai.js:2329:32)
      at http://localhost:9001/test/unit/core/error_helpers.js:792:14

  3) time and date
       p5.prototype.millis
         result should be greater than running time:
     AssertionError: everything is ok: expected 50 to be > 50

@limzykenneth
Copy link
Member

limzykenneth commented Sep 15, 2021

The unit tests are now fixed by #5412 but after testing updating to 10.2.0, the same error described in the original issue still occurs.

I filed an issue with puppeteer, hopefully that get some respond. In #5409 playwright was mentioned as an alternative, might be worth trying if it gives us better build stability.

@DivyamAhuja
Copy link
Contributor

DivyamAhuja commented Sep 30, 2021

  src\math\p5.Vector.js
    toString documentation
p5.Vector Object : [20, 30, 0]
       example #1 works
       example #2 works
    set documentation
       example #1 works
       example #2 works
    copy documentation
true
       example #1 works
    add documentation
       example #1 works
       example #2 works
Fatal error: Protocol error (Runtime.callFunctionOn): Object reference chain is too long

Am still getting this error.

Will I have to revert back to previous node and npm versions?
My current installation includes

node v16.8.0
npm 7.21.0

@limzykenneth
Copy link
Member

@DivyamAhuja Yes, that's what was mentioned in terms of the original error. Reverting to node 14 should work. We're looking into solutions for node 16.

@limzykenneth
Copy link
Member

limzykenneth commented Oct 19, 2021

Good news and bad news. Good news: I found the source of this problem. Bad news: may be a bit tricky to resolve.

For a more detailed explanation see the issue on puppeteer: puppeteer/puppeteer#7570. Essentially the problem occurs because there's a cyclic reference in p5.Vector instances, specifically it came from the p5 property in the object itself (which is a reference to the sketch instance and nested in there is a cyclic reference). The cyclic reference causes a stringify operation to fail in puppeteer which in node 14 and before throws a "Unhandled promise rejection" message but carries on, from node 15 onwards an unhandled promise rejection is a fatal error which stops everything, thus what we see here.

There are a few ways that we can possibly resolve this. First way is to not have the cyclic reference in the first place (it came from p5._curElement which has a reference to itself) which I think would be best but might not be possible. Second way is to not save the p5 instance in p5.Vector instances, looking at the code, the p5 instance property is used to know how we are running the different functions in p5.Vector and to call two hidden/private functions _toRadians and _fromRadians. As such I think it is possible to remove the full reference to the p5 instance from p5.Vector entirely without problems.

@limzykenneth
Copy link
Member

TLDR: I manage to remove the reference to the p5 instance from p5.Vector objects and everything seems to work now. Will file a PR.


For posterity:

So I've looked into some potential solutions and the first is to remove cyclic reference entirely. This proves to be very difficult as not just p5._curElement has cyclic reference, p5._elements, p5._renderer, and p5._preloadMethods all have cyclic references as well. The one in p5._curElement is easy to remove but the rest, from what I can see, will require at least major changes to many core parts of p5, if such a thing is even possible without breaking things.

Another potential solution is to remove the reference to the p5 instance in the p5.Vector objects. These instance is used so that when angleMode is changed, methods such as heading() can return angles in the specified angle mode. This is done by directly calling the instance's _fromRadians and _toRadians method. As such there isn't really a need for p5.Vector to have a reference to the whole p5 instance. I've removed that reference and change the internal signature of the p5.Vector constructor to directly pass bounded versions of _fromRadians and _toRadians to the constructor. With this all tests passed on both node 14 and 16.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants