Wp-calypso: Fix Runtime Exception in WP-Desktop

Created on 19 Dec 2019  路  5Comments  路  Source: Automattic/wp-calypso

A fairly recent commit to wp-calypso introduced a runtime exception in wp-desktop. The crash was reverted in wp-desktop #708, although the exact SHA of the Calypso change that introduced this behavior was unknown at the time. I've since been able to isolate the breaking SHA to be d748e25, with description "fix(deps): update dependency chalk to v3 (#37465)" (Author: renovate[bot]).

Description of the Error

The runtime exception would manifest with the following error message prior to the application crashing:

wp-desktop-calypso-exception

Steps to reproduce

  1. Build wp-desktop with Calypso submodule commit da02d87 (this is the last good commit). _Confirm that wp-desktop runs without error._
  2. Build wp-desktop with Calypso submodule commit d748e25 (first bad commit). _Confirm the application displays the error message shown above, and crashes immediately after._

Next Steps

We need to address the breaking code change so we can update wp-desktop as soon as possible.

Note

Since this error was observed, Calypso hasn't been updated in wp-desktop, and Calypso has since migrated to node v12. This issue is still present as of the most recent Calypso SHA 0928598. Once this patch has landed, we will be able to build Calypso with node v12 against wp-desktop (however you can add the patch manually in the mean time for development/testing). Update: Patch for building wp-desktop with latest Calypso has landed on develop branch, which should help with debugging. However this runtime crash is a separate issue and still persists.

Desktop App [Pri] High [Type] Bug

Most helpful comment

I believe chalk@3 was the first chalk to start using "modern" js features and require Node 8 to run. We may need to transpile chalk to use it with the older node that Electron is currently using?

All 5 comments

I believe chalk@3 was the first chalk to start using "modern" js features and require Node 8 to run. We may need to transpile chalk to use it with the older node that Electron is currently using?

@jsnajdr Thank you for assisting with this! I tested the fix in wp-desktop #725 and I'm observing that a runtime crash still persists. Also, CI in wp-desktop is failing (to rule out flaky tests I tried re-running CI a couple times with the same results). I'd love to be wrong, but I'm not sure the code change addresses the issue. To repro:

  1. Check out latest develop in wp-desktop (59ff0839
  2. Ensure you're building from a clean slate: rm -rf release && rm -rf build && rm -rf node_modules && rm -rf calypso/node_modules
  3. Update calypso to latest (bd8db57ac5 as of this writing)
  4. nvm use 12.13.1
  5. npm ci in both root of repo and in Calypso subdirectory
  6. make build (ignore the error Can't locate Mac/Memory.pm in @INC - this is related to being unable to generate DMGs on Catalina).

The built app is under release/mac. When I double-click to run the app, I observe the following:

calypso-runtime-error-2019-12-20

I could be missing something, or it could be that you had something cached locally when testing the proposed fix?

cc: @sirreal @belcherj

Did a little more testing. It seems the tip of wp-desktop/develop is building with the patch submitted (develop is still on an earlier SHA of Calypso).

However, updating to latest Calypso in wp-desktop results in another runtime error locally, and so far we've been unable to build on CI. It could be related to the fact that Calypso has since migrated to Node v12.

We should probably open a separate issue to track.

cc: @jsnajdr

The error is different this time: .flat is not a function. Array.prototype.flat is a recent addition to the language and must be polyfilled in many environments, including the Electron server. And we very recently (less than 24 hours ago as I write this) started using it in the ProductIcon component (fyi @delawski): https://github.com/Automattic/wp-calypso/blob/797fc80ce188d62709342d26a4673a2959b6d808/packages/components/src/product-icon/config.js#L46

We apparently don't polyfill correctly during the desktop build. Let's have a look at it.

Thanks for the deeper dive, @jsnajdr! Yep, let鈥檚 definitely have a closer look at this.

I think we鈥檙e pretty close to getting CI fixed for wp-desktop, so hopefully we鈥檙e fairly close to getting everything building again once this last runtime issue is fixed.

Was this page helpful?
0 / 5 - 0 ratings