Remove file when process is canceled

[kevva]

Currently, if you're doing stuff using fsWriteStreamAtomic() and cancels the process the temp file will remain in place. Optimally it'd clean up any temp files on exit.

Also, are there any reasons to not write the temp files to os.tmpdir()?

[othiym23]

Optimally it'd clean up any temp files on exit.

Optimally, yes, but cleanup on exit in Node is not really very optimal (because you have no guarantees that anything async will complete before the process shuts down). We could enqueue files into a list and then fs.unlinkSync() them on exit (npm 2 does something similar for rollback), but that feels a little high-level for what is right now a package focused on doing one relatively simple thing.

Also, are there any reasons to not write the temp files to os.tmpdir()?

VERY YES. The atomicity of the operation depends on fs.rename() being atomic, and you can't rename across filesystem boundaries (since all you're doing is changing what name is mapped to a given inode).

[kevva]

I guess we could do it manually in our package by removing this.__atomicTmp on process.exit or process.SIGINT.

[othiym23]

It's tricky, because I've seen exit handlers fail to fire once there are more than a couple process.on('exit') listeners in some versions of Node.

[sindresorhus]

because you have no guarantees that anything async will complete before the process shuts down

Maybe someday (hopefully I'm still alive :p): http://nodejs.org/docs/v0.11.13/api/process.html#process_event_beforeexit

It's tricky, because I've seen exit handlers fail to fire once there are more than a couple process.on('exit') listeners in some versions of Node.

Still better than nothing though.

[othiym23]

Yeah, beforeExit is a possibility, but it'd still be best-effort, because fs.writeStreamAtomic's primary consumer (not naming any names cough) is still going to need to support 0.10 after the release of 0.12, and realistically, people will complain if it doesn't work with 0.8 for some time after 0.12's release as well. That said, if somebody put together a patch adding this in an opt-in, backwards-compatible way, we'd take it.

[edef1c]

Failing to remove temporary files isn't a disaster, it's merely the current behaviour.
Adding best-effort tempfile removal shouldn't be blocked by imperfection, imo.

[sholladay]

It has been about a year, with a few major versions of Node with beforeExit. Any updated thoughts on this?