Skip to content

dont forget to retry#217

Closed
nlf wants to merge 2 commits intoisaacs:masterfrom
nlf:nlf/dont-forget-to-retry
Closed

dont forget to retry#217
nlf wants to merge 2 commits intoisaacs:masterfrom
nlf:nlf/dont-forget-to-retry

Conversation

@nlf
Copy link
Copy Markdown
Contributor

@nlf nlf commented Jul 21, 2021

if a user is only running one fs operation at a time, and that operation fails with an EMFILE, we never call their callback. because of that the event loop no longer has anything to do and the entire process silently exits.

this change makes it so we start retrying an operation immediately, rather than waiting for another operation to finish successfully or fs.close/fs.closeSync to be called. in the callbacks for those, we now reset all state for everything in the queue and make sure the retry loop is running, this will make sure things retry sooner if they can.

the retry now keeps running every tick until the queue is empty. items are removed from the queue either when we retry them, or when they've reached a maximum of 60 seconds. this corrects the contract so that we will not stop trying to process the queue until every item in it has had its callback fired one way or another.

Comment thread graceful-fs.js Outdated
@nlf nlf force-pushed the nlf/dont-forget-to-retry branch from 2dbe3ad to 02b4099 Compare July 28, 2021 19:56
@nlf nlf force-pushed the nlf/dont-forget-to-retry branch from 02b4099 to 18b7fd4 Compare July 28, 2021 20:01
Comment thread graceful-fs.js

// if we don't have a startTime we have no way of knowing if we've waited
// long enough, so go ahead and retry this item now
if (startTime === undefined) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is this ever undefined?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

older versions of this library use the same global queue, this is some defensive coding to make sure if a user is somehow using two versions at the same time we don't do whacky things

@isaacs isaacs closed this in e4ee5d6 Aug 5, 2021
@ljharb
Copy link
Copy Markdown

ljharb commented Aug 5, 2021

The merged commit seems to contain some changes that aren't present in this PR, which cause graceful-fs to suddenly start breaking in node <= 4: e4ee5d6#r54480099

isaacs pushed a commit that referenced this pull request Aug 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants