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

Return rejected promise #2

Closed
simplesmiler opened this issue Jul 28, 2015 · 6 comments
Closed

Return rejected promise #2

simplesmiler opened this issue Jul 28, 2015 · 6 comments

Comments

@simplesmiler
Copy link

If the promise, returned by plugin, is rejected, broccoli fails without proper message, but with the following:

Error: ENOTEMPTY, directory not empty
    at Error (native)
    at Object.fs.rmdirSync (fs.js:711:18)
    at rmkidsSync (<PLUGIN_ROOT_HERE>\node_modules\broccoli-plugin\node_modules\quick-temp\node_modules\rimraf\rimraf.js:247:11)
    at rmdirSync (<PLUGIN_ROOT_HERE>\node_modules\broccoli-plugin\node_modules\quick-temp\node_modules\rimraf\rimraf.js:237:7)
    at fixWinEPERMSync (<PLUGIN_ROOT_HERE>\node_modules\broccoli-plugin\node_modules\quick-temp\node_modules\rimraf\rimraf.js:150:5)
    at rimrafSync (<PLUGIN_ROOT_HERE>\node_modules\broccoli-plugin\node_modules\quick-temp\node_modules\rimraf\rimraf.js:216:26)
    at <PLUGIN_ROOT_HERE>\node_modules\broccoli-plugin\node_modules\quick-temp\node_modules\rimraf\rimraf.js:245:5
    at Array.forEach (native)
    at rmkidsSync (<PLUGIN_ROOT_HERE>\node_modules\broccoli-plugin\node_modules\quick-temp\node_modules\rimraf\rimraf.js:244:26)
    at rmdirSync (<PLUGIN_ROOT_HERE>\node_modules\broccoli-plugin\node_modules\quick-temp\node_modules\rimraf\rimraf.js:237:7)

Win7 x86_64, node 0.12.5.

@joliss
Copy link
Member

joliss commented Jul 29, 2015

Could this be a variation of isaacs/rimraf#72?

@simplesmiler
Copy link
Author

I guess it is closely related, but why does broccoli tries to delete files before printing the error?
The issue I'm facing is the lost error message.

@joliss
Copy link
Member

joliss commented Jul 31, 2015

why does broccoli tries to delete files before printing the error?

Ah, I see the problem - the cleanup happens before the error is reported, so the cleanup error "wins". It's a bit tricky to change this without silently swallowing cleanup errors, or adding a bunch of extra error handling code.

I'd like to suggest that it's actually acceptable to print the cleanup error rather than the build error. If a cleanup error happens, that's so fundamental an issue that it should usually be fixed first. I realize that this is happening a lot on Windows because of isaacs/rimraf#72, but then again perhaps one could say that Broccoli is simply broken on Windows until isaacs/rimraf#72 is fixed, and this is just one of many symptoms of this brokenness.

@simplesmiler
Copy link
Author

But would not additional .catch before .finally suffice?

@simplesmiler simplesmiler mentioned this issue Nov 2, 2015
@joliss
Copy link
Member

joliss commented Nov 2, 2015

I've thought about this some more, and I don't think the behavior should be changed. If there's a cleanup error, then that error should be reported.

@joliss joliss closed this as completed Nov 2, 2015
@simplesmiler
Copy link
Author

Fair enough. The isaacs/rimraf#72 is painful.

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

No branches or pull requests

2 participants