Net Fetch in Electron

[jameskerr]

Override fetch using net.fetch in our electron application to allow for the Happy Eyeballs feature.

I've confirmed that the flakey test run reliably, but I need help confirming the happy eyeballs.

Fixes #3063

[jameskerr]

This loosens up the eslint rules so I can @ts-ignore and use any type.

[philrz]

I think this now has a working set of changes. I checked out this branch at commit f08a717, but before testing I reverted the changes in commit 8b8817c just because I don't yet have total faith in why-is-node-running and didn't need to debug in this test. With that I was able to run the looping repro script through 100 iterations and saw zero failures. 🎉

Then to check the "happy eyeballs" behavior is still intact, I did a cherry-pick of the changes from dfdc525 to make sure that the Zed lake service was listening only on localhost:9867 and confirmed I was still able to successfully load data, so that also checks out. 👍

That said, I do see that the CI flagged a bunch of breakages in yarn test caused by the most recent commit and I was able to reproduce that locally, so I guess those Jest tests need some kind of treatment to run ok in this new config. @jameskerr hopefully that's easy? 🤞

[philrz]

I mentioned in a previous comment that despite the app functionality and e2e tests working ok after the changes thus far, we were still seeing many failures during yarn test that looked like the following, such as in this CI run:

FAIL apps/zui/src/electron/start.test.ts
  ● app opens a window on startup

    TypeError: Cannot read properties of undefined (reading 'fetch')

      3 |
      4 | export class ElectronZedLake extends Lake {
    > 5 |   fetch = net.fetch
        |               ^
      6 | }
      7 |

      at new fetch (src/core/electron-zed-lake.ts:5:15)
      at Function.boot (src/core/main/main-object.ts:47:18)
      at Object.boot (src/electron/run-main/boot.ts:13:16)
      at Object.main (src/electron/run-main/run-main.ts:19:19)
      at Object.<anonymous> (src/electron/start.test.ts:18:20)

  ● activate creates window if there are none

Based on my understanding, these Jest tests are more lightweight so I assume the root cause is that Electron stuff like net.fetch are simply not available when Jest tests are running. I figured one way to address this would be to determine if a test was running in Jest and, if so, choose a different fetch. I did some web searches and saw others claiming success with checking Jest-ness via the presence of this JEST_WORKER_ID env var so I used that approach here and it does seem to work, as yarn test now passes locally and in CI and the e2e tests still pass. I'd appreciate some close scrutiny of my approach to confirm it's sound, though!

[philrz]

At commit 6626d66 (i.e., after the final two commits from @jameskerr in this branch), I just did a final looping run of the e2e test discussed in #3063:

$ SUCCESSES=0; FAILURES=0; while true; do   yarn e2e -g pool-load-success.spec.ts --skip-nx-cache; if [ $? -eq 0 ] ; then SUCCESSES=$(expr $SUCCESSES + 1); else FAILURES=$(expr $FAILURES + 1); fi; echo "Successful test runs=$SUCCESSES, failed test runs=$FAILURES"; done

That saw 130 successful runs and 0 failures before I stopped the loop. So indeed, these changes are very much having the intended effect!

I then had a final chat with @jameskerr just to finalize my understanding of where things stand as of when this PR will merge. Going forward we'll depend on two different Fetch implementations:

1. When running zed-node outside if the Zui app context, the Fetch that will be used is whichever comes in the Node.js that's being used to run zed-node.
2. When running Zui, the Fetch that will be used is the "net Fetch" that's supplied by Chromium.

Which is all to say that we never use the Fetch that's part of the Node.js that's bundled with Electron (despite it being the default), since we're always doing one of those two overrides.