Better support for async handlers

[rubennorte]

Given that async functions are now standard it'd be great to have better support for them in Hapi. Specifically, using them as handlers:

server.route({
  method: 'GET',
  path: '/',
  handler: async function (request, reply) {
    const data = await getData();
    const other = await getOther(data);
    reply(other);
  }
});

We can use them now as I described but any error in the handler would become an unhandled promise rejection. Catching them in the framework instead of forcing the programmers wrap the code inside a try/catch would be nice. This is what I'd like to avoid:

server.route({
  method: 'GET',
  path: '/',
  handler: async function (request, reply) {
    try {
      const data = await getData();
      const other = await getOther(data);
      reply(other);
    } catch (error) {
      reply(error);
    }
  }
});

Would you accept a PR on this? Would you also use the result of the async function to call reply or only catch the errors?

Thanks for considering this.

[nlf]

i thought async function support was still behind the --harmony flag? if that's the case, anything like this is unlikely to be accepted until the flag is no longer required.

[cjihrig]

i thought async function support was still behind the --harmony flag?

It is.

[rubennorte]

Async functions will land in Node 8 without flags (they're available without them in V8 5.5, the version that's currently being used in Chrome stable).

I don't see a reason why this cannot be implemented now to be used with generators or transpiled code. Even with regular functions that return promises would be helpful:

server.route({
  method: 'GET',
  path: '/',
  handler: function (request, reply) {
    return getData().then(getOther).then(reply);
    // Or even: return getData().then(getOther);
  }
});

[hueniverse]

This would have to be done by me because I need to study this more.

[kanongil]

As far as I know, the only thing this request requires from hapi, is to support returning a Promise from a handler by calling reply() when they fail. This is somewhat similar to how a throw is already handled.

[rubennorte]

I reviewed the code and I think @kanongil is right. We just need to wrap the result of the handler as a promise and call reply with any errors it might throw.

@hueniverse good. I just want Hapi to support them no matter who implements it 😄

[devinivy]

While you wait it should be trivial to write a wrapper used like,

server.route({
  method: 'GET',
  path: '/',
  handler: Safely.catch(async function (request, reply) {
    const data = await getData();
    const other = await getOther(data);
    reply(other);
  })
});

[jeff-kilbride]

I'm trying to achieve the same basic thing, but I also wanted centralized error handling for all of my handlers without having to manually call some sort of utility error handling function. I'm new to Hapi and just wondering if I'm going about this the right way. I'm using what I have gleaned from looking at hapi-plugin-co to wrap all handlers using onPreStart. Also, I'm using haute-couture to wire my plugins. Here's what I have:

plugins/async-handlers/extensions.js:

'use strict';

const Boom = require('boom');


module.exports = [{
    type: 'onPreStart',
    method: function (server, next) {

        // Iterate over all connections.
        const connections = server.table();
        for (let conn of connections) {

            // Iterate over all routes in each connection.
            for (let route of conn.table) {
                if (typeof route.settings.handler !== 'function')
                    continue;

                // Store the original handler function.
                const h = route.settings.handler;
                route.settings.handler = (request, reply) => {

                    // Call the original handler.
                    const p = h(request, reply);

                    // If the handler returns a promise, attach a `catch()` method.
                    if (p && p.catch) {
                        p.catch((e) => {

                            // Ensure the error is a Boom object and capture a message
                            // for the logs.
                            let msg;
                            if (e instanceof Error) {
                                msg = e.stack;
                                if (!e.isBoom)
                                    e = Boom.wrap(e);
                            }
                            else {
                                msg = String(e);
                                e = Boom.create(500, msg);
                            }

                            // Log it.
                            request.log(['error', 'uncaught'], msg);

                            // Explicitly call the reply() method to handle the error.
                            reply(e);
                        });
                    }
                };
            }
        }

        next();
    }
}];

An example plugin with an async handler:

plugins/hello/routes.js:

'use strict';

const Boom = require('boom');


module.exports = [
    {
        method: 'GET',
        path: '/hello/{name?}',
        config: {
            handler: async function (request, reply) {
                const hello = await _asyncFunc(request.params.name);

                reply(hello);
            }
        }
    },
    {
        method: 'GET',
        path: '/goodbye/{name?}',
        config: {
            handler: function (request, reply) {
                const goodbye = `Good-bye ${request.params.name || 'World'}`;

                reply(goodbye);
            }
        }
    }
];


// Private methods.
function _asyncFunc(name) {
    return new Promise((resolve, reject) => {
        if (name === 'Joe')
            reject(Boom.badRequest(`Bad Parameter: ${name}`));
        else
            setTimeout(() => resolve(`Hello ${name || 'World'}`), 2000);
    });
}

I'm using Glue to compose the server and add the plugins, and I'm currently just running it through babel-register with the following in my .babelrc:

{
  "presets": [
    ["env", {
      "targets": {
        "node": "current"
      }
    }]
  ]
}

This seems to be working. When I hit the /hello route with anything but Joe, the server waits two seconds and then responds. When I hit it with Joe, I get:

Debug: handler, error 
    {"msec":2.306112051010132,"error":"Bad Parameter: Joe","data":{"data":null,"isBoom":true,"isServer":false,"output":{"statusCode":400,"payload":{"statusCode":400,"error":"Bad Request","message":"Bad Parameter: Joe"},"headers":{}}}}

in the logs and the correct error response in the browser. I added the /goodbye route just to make sure it was working with non-async functions. However, I would like some feedback. I'm wondering if there is a better (more Hapi or more idiomatic Javascript) way to accomplish this. If somebody can confirm it works the way I think it does, maybe it will help others who are looking to use async...await syntax in their handlers.

(I apologize for the length...)

-- EDIT --

I've cleaned it up quite a bit over the last few days, as I've looked at more examples. (specifically the hapi-es7-async-handler module) I was able to remove the bluebird dependency after I realized that wrapping an async function in Promise.resolve was redundant. The above is working well for me, now. The thing I like most is having a single spot to customize / standardize any errors thrown by my async handlers.

[sirgallifrey]

Based on this conversation I made a little plugin to handle this issue, but I saw there are already plugins for that when I was going to upload on npm, so I'm just leave my silly code and the npm one. Hope this help you guys :)

[cjnqt]

An async function always return a promise. So making Hapi handlers support async functions === making Hapi handlers support Promise-returning functions.

[jeff-kilbride]

@cjnqt Yeah, I'm learning as I go. I've primarily been using generators / yield and bluebird's coroutine for handling async stuff up to now. This is my first foray into using the async / await syntax with babel. I've edited my example above to make the Promise handling less redundant.

[rubennorte]

Just for you to know, async-await has officially landed in Node.js 7.6.0.

[devinivy]

@jeff-kilbride it looks pretty good to me. One concern– I'd avoid modifying the route public interface at runtime (I realize that is the approach of hapi-plugin-co). While it seems to work right now for handlers, my impression is that there's no hard guarantee it will continue to work that way in the future. I'd opt to instead create a handler type with server.handler().

Also a question! Is there a reason you'd like to centralize error handling specifically for async handlers? If you're looking for a more general-purpose solution, centralized error handling in hapi is typically done via a onPreResponse request extension. Happy to elaborate on that in hapijs/discuss but the docs also have a nice section on this.

[jeff-kilbride]

@devinivy thanks for the comments. i've read the server.handler() docs and am aware of how they work. i will take another look at that.

As far as the error stuff, i guess it would be better phrased as centralized error catching rather than error handling. Using generators in express, i was doing a lot of try...catch stuff inside my handler code. So, i would have:

foobar: co(function *(req, res) {
    try {
        [... handler code ...]
    }
    catch(e) {
        handleError(res, e);
    }
})

With the setup above, i don't need the try...catch statements in the handlers anymore. I still use onPreResponse to convert some errors to html responses, etc...