This small change allows an extra degree of protection for those who might be using promises in their handlers.
Problem Statement
Developers who are using promises in their handlers will often write handlers which look like this:
function handler(request, reply) {
doSomething
.then((interimResult) => {
return somethingElse(interimResult);
})
.then((result) => {
reply(result.whatever);
});
The challenge is that this opens up lots of opportunities for exceptions at different points to lead to uncaught promise rejections. For example:
function handler(request, reply) {
doSomething
.then((interimResult) => {
throw new Error('whoops...');
})
.then((result) => {
reply(result.whatever);
});
will lead to a timeout of the request.
Suggested Change
Rather than attempting some kind of large scale change which adds more support for promises (which is not needed, as reply
can take a promise), I propose allowing the handler to return a promise. If the promise is returned, then hapi can register a catch
and reply
with any error. This means that if the developer goofs up and forgets to catch (all to easy with the promise spec), hapi at least will attempt to deal with the error in the same way it would as if an exception was thrown.
This PR adds the support, updates the docs and tests.
Questions
Do we need this?
It is possible to rewrite the dodgy code as:
function handler(request, reply) {
reply(doSomething
.then((interimResult) => {
return somethingElse(interimResult);
})
.then((result) => {
return result.whatever;
}));
However this tends to feel a little unnatural, as the natural reading order is more 'do work, reply with the result'. Fairly subjective but I've seen quite a few devs on teams default to doing it the way described in the problem statement, rather than this. It also makes doing things like changing the status code conditionally based on the result near impossible. (CMIIW!)
Of course, people could just avoid promises (which might not be a bad idea given some of the issues around how they can swallow exceptions), but at least an appreciable proportion of people are using them heavily.
Should we ever have 'partial promise' support
Really, if you are returning a promise in the handler it might make more sense to simply not even have the reply
function used, and change the spec to say if you return a promise, hapi with reply with the result or if there is an error, wrap it in Boom and then reply with that
.
This would perhaps be cleaner in some cases:
function handler(req, reply) {
return doSomething().then(r = doSomethingElse(r));
}
But is a larger structural change. Also, it still means we cannot easily change status codes, add headers etc.
I think this is a happier middle group - hapi's API stays the same, but it has a 'safety net' for promises.
That's It!
I'd love to get any feedback on this as a potential feature. It would certainly help myself and my colleagues on some of the projects we're on at the moment, I believe it allows us to provide a safety net for potentially nasty issues (promise black holes are a pain) but am happy to take any input for changes!
Related Issues
- https://github.com/hapijs/hapi/issues/2097
- https://github.com/hapijs/hapi/issues/2771
- https://github.com/hapijs/hapi/issues/3242
- https://github.com/hapijs/hapi/issues/3429
feature bug