Better Handle EPERM on Windows

[NickGerleman]

Summary:

Use async fs-extra function for rename, which hooks into a graceful-fs implementation that can handle AV locked files on Windows. See https://github.com/isaacs/node-graceful-fs/blob/89dc1330dcd8fa218c5dff92a97d8792b7da6b12/polyfills.js#L96-L118 for details.

Test Plan:

Existing test coverage.

[thymikee]

Hm, we already utilize graceful-fs in gracefulifyFs.ts, which hooks into fs in this codebase. Shouldn't this handle the case?

[NickGerleman]

Hm, we already utilize graceful-fs in gracefulifyFs.ts, which hooks into fs in this codebase. Shouldn't this handle the case?

The special handling of rename on Win32 seems to only be applied to the async version of "rename". In the source linked above, I couldn't find any special handling of the sync version.

[thymikee]

can we switch this back to fs then and keep the rest of the changes?

[thymikee]

Thank you!

[NickGerleman]

One thing I may want to check is that graceful-fs patches the fs promises interface as well.

[NickGerleman]

I looked at the gracefullify implementation, and it doesn't seem to monkey patch the promises API. The promises API itself internally doesn't call into the callback API. https://github.com/nodejs/node/blob/0a62026f32d513a8a5d9ed857481df5f5fa18e8b/lib/fs.js#L2763

So, we could go back to fs-extra, as exposing promises over graceful-fs, or we could promisify bits of it ourselves. If fs-extra is already a dep that one seems a bit better.

[thymikee]

Ugh. Let's please leave a comment on why we use fs-extra in this place then. It's kinda unexpected, so it makes sense to do it for our future selves and others. We can consider gracefullifying fs.promises as a separate effort, if someone is able to properly test it for regressions.

[NickGerleman]

Added