Skip to content
This repository has been archived by the owner on Jan 13, 2024. It is now read-only.

fix: do not overwrite temp .node files #1547

Merged
merged 2 commits into from
Apr 1, 2022

Conversation

paul-marechal
Copy link
Contributor

This can happen when two processes try to load the same .node file.

@paul-marechal
Copy link
Contributor Author

The CI / test failures seem unrelated?

@paul-marechal paul-marechal changed the title do not overwrite temp .node files fix: do not overwrite temp .node files Mar 9, 2022
@jesec
Copy link
Contributor

jesec commented Mar 9, 2022

See #1492 .

@paul-marechal
Copy link
Contributor Author

paul-marechal commented Mar 9, 2022

@jesec I know about #1492, and the issue happened because it used to check for the folder to exist, and I can anecdotally say that I also encountered the same issue in the past where the folder would exist but the .node file would be missing.

This PR is different because it checks for the file's existence.

Copy link
Contributor

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

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

@jesec The other PR you mentioned is for the first if case where the entire folder is copied

This can happen when two processes try to load the same .node file.
@robertsLando
Copy link
Contributor

Can this be merged?

@jesec jesec merged commit 63eaeab into vercel:main Apr 1, 2022
@paul-marechal paul-marechal deleted the node-overwrite-fix branch April 1, 2022 20:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants