FI-2086: fix errors on webpack shutdown

[alisawallace]

Summary

Bug fix for FI-2086 webpack not shutting down correctly

This PR fixes two bugs related to shutdown of inferno-core:

1. Hanging terminal on exit

• Issue summary
  • When inferno is successfully started via terminal and then exited, the terminal window hangs after the shutdown process has been completed
• Steps to reproduce
  1. Follow inferno-core README instructions for preliminary setup
  2. Run inferno from the CLI: bin/inferno start
  3. Exit with Ctrl+C
  4. Shutdown messages will print and all processes will terminate, but the terminal will not fully exit back to the command line prompt until additional keystrokes are entered
• Solution
  • Changed Ruby method used to execute shell commands in main.rb. The original system method invokes a separate process to run foreman start and waits for this new process to return, but it never does. Instead, the exec method call replaces the current process and never returns by design - this is appropriate for our use cases since there is no further Ruby code that needs to be executed after the foreman start command is launched.

2. Webpack process is not shut down along with other processes

• Issue summary
  • If inferno encounters terminal errors while launching and prematurely exits, the node process running webpack is not also terminated. As a result, when inferno is restarted, it produces an EADDRINUSE error for IP 0.0.0.0 and port 3000. This error also occurs upon an automated restart when inferno is run with the watch flag set.
• Steps to reproduce
  • Follow inferno-core README instructions for preliminary startup
  • Case 1 - failed launch
    • Inject an error into the inferno source code prior to launching (commenting out the id line in demo_suite.rb is an easy option)
    • Run inferno: bin/inferno start - inferno will throw an error and exit
    • Run inferno again - before the original error can be thrown, webpack will print the EADDRINUSE error and inferno will terminate
  • Case 2 - successful launch with automatic re-run on changes
    • With no errors in the source code, run inferno with the watch flag set: bin/inferno start --watch
    • Change a file in the source code to trigger a rerun - when the startup process occurs for a second time, the EADDRINUSE error will be thrown and inferno will terminate
  • In both cases, the webpack process can still be found running and needs to be manually terminated
• Solution
  • The source of this error appears to result from npm start not propagating signal interrupts to the process it launches (webpack in our case). Here is a blog post about the issue in a different context.
  • The solution is to bypass the npm process entirely by replacing the call to npm run start in the Procfile with the command that npm start would trigger in package.json, which is the webpack serve invocation. This eliminates an unnecessary intermediary process and means signals to pass directly from the foreman process to the node (webpack) process, allowing graceful shutdown.

Testing Guidance

These solutions were developed and tested on a Mac OS and verified to also work on a Windows and Linux OS by Diego.

[codecov]

Codecov Report

Patch and project coverage have no change.

Comparison is base (08ec136) 76.95% compared to head (d1f71ea) 76.95%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #389   +/-   ##
=======================================
  Coverage   76.95%   76.95%           
=======================================
  Files         213      213           
  Lines       10597    10597           
  Branches      978      978           
=======================================
  Hits         8155     8155           
  Misses       1873     1873           
  Partials      569      569           

Flag	Coverage Δ
backend	94.36% <ø> (ø)
frontend	69.62% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[360dgries]

Output from running instructions from "Hanging Terminal on Exit". Looks good I think?

[360dgries]

No issues running instructions in 2.1 -- running both with id still commented out and with it back in the code, neither ran into port issues!

[360dgries]

Having trouble running through the steps of 2.2, but I think it's unrelated. my npm install command has some output that, if I recall correctly, is different from the Mac outputs of the rest of the team. Pasting the output here

npm WARN deprecated rollup-plugin-terser@7.0.2: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-terser
npm WARN deprecated w3c-hr-time@1.0.2: Use your platform's native performance.now() and performance.timeOrigin.
npm WARN deprecated sourcemap-codec@1.4.8: Please use @jridgewell/sourcemap-codec instead
npm WARN deprecated workbox-cacheable-response@6.6.0: workbox-background-sync@6.6.0
npm WARN tarball tarball data for semver@https://registry.npmjs.org/semver/-/semver-7.5.3.tgz (sha512-b39TBaTSfV6yBrapU89p5fKekE2m/NwnDocOVruQFS1/veMgdzuPcnOM34M6CwxW8jH/lxEa5rBoDeUwu5HHTw==) seems to be corrupted. Trying again.
npm WARN tarball tarball data for semver@https://registry.npmjs.org/semver/-/semver-7.5.3.tgz (sha512-b39TBaTSfV6yBrapU89p5fKekE2m/NwnDocOVruQFS1/veMgdzuPcnOM34M6CwxW8jH/lxEa5rBoDeUwu5HHTw==) seems to be corrupted. Trying again.
npm ERR! code EINTEGRITY
npm ERR! sha512-b39TBaTSfV6yBrapU89p5fKekE2m/NwnDocOVruQFS1/veMgdzuPcnOM34M6CwxW8jH/lxEa5rBoDeUwu5HHTw== integrity checksum failed when using sha512: wanted sha512-b39TBaTSfV6yBrapU89p5fKekE2m/NwnDocOVruQFS1/veMgdzuPcnOM34M6CwxW8jH/lxEa5rBoDeUwu5HHTw== but got sha512-QBlUtyVk/5EeHbi7X0fw6liDZc7BBmEaSYn01fMU1OUYbf6GPsbTtd8WmnqbI20SeycoHSeiybkE/q1Q+qlThQ==. (26872 bytes)

npm ERR! A complete log of this run can be found in: /home/dgriese/.npm/_logs/2023-09-08T14_13_55_554Z-debug-0.log

[alisawallace]

Confirming I have the same npm install output and error when I do a fresh clone of the inferno-core repo. My original workaround involved a lot of tinkering and eventually deleting the default package-lock.json file, but it would be nice to not have the error appear in the first place - I'll do another quick check on a solution and if it's not a super straightforward fix, I'll open a ticket for it.

[Jammjammjamm]

This is what I get when I get rid of node_modules and npm install:

~/code/inferno (main ✘)✭ ᐅ npm install
npm WARN deprecated stable@0.1.8: Modern JS already guarantees Array#sort() is a stable sort, so this library is deprecated. See the compatibility table on MDN: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#browser_compatibility
npm WARN deprecated rollup-plugin-terser@7.0.2: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-terser
npm WARN deprecated w3c-hr-time@1.0.2: Use your platform's native performance.now() and performance.timeOrigin.
npm WARN deprecated sourcemap-codec@1.4.8: Please use @jridgewell/sourcemap-codec instead
npm WARN deprecated workbox-cacheable-response@6.6.0: workbox-background-sync@6.6.0
npm WARN deprecated svgo@1.3.2: This SVGO version is no longer supported. Upgrade to v2.x.x.

added 1858 packages, and audited 1859 packages in 13s

326 packages are looking for funding
  run `npm fund` for details

7 vulnerabilities (1 moderate, 6 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

Run `npm audit` for details.
~/code/inferno (main ✘)✭ ᐅ npm --version
8.19.4

[Jammjammjamm]

This is really great. I'm not able to duplicate the issue with webpack not exiting, but the fix seems ok.

I definitely want to get this in ASAP.