Add shutdown.

[davidlehn]

A shutdown() API is added to perform orderly shutdown. This came about due to how bedrock-test was exiting. In the flow it uses, calling bedrock.exit() or process.exit() after successful testing results in a more abrupt shutdown. There isn't a way to exit and emit orderly events on the way. In some testing cases you may want bedrock.exit or similar events to trigger to do cleanup.

This patch attempts to keep the same semantics for current code and only adds the shutdown() call (and does some refactoring). Retaining all prior behavior in all cases isn't well tested. Help wanted.

shutdown() could be named something else or be hacked into exit() as options, but this seemed like a simple approach. At the time this was written, it wasn't named stop(), but now I can't recall the reasoning and that might be a better choice as it aligns with start/stop semantics and event naming. Comments welcome.

[codecov]

Codecov Report

Attention: Patch coverage is 73.91304% with 6 lines in your changes missing coverage. Please review.

Project coverage is 81.35%. Comparing base (a3da45b) to head (26d6943).
Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/index.js	73.91%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #111      +/-   ##
==========================================
- Coverage   81.43%   81.35%   -0.09%     
==========================================
  Files          10       10              
  Lines        1939     1952      +13     
==========================================
+ Hits         1579     1588       +9     
- Misses        360      364       +4     

Files with missing lines	Coverage Δ
lib/index.js	69.28% <73.91%> (-0.01%)	⬇️

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3da45b...26d6943. Read the comment docs.

---- 🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests
• JS Bundle Analysis - Avoid shipping oversized bundles

[dlongley]

The high-level design here looks good, so assuming this works as expected, +1. Thanks!

[mattcollier]

This does not appear to impact how Bedrock behaves when it receives a SIGTERM, is that correct?

[davidlehn]

This does not appear to impact how Bedrock behaves when it receives a SIGTERM, is that correct?

The signal handler calls _exit() so it will emit bedrock.stopped now. That could cause behavior change if you implement that event handler, but if it doesn't exist, that wouldn't matter. Looks like a finally block also will call cluster.worker.kill() for workers. It's been a while so I can't recall the exact reasoning there. I think that was needed to stop everything when the program started orderly shutdown, where before it would simply exit abruptly and not need that call? Testing for all these specific cases would be useful but I imagine may be a bit tricky in practice.

Also there's still the open naming question in the PR description if anyone has thoughts.

[dlongley]

@mattcollier, I'd like to merge this (and do a minor release). Can you take a look at @davidlehn's comments and figure out if we need to make any changes to enable this to happen or if it's good to go?

[dlongley]

@mattcollier @davidlehn This PR keeps not landing -- we should try to push it forward so it doesn't hang out here any longer.

[mattcollier]

@davidlehn can you do a quick audit to look for bedrock.stopped event handlers? I don't think that is used, certainly not commonly.

[davidlehn]

@davidlehn can you do a quick audit to look for bedrock.stopped event handlers? I don't think that is used, certainly not commonly.

bedrock.stopped is new in this PR, and I don't think was ever used elsewhere.