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

stop roll() from failing if the new cache folder is not created #243

Merged
merged 3 commits into from
Sep 25, 2020

Conversation

xaviergonz
Copy link
Contributor

Today I stumbled upon an error where it would fail when the cache folder was not present
(typescript) Error: ENOENT: no such file or directory, rename 'REDACTED\node_modules\.cache\rollup-plugin-typescript2/rpt2_759e2fd8d3f20c1a0805faf5404af6bea36632fd/code/cache_' -> 'REDACTED\node_modules\.cache\rollup-plugin-typescript2/rpt2_759e2fd8d3f20c1a0805faf5404af6bea36632fd/code/cache'

This PR fixes it by checking its existence before trying to rename it

Today I stumbled upon an error where it would fail when the cache folder was not present
```(typescript) Error: ENOENT: no such file or directory, rename 'REDACTED\node_modules\.cache\rollup-plugin-typescript2/rpt2_759e2fd8d3f20c1a0805faf5404af6bea36632fd/code/cache_' -> 'REDACTED\node_modules\.cache\rollup-plugin-typescript2/rpt2_759e2fd8d3f20c1a0805faf5404af6bea36632fd/code/cache'```

This PR fixes it by checking its existence before trying to rename it
@xaviergonz xaviergonz changed the title stop roll() from failing if the new cache has no files stop roll() from failing if the new cache folder is not created Sep 25, 2020
@ezolenko ezolenko merged commit c183d33 into ezolenko:master Sep 25, 2020
@thiamsantos
Copy link

@ezolenko There is any plan to release this fix on npm?

Seems that on the repo we are already on 0.27.4, but this version was not published on npm.

@ezolenko
Copy link
Owner

This should be in 0.27.3 on npm already (unless I messed up releases :)).
0.27.4 is version of git master, it is always higher than last npm release

@agilgur5
Copy link
Collaborator

agilgur5 commented Oct 7, 2020

@xaviergonz @ezolenko any chance one of you could explain how this fixes the root cause or what the root cause of this bug is? I investigated this a bit in jaredpalmer/tsdx#896 / jaredpalmer/tsdx#892 / jaredpalmer/tsdx#888 but did not really understand the root cause here. Per one of my comments there:

The cache code initiates both directories in its constructor which would by definition be called before its roll() call

I'm not sure why one of the directories wouldn't exist during roll() if the constructor creates it. Am I missing something here or is this some complicated race condition?
TSDX also didn't hit this bug until recently, despite not updating rpts2 in several months (though the cache dir was changed in a previous release, but per the first link, it occurred on a fresh install as well).

@agilgur5
Copy link
Collaborator

agilgur5 commented Nov 1, 2020

@xaviergonz @ezolenko following up as I still don't quite understand root cause here

@ezolenko
Copy link
Owner

ezolenko commented Nov 9, 2020

@agilgur5 I think the error was not due to uninitialized variable, but because directory itself went missing during the build.

@agilgur5
Copy link
Collaborator

agilgur5 commented Nov 9, 2020

@ezolenko thanks, yea I figured that was the issue, but I still don't get why that would occur and if this were really an optimal solution to that.

Based on the issues downstream it seems to happen when you build certain combinations of outputs with Rollup in parallel with the same cache directory for this plugin. I would think those actions would be additive to the cache as opposed to destructive and that the hashing means that a race condition shouldn't happen, but it doesn't seem to be the case.

If it indeed a race condition like that, I would think there should be a better fix that prevents the directories from going missing / being deleted / being overwritten instead of just ignoring it, since just ignoring could make for consistent cache misses and therefore poor performance. I'm not entirely sure what causes that race though or why a race would be destructive.

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