hapijs / glue

Server composer for hapi.js
Other
245 stars 62 forks source link

Callback is not called when options or manifest Assertion fails #90

Closed valdemon closed 7 years ago

valdemon commented 7 years ago

The assertion of options and manifests should be wrapped in try...catch clause due to call the callback(err) in case of an error.

Following code:

Joi.assert(options, internals.schema.options, 'Invalid options');
Joi.assert(manifest, internals.schema.manifest, 'Invalid manifest');

should be wrapped by try...catch and relocated after the optional Promise wrapping.

Marsup commented 7 years ago

Unless he wants it to crash the process, which seems like it, just like the assert 2 lines above.

devinivy commented 7 years ago

The idea is that if these assertions don't hold it is an unrecoverable programmer error, not dissimilar in character to what would happen if you tried to call undefined as a function, or read a property of null. So the program should crash and indicate to you to adjust your options or manifest.

valdemon commented 7 years ago

Ok, got it :) I just thought when one provides a contract with the error, either provided by a callback or promise then I can rely on it - means, that I'm able to handle the error in the right place (callback function, or promise catch clause. You could be right in case of some dummy programming, when one would ignore error objects given according to mentioned contract. So i understand you were worried about such situations, right?

devinivy commented 7 years ago

Ultimately I defer to @csrl, but that's my interpretation of it :)

valdemon commented 7 years ago

Ok, thanks for the explanation :) Anyway, also thanks for this cool & helping library! Closing the issue.

csrl commented 7 years ago

It is how hapi code is written - wrong types / schema validation failures generally throw exceptions. Its considered a non recoverable programming error. I've no strong opinion on it, but grep the hapi code for assert and you'll see its the common practice.

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.