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

throw vs cb if value #4

Closed
stevemao opened this issue Jun 15, 2015 · 7 comments
Closed

throw vs cb if value #4

stevemao opened this issue Jun 15, 2015 · 7 comments

Comments

@stevemao
Copy link

If I type brightness 2 the whole stack is blowing in my console

Error: Expected a value between 0 and 1
    at Object.exports.set (/usr/local/lib/node_modules/brightness/node_modules/osx-brightness/index.js:73:9)
    at Object.<anonymous> (/usr/local/lib/node_modules/brightness/cli.js:27:12)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Function.Module.runMain (module.js:501:10)
    at startup (node.js:129:16)
    at node.js:814:3

Any reason this is throwing an error not cb with an error?

    if (typeof val !== 'number' || isNaN(val) === 'true' || val < 0 || val > 1) {
        throw new Error('Expected a value between 0 and 1');
    }
@kevva
Copy link
Collaborator

kevva commented Jun 15, 2015

It's sort of a type annotation thing. But yeah, maybe we could fallback gracefully setting it to 1 if >1.

@stevemao
Copy link
Author

I would like to see the error (at least a warning if you wanna fallback). If you have a good reason to throw maybe just catch it?

@kevva
Copy link
Collaborator

kevva commented Jun 15, 2015

Yeah, in the CLI that makes sense.

@stevemao
Copy link
Author

@kevva Just out of curiosity, do you know any link that tells you how to handle errors? This is a perfect example I think I would cb the error. Or can you elaborate why you should definitely throw the error here.

@stevemao
Copy link
Author

if (typeof val !== 'number' || isNaN(val) === 'true' || val < 0 || val > 1)

Is probably checking too much at the same time. If it's a type error it's definitely throw

@kevva
Copy link
Collaborator

kevva commented Jun 15, 2015

99% of the times, cb is correct. But for input errors throwing can be good. This is sort of a type error but not exactly so I guess it could be changed.

@stevemao
Copy link
Author

👍 exactly what I think

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