phetsims / perennial

Maintenance tools that won't change with different versions of chipper checked out
MIT License
2 stars 5 forks source link

Upgrade puppeteer #268

Closed zepumph closed 1 year ago

zepumph commented 2 years ago

Over in https://github.com/phetsims/chipper/issues/1171 we needed to upgrade Node to 16, and perennial 5 breaks on Mac M1 chips and won't even install. So we should upgrade. The latest version is 13.5.2 right now.

zepumph commented 2 years ago

That version works great for me on windows, but @jonathanolson and @samreid have failures when trying to call page.close(), which we do a lot. The error is something like "Target closed"

@samreid found this, which is the same error, but when opening a page, which we aren't finding to be a problem: https://github.com/puppeteer/puppeteer/issues/7916

Anyways, it looks easiest and best for now to just bring perennial up to version 10, and keep chipper the same.

zepumph commented 2 years ago

This is good enough for now. We are upgrade again once we NEED to.

In general we are not excited about this issue, and this solution. Here are a couple reasons:

  1. An old version of puppeteer BROKE builds because it couldn't npm install on a newer device. We thought this was impossible.
  2. Version 10 is "no longer supported" when npm installed, so how long until version 10 breaks builds on npm install.
  3. We are about to re-release all phet-io sims that require puppeteer to build, so this would be an awesome change to get ahead of this issue and release them with a current version of puppeteer, but we didn't like the solution of not calling page.close()
samreid commented 2 years ago

@zepumph discovered that running cleanup before resolve in generate-phet-io-api enabled compatibility with puppeteer 13, so we would like to proceed with that.

samreid commented 2 years ago

We saw this error on CT:

Error: Could not find browser revision 818858. Run "PUPPETEER_PRODUCT=firefox npm install" or "PUPPETEER_PRODUCT=firefox yarn install" to download a supported Firefox browser binary.

So @zepumph and I agreed to update aqua's version of puppeteer as well.

zepumph commented 2 years ago

The issue was reported by the CT quick server and by @mattpen from the build-server side. For CT, SR and I have "fixed" the problem by stopping the service, running prune/update commands in chipper, perennial ,and perennial-alias, and restarting the server.

mattpen commented 2 years ago

I ran the prune/update commands on phet-server2. @jbphet - to verify if this fixes the problem we'll need to resubmit the latest swahili translation of balancing-act.

mattpen commented 2 years ago

@jbphet reran the translation and it worked successfully.

mattpen commented 2 years ago

Back to you for any further steps @zepumph

samreid commented 2 years ago

@pixelzoom reported:

Since yesterday’s puppeteer upgrade, many sims are failing the CT “build” test with: AssertionError [ERR_ASSERTION]: Full API expected but not created from puppeteer step, likely caused by https://github.com/phetsims/chipper/issues/1022. This used to happen once in awhile, but is now occurring more often, and for more sims.

zepumph commented 2 years ago

@samreid, I'm worried about our fix in the PhET-iO API generation, because it looks like @jonathanolson made https://github.com/phetsims/chipper/commit/c30502c26d3a6f30c9fe7dbcdfad9ae2ecf9edfc, and I'm seeing the resurgence of

"Error: Navigation failed because browser has disconnected!"

@jonathanolson, can we pair on this sometime soon?

samreid commented 2 years ago

Is that problem only happening on CT? We have been able to deploy phet-io RCs without a problem.

zepumph commented 2 years ago

If it is the same issue as it presents as from https://github.com/phetsims/chipper/issues/1032 and https://github.com/phetsims/perennial/issues/229, it is a race condition based on something like threads and CPU availability. I have not yet hit it locally while building phet-io sims or generating APIs, but in those issues we did.

samreid commented 2 years ago

I logged into CT and ran a two builds and generate-phet-io-api calls. Both builds succeeded, and 5/6 grunt generate-phet-io-api calls succeeded. The failed one reported:

[reids@bayes center-and-variability]$ grunt generate-phet-io-api --temporary
Running "generate-phet-io-api" task
Error in fulfilling chunk Promise: TimeoutError: Navigation timeout of 30000 ms exceeded
    at /data/share/phet/continuous-testing/chipper/node_modules/puppeteer/lib/cjs/puppeteer/common/LifecycleWatcher.js:108:111
Fatal error: Perennial task failed:
AssertionError [ERR_ASSERTION]: api expected
    at module.exports (/data/share/phet/continuous-testing/chipper/js/phet-io/formatPhetioAPI.js:41:3)
    at /data/share/phet/continuous-testing/chipper/js/grunt/Gruntfile.js:567:37
    at Array.forEach (<anonymous>)
    at /data/share/phet/continuous-testing/chipper/js/grunt/Gruntfile.js:564:12
    at async wrap (/data/share/phet/continuous-testing/chipper/js/grunt/Gruntfile.js:56:7)
Full Error details:
AssertionError [ERR_ASSERTION]: api expected
pixelzoom commented 2 years ago

@KatieWoe asked about this problem today, it's affecting sims frequently in CT. The reports in CT generally look like this, where the first error message is "Full API expected but not created from puppeteer step".

hookes-law : build
Build failed with status code 1:
Running "report-media" task

Running "clean" task

Running "build" task
>> TypeScript compilation complete: 8114ms
12:25:15 PM, 417 ms: ../hookes-law/js/common/HookesLawColors.js
12:25:15 PM, 49 ms: ../hookes-law/js/common/HookesLawConstants.js
...
12:25:18 PM, 26 ms: ../hookes-law/js/systems/view/SystemsViewProperties.js
12:25:18 PM, 37 ms: ../hookes-law/js/systems/view/SystemsVisibilityPanel.js
Building runnable repository (hookes-law, brands: phet, phet-io)
Building brand: phet
>> Webpack build complete: 7497ms
>> Production minification complete: 37950ms (1753728 bytes)
>> Debug minification complete: 0ms (19321861 bytes)
Building brand: phet-io
>> Webpack build complete: 5819ms
>> Production minification complete: 33686ms (1787148 bytes)
>> Debug minification complete: 34475ms (2101456 bytes)
>> No client guides found at ../phet-io-client-guides/hookes-law/, no guides being built.
>> TypeScript compilation complete: 1194ms
Fatal error: Perennial task failed:
AssertionError [ERR_ASSERTION]: Full API expected but not created from puppeteer step, likely caused by https://github.com/phetsims/chipper/issues/1022.
at module.exports (/data/share/phet/continuous-testing/ct-snapshots/1649697426084/chipper/js/grunt/copySupplementalPhetioFiles.js:323:5)
at async module.exports (/data/share/phet/continuous-testing/ct-snapshots/1649697426084/chipper/js/grunt/buildRunnable.js:355:5)
at async /data/share/phet/continuous-testing/ct-snapshots/1649697426084/chipper/js/grunt/Gruntfile.js:283:11
at async wrap (/data/share/phet/continuous-testing/ct-snapshots/1649697426084/chipper/js/grunt/Gruntfile.js:56:7)
Full Error details:
AssertionError [ERR_ASSERTION]: Full API expected but not created from puppeteer step, likely caused by https://github.com/phetsims/chipper/issues/1022.

Error in fulfilling chunk Promise: Error: Navigation failed because browser has disconnected!
at /data/share/phet/continuous-testing/ct-snapshots/1649697426084/chipper/node_modules/puppeteer/lib/cjs/puppeteer/common/LifecycleWatcher.js:51:147
at /data/share/phet/continuous-testing/ct-snapshots/1649697426084/chipper/node_modules/puppeteer/lib/cjs/vendor/mitt/src/index.js:51:62
at Array.map (<anonymous>)
at Object.emit (/data/share/phet/continuous-testing/ct-snapshots/1649697426084/chipper/node_modules/puppeteer/lib/cjs/vendor/mitt/src/index.js:51:43)
at CDPSession.emit (/data/share/phet/continuous-testing/ct-snapshots/1649697426084/chipper/node_modules/puppeteer/lib/cjs/puppeteer/common/EventEmitter.js:72:22)
at CDPSession._onClosed (/data/share/phet/continuous-testing/ct-snapshots/1649697426084/chipper/node_modules/puppeteer/lib/cjs/puppeteer/common/Connection.js:271:14)
at Connection._onMessage (/data/share/phet/continuous-testing/ct-snapshots/1649697426084/chipper/node_modules/puppeteer/lib/cjs/puppeteer/common/Connection.js:105:25)
at WebSocket.<anonymous> (/data/share/phet/continuous-testing/ct-snapshots/1649697426084/chipper/node_modules/puppeteer/lib/cjs/puppeteer/node/NodeWebSocketTransport.js:13:32)
at WebSocket.onMessage (/data/share/phet/continuous-testing/ct-snapshots/1649697426084/chipper/node_modules/ws/lib/event-target.js:199:18)
at WebSocket.emit (events.js:315:20)
Snapshot from 4/11/2022, 11:17:06 AM
samreid commented 2 years ago

It sounds like @zepumph intends to collaborate with @jonathanolson on this issue, self-unassigning for now.

zepumph commented 2 years ago

In https://github.com/phetsims/chipper/commit/83f31efea564645d9282d10b4e9e36e14165767d @jonathanolson and I made good progress in error handling for API generation. In general that has gotten rid of the error we were seeing all over the place, https://github.com/phetsims/perennial/issues/268#issuecomment-1095431686.

I am still seeing some error potentially in the same spot during the build (API generation), like this:

ratio-and-proportion : build
Build failed with status code 3:
Running "report-media" task

Running "clean" task

Running "build" task
>> TypeScript compilation complete: 10093ms
Building runnable repository (ratio-and-proportion, brands: phet, phet-io)
Building brand: phet
>> Webpack build complete: 8724ms
>> Production minification complete: 48566ms (2141060 bytes)
>> Debug minification complete: 1ms (20580623 bytes)
Building brand: phet-io
>> Webpack build complete: 6681ms
>> Production minification complete: 42176ms (2174317 bytes)
>> Debug minification complete: 42491ms (2489796 bytes)
>> No client guides found at ../phet-io-client-guides/ratio-and-proportion/, no guides being built.
>> TypeScript compilation complete: 1165ms
Fatal error: Protocol error: Connection closed. Most likely the page has been closed.
Snapshot from 4/20/2022, 3:33:58 AM
zepumph commented 2 years ago

@jonathanolson FYI,

jonathanolson commented 2 years ago

Any idea what is causing this? Possibly CPU or memory limits? (What happens if we try to run 100 copies of things at the same time?)

zepumph commented 2 years ago

Yeah. That is a good idea, though I'm unsure how to do that locally. It seems like too much of a space constrain to have that many checkouts. Perhaps we can adapt CTQ to kick off 100 puppeteer loads and see if we can recreate.

Perhaps the issue is that there is just one error/exit hook we are missing from Puppeteer that we could use to trigger a retry. Like on browser crash or something.

samreid commented 2 years ago

Today for https://github.com/phetsims/phet-io/issues/1848 @zepumph and I tried checking out grunt checkout-shas for gravity-and-orbits 1.5 and got this error during npm install:

``` ~/apache-document-root/vanilla/gravity-and-orbits$ grunt checkout-shas Running "checkout-shas" task (Forwarding task to perennial) Running "checkout-shas" task info: checking out dependencies for gravity-and-orbits info: git checkout 1971d73693abe3396a5056c4962feead10507e41 on assert info: git checkout a0a8d86a3146fddbb59de25e1062a354c6660eb6 on axon info: git checkout 0c723e76a7bad4344cb588c486ae41d0f7fc4995 on brand info: git checkout e05598d263c1f99551d55bbd7041a5340788b131 on chipper info: git checkout 7f9c76f731915e1c77c647665b247b117d386424 on dot info: git checkout d916fb01f08081dd5d6c41a367868340ab1e1722 on joist info: git checkout 35b503179249118c63892add8e2d3286cc3cbd3c on kite info: git checkout e5408a37539b6ceef9d753b974816903b7a6fbf5 on phet-core info: git checkout 37104fd41a4f4190fad6f3110ccdeb7472ccf474 on phet-io info: git checkout f7d065f94affa5cad58915710ed5eb9fb1c60ac5 on phet-io-client-guides info: git checkout 7a600e3612f5c4fc390b6208efab9879e2539949 on phet-io-wrappers info: git checkout 6a50b84b7fb7084546f2e7023ce59715789fbeb6 on phetcommon info: git checkout 5f8010bebab12c773d4e43fbe564e2012fd95729 on phetmarks info: git checkout 9a9adacabf0f425e7fc0731015f2ffd25c0b9df7 on query-string-machine info: git checkout b3b2d19a7b095bbb0bd616dd285aa342aa10dead on scenery info: git checkout 9f5f8a32e864a9db54e6ade9d5aaf656dbde1f27 on scenery-phet info: git checkout e2d109f45766c8a0461ef8efc5cace8c0486ccf5 on sherpa info: git checkout f0ef021c291b6953ed1e59373de2fdb0c14ccdae on studio info: git checkout 3a4784bcd983e0e5bbb6bfb4fc1c132d29e17d17 on sun info: git checkout fc102421db03f7b2c86076cff6d4fe86765a932d on tambo info: git checkout df84f52086df36d37ad51aeeb3cc19f505946848 on tandem info: git checkout c7c764281d96a2a546c1a755d658e4a05556e2fa on twixt info: git checkout 532a247a34f18f70d55950cbdc8222ad65247cbe on utterance-queue info: npm update on gravity-and-orbits info: npm update on chipper Fatal error: Perennial task failed: Error: npm prune in ../chipper failed with exit code 1 stderr: npm WARN EBADENGINE Unsupported engine { npm WARN EBADENGINE package: 'pngjs@0.4.0', npm WARN EBADENGINE required: { node: '0.8.x' }, npm WARN EBADENGINE current: { node: 'v16.13.1', npm: '8.3.0' } npm WARN EBADENGINE } npm WARN deprecated source-map-url@0.4.1: See https://github.com/lydell/source-map-url#deprecated npm WARN deprecated request-promise-native@1.0.9: request-promise-native has been deprecated because it extends the now deprecated request package, see https://github.com/request/request/issues/3142 npm WARN deprecated urix@0.1.0: Please see https://github.com/lydell/urix#deprecated npm WARN deprecated har-validator@5.1.5: this library is no longer supported npm WARN deprecated resolve-url@0.2.1: https://github.com/lydell/resolve-url#deprecated npm WARN deprecated source-map-resolve@0.5.3: See https://github.com/lydell/source-map-resolve#deprecated npm WARN deprecated fsevents@1.2.13: fsevents 1 will break on node v14+ and could be using insecure binaries. Upgrade to fsevents 2. npm WARN deprecated chokidar@2.1.8: Chokidar 2 does not receive security updates since 2019. Upgrade to chokidar 3 with 15x fewer dependencies npm WARN deprecated querystring@0.2.0: The querystring API is considered Legacy. new code should use the URLSearchParams API instead. npm WARN deprecated uuid@3.4.0: Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details. npm WARN deprecated uuid@3.4.0: Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details. npm WARN deprecated mkdirp@0.5.1: Legacy versions of mkdirp are no longer supported. Please update to mkdirp 1.x. (Note that the API surface has changed to use Promises in 1.x.) npm WARN deprecated request@2.88.2: request has been deprecated, see https://github.com/request/request/issues/3142 npm WARN deprecated puppeteer@5.5.0: Version no longer supported. Upgrade to @latest npm ERR! code 1 npm ERR! path /Users/samreid/apache-document-root/vanilla/chipper/node_modules/puppeteer npm ERR! command failed npm ERR! command sh -c node install.js npm ERR! The chromium binary is not available for arm64: npm ERR! If you are on Ubuntu, you can install with: npm ERR! npm ERR! apt-get install chromium-browser npm ERR! npm ERR! /Users/samreid/apache-document-root/vanilla/chipper/node_modules/puppeteer/lib/cjs/puppeteer/node/BrowserFetcher.js:112 npm ERR! throw new Error(); npm ERR! ^ npm ERR! npm ERR! Error npm ERR! at /Users/samreid/apache-document-root/vanilla/chipper/node_modules/puppeteer/lib/cjs/puppeteer/node/BrowserFetcher.js:112:19 npm ERR! at FSReqCallback.oncomplete (node:fs:198:21) npm ERR! A complete log of this run can be found in: npm ERR! /Users/samreid/.npm/_logs/2022-05-16T19_15_43_954Z-debug-0.log at ChildProcess. (/Users/samreid/apache-document-root/vanilla/perennial/js/common/execute.js:89:21) at ChildProcess.emit (node:events:390:28) at maybeClose (node:internal/child_process:1064:16) at Process.ChildProcess._handle.onexit (node:internal/child_process:301:5) Full Error details: Error: npm prune in ../chipper failed with exit code 1 stderr: npm WARN EBADENGINE Unsupported engine { npm WARN EBADENGINE package: 'pngjs@0.4.0', npm WARN EBADENGINE required: { node: '0.8.x' }, npm WARN EBADENGINE current: { node: 'v16.13.1', npm: '8.3.0' } npm WARN EBADENGINE } npm WARN deprecated source-map-url@0.4.1: See https://github.com/lydell/source-map-url#deprecated npm WARN deprecated request-promise-native@1.0.9: request-promise-native has been deprecated because it extends the now deprecated request package, see https://github.com/request/request/issues/3142 npm WARN deprecated urix@0.1.0: Please see https://github.com/lydell/urix#deprecated npm WARN deprecated har-validator@5.1.5: this library is no longer supported npm WARN deprecated resolve-url@0.2.1: https://github.com/lydell/resolve-url#deprecated npm WARN deprecated source-map-resolve@0.5.3: See https://github.com/lydell/source-map-resolve#deprecated npm WARN deprecated fsevents@1.2.13: fsevents 1 will break on node v14+ and could be using insecure binaries. Upgrade to fsevents 2. npm WARN deprecated chokidar@2.1.8: Chokidar 2 does not receive security updates since 2019. Upgrade to chokidar 3 with 15x fewer dependencies npm WARN deprecated querystring@0.2.0: The querystring API is considered Legacy. new code should use the URLSearchParams API instead. npm WARN deprecated uuid@3.4.0: Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details. npm WARN deprecated uuid@3.4.0: Please upgrade to version 7 or higher. Older versions may use Math.random() in certain circumstances, which is known to be problematic. See https://v8.dev/blog/math-random for details. npm WARN deprecated mkdirp@0.5.1: Legacy versions of mkdirp are no longer supported. Please update to mkdirp 1.x. (Note that the API surface has changed to use Promises in 1.x.) npm WARN deprecated request@2.88.2: request has been deprecated, see https://github.com/request/request/issues/3142 npm WARN deprecated puppeteer@5.5.0: Version no longer supported. Upgrade to @latest npm ERR! code 1 npm ERR! path /Users/samreid/apache-document-root/vanilla/chipper/node_modules/puppeteer npm ERR! command failed npm ERR! command sh -c node install.js npm ERR! The chromium binary is not available for arm64: npm ERR! If you are on Ubuntu, you can install with: npm ERR! npm ERR! apt-get install chromium-browser npm ERR! npm ERR! /Users/samreid/apache-document-root/vanilla/chipper/node_modules/puppeteer/lib/cjs/puppeteer/node/BrowserFetcher.js:112 npm ERR! throw new Error(); npm ERR! ^ npm ERR! npm ERR! Error npm ERR! at /Users/samreid/apache-document-root/vanilla/chipper/node_modules/puppeteer/lib/cjs/puppeteer/node/BrowserFetcher.js:112:19 npm ERR! at FSReqCallback.oncomplete (node:fs:198:21) npm ERR! A complete log of this run can be found in: npm ERR! /Users/samreid/.npm/_logs/2022-05-16T19_15_43_954Z-debug-0.log Fatal error: perennial grunt "--repo=gravity-and-orbits" "checkout-shas" failed with code 1 ```

We think this means we may need to maintenance release updated puppeteer versions.

zepumph commented 2 years ago

@jonathanolson and I think that the "Error: Navigation failed because browser has disconnected!" problems are not related to this issue. But instead because we are doing killall chrome on bayes, which breaks builds sometimes. The only thing left for this issue is to MR puppeteer so that previous PhET-iO sims can still build.

zepumph commented 2 years ago

We did one manual patch for this in Natural Selection 1.4

zepumph commented 2 years ago

@samreid and @jonathanolson ran into some build failures such that it looks like even phet brand sims that don't run puppeteer are still having a build failure on old versions of puppeteer. We should MR all sims with PUppeteer in the package.json. We also found that this failed when npm installing perennial-alias as well, so I believe that we have to do that repo too.

zepumph commented 1 year ago

@jonathanolson just ran into the MR stuff around this for another MR. I'm so sorry!!!! We are discussing and want to proceed with this MR.

I see that the new latest version is 19.2.2. We should update to that on master and then also propagate that to old versions too.

samreid commented 1 year ago

I committed a revert commit on the perennial one to test https://github.com/phetsims/gravity-and-orbits/issues/479#issuecomment-1319496893

samreid commented 1 year ago

I don't know if I need to restart the build server to pick up perennial package.json changes.

samreid commented 1 year ago

It seems the build succeeded, so I'll leave perennial puppeteer at "puppeteer": "~13.5.2", for now. We may need to do the same for aqua or chipper, until we sort out the ~/.cache/puppeteer issue on linux.

zepumph commented 1 year ago

I worked with @mattpen, we needed to restart the build server. After pulling on the server, npm updating, and restarting, we production deployed chains and had no errors. We are ready to close this in terms of the recent upgrade to 19.2. We are NOT ready to close this issue in terms of the MR to back propagate the new version to old shas. Over to @jonathanolson to close if all goes well with the MR (and thank you so much for taking this on!).

zepumph commented 1 year ago

I believe that all has been done here on my end.

jonathanolson commented 1 year ago

Maintenance complete, closing.

zepumph commented 1 year ago

Really great work! Thank you so much.

On Tue, Dec 20, 2022, 15:53 Jonathan Olson @.***> wrote:

Closed #268 https://github.com/phetsims/perennial/issues/268 as completed.

— Reply to this email directly, view it on GitHub https://github.com/phetsims/perennial/issues/268#event-8082396855, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABUKB34NS3UEFDWRY4XQ66TWOI2INANCNFSM5STMESXA . You are receiving this because you were mentioned.Message ID: @.***>

zepumph commented 1 year ago

@jonathanolson and @samreid and I ran into this once again while publishing this MR. We cracked the case though! our fix for old branches (to move the cleanup call to before the resolve). This is not sufficient! @samreid and I believe that there is a behavior change between the old version of Puppeteer originally published with these sims vs the new, 19.2 in which page.goto is still running when we get the message callback to resolve. This means that we need to defensively guard our rejections, knowing that resolving and closing the page will trigger page.goto (which is mid run) to error out.

We will add code comments to master to make this more clear. The easiest way to see this is to console.log in the cleanup function.

For the MR:

I think the fastest thing to do would be for @jonathanolson and I to pair on this to find the sims needed and work out each intricacy.

jonathanolson commented 1 year ago

Patched and deployed without incident. Closing.

zepumph commented 1 year ago

A real life miracle! Thanks @jonathanolson for all the hard work.