search

sindresorhus / tempy

Get a random temporary file or directory path
MIT License
423 stars 26 forks source link

Configure maxRetries or otherwise ignore failures when cleaning up tasks #47

Open novemberborn opened 1 year ago

novemberborn commented 1 year ago

Recursive removal can fail:

https://github.com/sindresorhus/tempy/blob/098c043a85243c407c22a9571f5d0b5c6cdf6647/index.js#L20

Node.js lets us configure a maxRetries for some of these errors, see the documentation.

tempy should do at least one retry, or maybe make it configurable, but perhaps also have an option to ignore EBUSY, EMFILE, ENFILE, ENOTEMPTY, or EPERM errors.

sindresorhus commented 1 year ago

I prefer to set a good default for this before exposing such an option.

I have added 2 as retry: https://github.com/sindresorhus/tempy/releases/tag/v3.1.0

novemberborn commented 1 year ago

@sindresorhus the retries did not do the trick: https://github.com/avajs/ava/pull/3228

sindresorhus commented 1 year ago

I think first you should make sure that actually increasing maxRetries and retryDelay resolves the issue (try a very high number). Then we should find a number for these that always work. I think it's better to make it just work than to make it configurable, if possible.

sindresorhus commented 1 year ago

We could of course ignore EBUSY, but then we would have to document that and make it configurable, and users may expect the temp directory to always be gone.

novemberborn commented 1 year ago

users may expect the temp directory to always be gone.

Actually, my problem is in CI (I don't know if it would happen on regular Windows machines). And in CI I don't care about clean-up at all.

Waiting additional time when AVA's tests are already slow on Windows seems silly.

I'd be happy to have a noClean option.

sindresorhus commented 1 year ago

Yeah. I'm willing to have a clean: false option added.

I would still like to find a a combination of the options which just work on Windows CI though. Then we could potentially have higher numbers for those options when process.env.CI.