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

Transform error should not kill build #48

Closed
mattdesl opened this issue Feb 13, 2016 · 6 comments
Closed

Transform error should not kill build #48

mattdesl opened this issue Feb 13, 2016 · 6 comments

Comments

@mattdesl
Copy link
Contributor

Getting this when using an invalid filename to sheetify function:

assert.js:89
  throw new assert.AssertionError({
  ^
AssertionError: filename must be a string
    at parseCss (/projects/mattdesl.github.com/node_modules/sheetify/index.js:31:10)
    at sheetify (/projects/mattdesl.github.com/node_modules/sheetify/index.js:23:17)
    at nr (/projects/mattdesl.github.com/node_modules/browserify/node_modules/module-deps/index.js:297:23)
    at /projects/mattdesl.github.com/node_modules/browserify/node_modules/resolve/lib/async.js:44:21
    at ondir (/projects/mattdesl.github.com/node_modules/browserify/node_modules/resolve/lib/async.js:187:31)
    at onex (/projects/mattdesl.github.com/node_modules/browserify/node_modules/resolve/lib/async.js:93:22)
    at /projects/mattdesl.github.com/node_modules/browserify/node_modules/resolve/lib/async.js:24:18
    at FSReqWrap.oncomplete (fs.js:82:15)

And it kills budo and other tools. Instead, might be better for the transform to emit an error event so budo and similar tools can catch it and display this error in the browser.

@mattdesl
Copy link
Contributor Author

Hmm, I think maybe this issue is a bit different. I get the error even when my path is correct

@mattdesl
Copy link
Contributor Author

D'oh! I'm a tool. I was using -t sheetify instead of -t sheetify/transform

@mattdesl
Copy link
Contributor Author

I've noticed some other errors like #49 are killing the build, so I will re-open this issue since it's mostly about not killing the build on errors. 😄

@mattdesl mattdesl reopened this Feb 13, 2016
@yoshuawuyts
Copy link
Contributor

Ah yes, you're fully right I'll make sure #49 emits an error (let's see if it can be tested properly ^^) Thanks for raising!

@yoshuawuyts
Copy link
Contributor

Cool, so I've created patch for this that should at least catch this particular error - I feel some refactoring might be in place to make this more solid. I'll upload it once I get home because I'm silly and don't have a VPN that can circumvent cafe firewall 😕 haha

@yoshuawuyts
Copy link
Contributor

Should be resolved in #52 (mostly, I hope we're not accidentally throwing for more runtime errors - hah) Thanks!

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