Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix copyFile wrapper when retry() hits EMFILE again #209

Merged
merged 1 commit into from
Jul 20, 2021

Conversation

mbargiel
Copy link
Contributor

@mbargiel mbargiel commented Mar 15, 2021

The copyFile wrapper introduced in #199 does not follow the same pattern as the other wrappers, and enqueued the wrapped copyFile rather than the wrapper function, which means that a subsequent EMFILE or ENFILE error occurring when retrying the copyFile would fail and bubble the EMFILE or ENFILE to the calling code rather than enqueue again. This fix applies the same pattern by introducing go$copyFile and enqueueing it rather than fs$copyFile.

Fixes #208.

All tests pass and coverage goes up a bit (probably just because the new lines of test code are covered). I couldn't find tests that validate the EMFILE retry logic for the fs function wrappers (at least not for copyFile).

@mbargiel
Copy link
Contributor Author

mbargiel commented Mar 15, 2021

I don't understand the CI failures. The Npm run test step has identical outputs between failed and successful builds, unless it's failing because it takes a bit longer on some test builds. ⁉️

EDIT: Ah, I see it's not an abnormal situation. 😌

@isaacs isaacs force-pushed the feature/fix-copyfile-enqueue branch from 51913d5 to 16f8da2 Compare July 20, 2021 18:29
@isaacs isaacs closed this in 16f8da2 Jul 20, 2021
@isaacs isaacs merged commit 16f8da2 into isaacs:master Jul 20, 2021
Copy link

@manjaneqx manjaneqx left a comment

Choose a reason for hiding this comment

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

🔌

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.

copyFile can fail with EMFILE
3 participants