Page MenuHomePhabricator

[keyserver] Ignore potential to crash on promise rejections in scripts
ClosedPublic

Authored by ashoat on Dec 4 2023, 1:18 PM.
Tags
None
Referenced Files
F3365886: D10177.id34235.diff
Mon, Nov 25, 8:08 AM
F3365009: D10177.id34310.diff
Mon, Nov 25, 6:17 AM
Unknown Object (File)
Fri, Nov 22, 3:05 AM
Unknown Object (File)
Fri, Nov 22, 2:18 AM
Unknown Object (File)
Thu, Nov 21, 8:30 PM
Unknown Object (File)
Thu, Nov 21, 5:02 AM
Unknown Object (File)
Thu, Nov 21, 5:02 AM
Unknown Object (File)
Thu, Nov 21, 5:02 AM
Subscribers

Details

Summary

Node.js runtimes such as keyserver can crash on unhandled promise rejections. In general, that's something we probably want to avoid.

However, in the context of scripts, we actually want an unhandled promise rejection to crash the script. In this diff, we add void declarations to silence the unused-promise Lint, and otherwise leave the behavior as-is.

Regarding the void keyword: the Flow team recommends using the void keyword to silence the unused-promise Lint. See the "Fire-and-forget" part of this blog post.

Depends on D10176

Test Plan

Flow

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Dec 4 2023, 1:53 PM
Harbormaster failed remote builds in B24756: Diff 34219!

(Should we put [skip-ci] in title of all of these diffs if they're expected to fail?)

ashoat published this revision for review.Dec 4 2023, 6:27 PM
keyserver/src/scripts/utils.js
15 ↗(On Diff #34235)

Now that this function is no longer async, and doesn't return anything, is it required that we add void in each script?

keyserver/src/scripts/utils.js
15 ↗(On Diff #34235)

Those are different mains. The mains in each script avoid are defined in that script, and don't take any parameters. No voids were added in front of invocations of this main function

Adding @atul here to potentially unblock the stack

keyserver/src/scripts/utils.js
15 ↗(On Diff #34235)

s/avoid//

This revision is now accepted and ready to land.Dec 5 2023, 10:23 AM