8000 error recovery · Issue #2 · talmobi/wrollup · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

error recovery #2

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.

Sign up for GitHub

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

Jump to bottom
Closed
FND opened this issue Oct 5, 2016 · 9 comments
Closed

error recovery #2

FND opened this issue Oct 5, 2016 · 9 comments

Comments

@FND
Copy link
FND commented Oct 5, 2016

I saw you mentioning wrollup at rollup/rollup-watch#16 (comment), so thanks for sharing!

Unfortunately, I'm observing similar issues as described in rollup/rollup-watch#7 (comment):

After introducing a syntax error, wrollup reports "SyntaxError" with an excerpt of the source code. Immediately afterwards it reports numerous "RangeError: Maximum call stack size exceeded" exceptions at node_modules/rollup/dist/rollup.js:165:18, from which it doesn't seem to recover - i.e. the bundle won't recompile after fixing the syntax error without restarting the wrollup process.

It appears that --nocache might fix this, but that's of course too slow for regular use.
@talmobi
Copy link
Owner
talmobi commented Oct 5, 2016

Hello!

Hmm, indeed there seems to be something funky going on with the way rollup re-uses cached bundles.

I tried your reduced test case in the link you mentioned but didn't really notice anything out of the ordinary, the thread you linked seem to deal with being unable to recover from initial builds?

Could you perhaps make another reduced test case of your issue that I could explore a bit? Thanks so much!

@talmobi
Copy link
Owner
talmobi commented Oct 5, 2016

Hello again!

I updated rollup to version 0.1.7 which adds a --verbose flag so you can see more clearly what wrollup is doing.

Also fixed a glob pattern issue on the global recovery watcher. Wrollup temporarily creates a global watcher to recover from failures by itself when rollup fails -- however, the pattern I had used I was **/**/*.js? which works for .jsx files but not .js files, also the extra recursion was unnecessary so I switched to **/*.js* which seems to work as intended now.

However, this still shouldn't do anything to fix your call stack exceed error with cached bundles, that's still a bit of a mystery that I haven't been able to reproduce.

@FND
Copy link
Author
FND commented Oct 5, 2016

Thanks for the quick response!

I believe I've managed to create a reduced test case:
https://gist.github.com/FND/6ed5a6fa6c2dfa64597c47422294ef7e
Hopefully this helps your analysis.

@talmobi
Copy link
Owner
talmobi commented Oct 5, 2016

Interesting! I couldn't reproduce the error at all when running wrollup directly from source, but when I tried to run it from the local node_modules (as an npm script), then the RangeError: Maximum call stack size exceeded errors occurred.

The reason was that my local source code (git clone this repo) was using rollup version 0.34.10 which produced no call stack errors

see: https://github.com/talmobi/wrollup/blob/master/package.json#L27

but in the test case project I was using the latest rollup version 0.36.1 (and wrollup will use your local rollup version, it does not include rollup inside itself)

Could you try install rollup version 0.34.10 and see if the issue still exist?

npm install rollup@0.34.10

Made a small unrelated update to wrollup as version 0.1.18 also that properly removes old watcher when reattaching watcher on error recovery.

@talmobi
Copy link
Owner
talmobi commented Oct 5, 2016

I made a test case explaining the setup to reproduce the error: https://github.com/talmobi/wrollup_issue_2/tree/master

@FND
Copy link
Author
FND commented Oct 5, 2016

You're right: wrollup v0.1.8 recovers nicely with Rollup v0.34.10 (also on my actual application). So downgrading Rollup is a good interim solution; thank you!

This was referenced Oct 5, 2016
Open
Closed
@FND
Copy link
Author
FND commented Oct 6, 2016

Thanks for your thorough investigation over at rollup/rollup-watch#7 - I very much appreciate your efforts!

@talmobi
Copy link
Owner
talmobi commented Oct 6, 2016 •
edited
Loading

Sure! Also I created a workaround for wrollup v0.1.9 that should work even with newer versions of rollup.

The workaround creates a lazy backup bundle for use in the next bundle generation (lazily so it doesn't affect build times) (Similarly to what @jo-sm did here: rollup/rollup#1010). Since the deepClone method that rollup 0.35.0 and onwards uses somehow mutates the cache itself oddly -> if we give it a fresh copy of the bundle it seems to work fine.

But creating this fresh copy using JSON.parse(JSON.stringify(bundle)) during the bundle generation (or just before saving to disk) is expensive I try and do it afterwards so it doesn't affect the bundle generation time. Only a few added lines of code to the wrollup source, see here: 8c1be80

@FND
Copy link
Author
FND commented Oct 7, 2016

Excellent - that seems to work quite well, great job!

I'll go ahead and close this issue, as from a user's perspective, it's all good now.

@FND FND closed this as completed Oct 7, 2016
@talmobi talmobi mentioned this issue Nov 16, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@FND @talmobi
0