koajs / convert

Convert koa generator-based middleware to promise-based middleware
MIT License
254 stars 21 forks source link

convert.back() not respecting return from function? #10

Open andrewc89 opened 6 years ago

andrewc89 commented 6 years ago

I am working on a Koa v1 app and would like to implement new routes with async/await instead of generator functions. I found this library has convert.back() which is mostly working for me. However, the following example code will illustrate my problem:

router.get('/endpoint',
async function(next) {
    const token = this.request.headers.token;
    if (!token) {
        this.response.status = 401;
        return;
    }
    // I assume next is a Promise so this is not an issue?
    await next;
},
async function(next) {
    // This code still executes, even if there is no token defined on the headers.

    // Do something important that shouldn't execute with invalid auth...
});

return convert.back(router.routes());

If there is no token defined, the endpoint does return a 401 and exits the first function, but the second function is still executed. If I set up the first function as a generator function and use yield next, it works as would be expected.

Versions:

node: 6.13.1 npm: 3.10.10

├─┬ koa@1.6.0 │ ├── koa-compose@2.5.1 │ ├── koa-is-json@1.0.0 ├─┬ koa-convert@1.2.0 │ └── koa-compose@3.2.1 ├── koa-json-logger@1.0.2 ├─┬ koa-router@5.4.2 ├─┬ koa-socket@4.4.0 │ ├── koa-compose@3.1.0 ├─┬ koa-static-server@0.1.8 │ └─┬ koa-send@1.3.1

gyson commented 6 years ago

I think koa-router@5.4.2 is still using koa-v1 signature (generator function) and does not support koa-v2+ signature (async await function).

How about using koa-router@7 (it support koa-v2+ signature) with following code ?

// koa-router@7.4.0
router.get('/endpoint',
async (ctx, next) => {
    const token = ctx.request.headers.token;
    if (!token) {
        ctx.response.status = 401;
        return;
    }
    await next();
},
async (ctx, next) => {
    // This code still executes, even if there is no token defined on the headers.

    // Do something important that shouldn't execute with invalid auth...
});

// `router.routes()` will return async-await style middleware in koa-router@7
// `convert.back` will convert async-await style middleware to generator function style middleware
// then it can be used for koa-v1 app.
return convert.back(router.routes());

Or if you have to use koa-router@5.4.2, you can convert each individual async functions like following:

// koa-router@5.4.2
router.get('/endpoint',
convert.back(async (ctx, next) => {
    const token = ctx.request.headers.token;
    if (!token) {
        ctx.response.status = 401;
        return;
    }
    // I assume next is a Promise so this is not an issue?
    await next();
}),
convert.back(async (ctx, next) => {
    // This code still executes, even if there is no token defined on the headers.

    // Do something important that shouldn't execute with invalid auth...
}));

// no need `convert.back` here because it's already koa-v1 style middleware
// and it can be consumed by koa-v1 app directly.
return router.routes();
andrewc89 commented 6 years ago

Thanks for the quick response. Makes sense!

I think the easiest router forward at this point is to just stick with 5.4.2 and wrap them all until I have the time to upgrade to v7 and test everything.

Was there somewhere I could have found this in documentation? I was struggling to find info about properly implementing the convert.back() feature. This issue will at least exist for anyone stuck in this situation in the future.