Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
andrewc89 opened this issue Mar 23, 2018 · 2 comments
Open

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

andrewc89 opened this issue Mar 23, 2018 · 2 comments

Comments

@andrewc89
Copy link

andrewc89 commented Mar 23, 2018

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
Copy link
Member

gyson commented Mar 23, 2018

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants
@andrewc89 @gyson and others