refactor: insert await null;s needed for await safety

[erights]

Closes: #XXXX
Refs: #XXXX

Description

Prior to this PR, yarn lint warns about many violations of our await safety rules. In vscode, I was very pleasantly surprised to learn about the automated fixes offered interactively for these:

For each of these, I accepted the "insert await null; before..." option. Uniquely for the one shown in the above screenshot I needed to do an additional manual readjustment to move the // eslint-disable-next-line ... so it would still be about the next line.

Caveats:

• I applied the automated fix manually, without reading the code in question or in any other way validating that this change is correctness preserving. My only evidence at the moment is that all the tests still pass.
• I would have loved to mark this PR with refactor: or at least fix:. But I marked it with fix! because there's no reason yet to think these changes are either correctness preserving or compat.
• I applied the automated change blindly even to the test files, even though we're not normally concerned about await safety there. Indeed, we should also change our lint configuration so await safety is unenforced for those. After this PR, we should also change our await safety enforcement for non-test files from "warning" to "error", to keep these from creeping back in.
• I applied the automated fix to the test files anyway since test code still often serves as precedent-by-example for production code. Since the automated fix made it easy to do this, I decided it was better to improve our test-code-as-precedent.

Obviously, this should not be merged until we're confident that these changes are correctness preserving. I don't know of any way to do so other than careful manual review. Perhaps as part of the same review, we might also conclude that it is compat, in which case we can change back from fix! to fix: or even hopefully refactor:.

For the various "considerations" below, I'll answer under the assumption that we have already reviewed these changes for correctness, and deemed them all to be correctness preserving. Obviously, until that happens, this PR is fatal re all the considerations below.

Update: As suggested by review comments, I moved those inserted await null;s to the top of their respective functions.

Update: Between @kriskowal 's evaluation comments below and my own inspection, I'm reasonably confident that this PR should count as a refactor, so I'm changing the title from fix!... to refactor:....

Security Considerations

Assuming the changes are correctness preserving, await safety aids security.

Scaling Considerations

none

Documentation Considerations

we already need to explain await safety. This PR doesn't change that, but repairs our code so that we practice what we will preach.

Testing Considerations

A bit distressing that all tests pass without any further intervention. This perhaps indicates a failure to test for the semantics that are changed by these extra awaits.

Compatibility Considerations

If this change is correctness preserving, then it is likely also compat. But not necessarily. The extra awaits introduce extra turn delays and extra interleaving points.

Upgrade Considerations

If this PR is correctness preserving and compat, then there are unlikely to be any upgrade considerations. But I don't know that for sure, so we should also worry about this during review.

[kriskowal]

For many of these cases, I think the automated refactor was very correctly conservative and placed the await null immediately before a block that introduces await order uncertainty, while preserving rejected-in-one-turn for any exceptions that precede it.

But, a consequence is that the await null is unnecessarily buried in many cases where await null at the beginning of the async function would more clearly communicate that no additional await null will ever be needed in that function.

Having looked at all of these, I believe floating await null to the first statement of every function that needs one preserves correctness. I see no case where we depend on the rejection to occur on the stack of the caller.

[erights]

@kriskowal before hoisting, everything passed CI without any manual changes. After hoisting, I'm getting a snapshot divergence at https://github.com/endojs/endo/actions/runs/9915491154/job/27396418994?pr=2347#step:8:1224

  ✘ [fail]: error-handling › fixtures-error-handling / cjs / loadLocation Did not match snapshot
  ✘ [fail]: error-handling › fixtures-error-handling / cjs / importLocation Did not match snapshot

that I don't know how to evaluate. PTAL, and what do you suggest?

[kriskowal]

@kriskowal before hoisting, everything passed CI without any manual changes. After hoisting, I'm getting a snapshot divergence at https://github.com/endojs/endo/actions/runs/9915491154/job/27396418994?pr=2347#step:8:1224

  ✘ [fail]: error-handling › fixtures-error-handling / cjs / loadLocation Did not match snapshot
  ✘ [fail]: error-handling › fixtures-error-handling / cjs / importLocation Did not match snapshot

that I don't know how to evaluate. PTAL, and what do you suggest?

You can regenerate these snapshots, yarn ava --update-snapshots. These two tests are over-sensitive to changes in the (expected) stack trace.