senecajs / seneca

A microservices toolkit for Node.js.
http://senecajs.org
MIT License
3.95k stars 314 forks source link

seneca.fail - allow the boolean first arg to trigger the error throw #855

Closed lilsweetcaligula closed 4 years ago

lilsweetcaligula commented 4 years ago

781

Ave, guys. Thank you for your hard work on Seneca!

This PR implements the optional cond argument, as requested in #781, that only causes the method to throw if it's true.

Q: Should the automatically updated ./test/coverage.html file be committed as well? Q: Where may I update documentation? The package.json file does not seem to have the annotate script as suggested by the readme file.

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.06%) to 93.708% when pulling 2bdb794d42cbc5ad3c915665ba90240940bb8eb8 on lilsweetcaligula:extend-seneca-fail into 183102e15c17b26bcc9728ed4832cfd5fed1c886 on senecajs:master.

lilsweetcaligula commented 4 years ago

Just a note - I am a bit concerned that, in some rare cases, current/legacy client code may pass extraneous arguments to the seneca.fail method, e.g.:

const args = ['code', { foo: 'bar' }, 1]
//
// 10 lines below

seneca.fail(...args)

Such client code will break if we "overload" the method, based on the number of arguments.

rjrodger commented 4 years ago

Thank you - this looks good!

rjrodger commented 4 years ago

Please also commit test/coverage.html - we are experimenting with committing this as standard as it feel useful

rjrodger commented 4 years ago

documentation: fail is missing from this page: https://github.com/senecajs/senecajs.org/blob/master/src/pages/api/index.html.ejs#L119 https://senecajs.org/api/#methods

lilsweetcaligula commented 4 years ago

@rjrodger Thank you for taking the time to take a look!

Would you like me to create the issues?

Please also commit test/coverage.html - we are experimenting with committing this as standard as it feel useful

documentation: fail is missing from this page: https://github.com/senecajs/senecajs.org/blob/master/src/pages/api/index.html.ejs#L119 https://senecajs.org/api/#methods

rjrodger commented 4 years ago

yes please :)

lilsweetcaligula commented 4 years ago

@rjrodger Please take a look when you have the time: https://github.com/senecajs/seneca/issues/860 https://github.com/senecajs/seneca/issues/859