Middleware does not stop on successful match

[IlyaSemenov]

When koa-better-router successfully matches a URL to a route and runs the handler, it doesn't stop and continues through Koa middleware stack. This is unlike what probably every other router (including koa-router) does.

Compare two examples:

const Koa = require('koa')
const Router = require('koa-better-router')

const router = Router({prefix: '/api'})
router.addRoute('GET', '/', ctx => {
  console.log('api called')
  ctx.body = 'api called'
})

const app = new Koa()
app.use(router.middleware())
app.use(ctx => {
    ctx.body = 'Nothing here, mate.'
})

app.listen(3000)

// curl localhost:3000/api -> Nothing here, mate. (console.log called!)
// curl localhost:3000/boo -> Nothing here, mate.

Now the same example with koa-router will work as expected:

const Koa = require('koa')
const Router = require('koa-router')

const router = Router({prefix: '/api'})
router.get('/', ctx => {
  ctx.body = 'api called'
})

const app = new Koa()
app.use(router.routes()).use(router.allowedMethods())
app.use(ctx => {
    ctx.body = 'Nothing here, mate.'
})

app.listen(3000)

// curl localhost:3000/api -> api called
// curl localhost:3000/boo -> Nothing here, mate.

Please explain what am I missing? How do I stop propagation with koa-better-router (and koa-rest-router), and what's the rationale of NOT doing that by default?

[tunnckoCore]

I believe it does not doing it by default. Probably is very specific case with the index (i.e. /) route for prefix. I'll look it as soon as i can, sorry about that.

If you switch it to something like /foo it won't have problems, in my opinion.

One more thing may worth mentioning that in practice i don't see what index route of one api may return when it does nothing - it's api, it should return some resource.

But yes, different preferences and user cases.. and the router shouldn't care about the architecture decisions.

edit: is there something actual that you can share or it's private? I may think for some workaround later.

[tunnckoCore]

And one more thing, can you replace this code

const router = Router({prefix: '/api'})
router.addRoute('GET', '/', ctx => {
  console.log('api called')
  ctx.body = 'api called'
})

to use createRoute instead, like this

const router = Router({prefix: '/api'})
const index = router.createRoute('GET', '/', ctx => {
  console.log('api called')
  ctx.body = 'api called'
})
console.log(index)

and show what the route object is logged, i have some idea what may be the problem.

[IlyaSemenov]

Probably is very specific case with the index (i.e. /) route for prefix. I'll look it as soon as i can, sorry about that. If you switch it to something like /foo it won't have problems, in my opinion.

No, I don't think it's a specific case. For example, I removed the prefix completely and left a simple route:

const router = Router()
router.addRoute('GET', '/foo', ctx => {
  console.log('/foo handler')
  ctx.body = 'This is foo!'
})

Then if I curl localhost:3000/foo, the handler is called, but the control flow is passed to the next Koa middleware so the app still responds with "Nothing here, mate".

One more thing may worth mentioning that in practice i don't see what index route of one api may return when it does nothing - it's api, it should return some resource.

Of course in the real app the router is more complex. I deliberately trimmed the example as much as possible to narrow down to the problem. I support a few open source libraries of my own and I hate when people copy/paste huge parts of their real boilerplate instead of providing a small self-sufficient piece of code that fully demonstrates the problem. You can run my code unmodified, it only needs koa@^2.3.0 and koa-better-router@^2.1.1.

to use createRoute instead, like this

The result is:

{ prefix: '/api',
  path: '/api/',
  route: '/',
  match: [Function],
  method: 'GET',
  middlewares: [ [Function] ] }

[tunnckoCore]

Hm, wait a bit. I believe it's totally normal to run next middlewares too. You can't stay with only one middleware which will be the router one.

const router = Router()

router.addRoute('GET', '/', ctx => {
  ctx.body = 'home'
})

router.addRoute('GET', '/foo', ctx => {
  ctx.body = 'foo bar'
})

const app = new Koa()
app.use(router.middleware())
app.use(ctx => {
    console.log('called for both routes of course, but you should not set body')
})

app.listen(3000)

And in many cases, router is last middleware anyway, after it may handle the errors. You don't have work after the router? Most things of api are inside the routes, before the router middleware are some global auth, body parser, cookies/sessions, etc. Not to mention that routes can have middlewares too.

[IlyaSemenov]

I believe it's totally normal to run next middlewares too.

No, it's not normal if a route specifically matched. This loses the whole point of composing middlewares. A middleware falls back to the next/underlying middleware if (and only if) it can't handle the request itself fully. Once it handles the request, the middleware stack only unwinds back/up.

Take a typical auth middleware for example. When it sees an unauthorized request, it responds with "Auth required" response and intentionally stops propagating the request further. I don't see how it's different from a matched route.

And in many cases, router is last middleware anyway, after it may handle the errors. You don't have work after the router?

In many cases, yes. But not in all cases. I do have work after the router. I put Nuxt (https://nuxtjs.org/) after my normal (API) routes which uses its internal router.

As you told yourself, the router shouldn't care about the architecture decisions. Currently, koa-better-router imposes the limitation of being the last router.

console.log('called for both routes of course, but you should not set body')

Why shouldn't I? What if I want to put a fancy 404 page as a last middleware in the stack? It will work if I use a naive plain if (ctx.path == '/foo') { ctx.body = 'foo' } else { await next() } Koa middleware, it will work with koa-router. But it will not work with koa-better-router for no reason.

Most things of api are inside the routes, before the router middleware are some global auth, body parser, cookies/sessions, etc. Not to mention that routes can have middlewares too.

Yes, I believe I grasp how the HTTP stack and middleware processing works. I contributed to Django and aiohttp development, for what it's worth.

[IlyaSemenov]

By the way, it's two related but different problems that I had to solve in my setup:

1. A matched route under /prefix falls down to subsequent middlewares - this is what this issue is about, I believe this is a bug.

2. A non-matched route under /prefix falls down to subsequent middlewares - I agree that this should be the default behaviour, but I think koa-better-router would benefit if there was an option to disable this.

I currently use a wrapper around router.middleware() that solves both problems. However, that doesn't mean that issue (1) should not be addressed.

[tunnckoCore]

Sorry, didn't know your background, except some repos in your profile :wave:

A middleware falls back to the next/underlying middleware if (and only if) it can't handle the request itself fully. Once it handles the request, the middleware stack only unwinds back/up.

Not, agree, not very true.

http://koajs.com/#application: The following example responds with "Hello World", however first the request flows through the x-response-time and logging middleware to mark when the request started, then continue to yield control through the response middleware. When a middleware invokes next() the function suspends and passes control to the next middleware defined. After there are no more middleware to execute downstream, the stack will unwind and each middleware is resumed to perform its upstream behaviour.

All of these - some old, some up-to-date, calls next if not match:

koa-bestest-router koa-path-match/lib/index.js#L66 koa-barista (pretty old) koa-routing (pretty old too) koa-route

Even, koa-better-router helps with option for handling "not found". Btw, thanks for #21, i'll see it. sounds like thing i didn't think for.

I have some assumptions that, 1) if all that isn't correct behavior; 2) i'm missing something; 3) or i'm totally wrong; that problem may be because koa-compose@3.1.0/index.js#L40 which in any way will call next middleware until you pass it noop (which btw is done in koa-trie-router/lib/Router.js#L184-L185). I'm always been thinking about that.

Even koa-router@7.x calls next (on match /master/lib/router.js#L341-L350 and on non-match (/master/lib/router.js#L332)) and uses koa-compose@3.0.0. Not sure what matchedLayers is, but it calls next() for each of them. This is the first thing, second is even if there is no matchedLayers it just passes the things to koa-compose which in turn, as i said, always will call next (/index.js#L43).

[tunnckoCore]

Even after that quick research, i still have bit of a feel that it may be something wrong here.

[IlyaSemenov]

The following example responds with "Hello World",

Absolutely. You see - it responds with "Hello World" and does not call next! Because it knows how to handle this request fully.

All of these - some old, some up-to-date, calls next if not match:

That's exactly what I would expect from koa-better-router! Call next if not match. However, koa-better-router calls next always (even on successful match).

[IlyaSemenov]

I think the problem is here: https://github.com/tunnckoCore/koa-better-router/blob/master/index.js#L547

If you replace

return utils.compose(route.middlewares)(ctx).then(() => next())

with

return utils.compose([...route.middlewares, next])(ctx) // or ES5 boilerplate for spread operator

it will work correctly.

[tunnckoCore]

If you replace

As i said. But it not make any sense, because koa-compose will call next in any way, even if we don't pass next to it. for example return utils.compose(route.middlewares)(ctx)

with

why next is in the middlewares (oh i think i got it, it will be just empty middleware that does not call its next)? The only way i see is to pass noop function, like in koa-trie-router.

Can you please edit it from the node_modules folder, and replace that line with this

const noop = () => {}
return utils.compose(route.middlewares)(ctx, noop)

[IlyaSemenov]

But it not make any sense, because koa-compose will call next in any way, even if we don't pass next to it. for example return utils.compose(route.middlewares)(ctx)

Absolutely not:

const compose = require('koa-compose')

async function m0(ctx, next) { console.log("m0"); await next(); }
async function m1(ctx, next) { console.log("m1"); ctx.body = "Hello, world"; }
async function m2(ctx, next) { console.log("m2"); await next(); }

compose([m0, m1, m2])({}, () => {
  console.log("next")
})

This will print:

m0
m1

Neither m2 or next will be called. So you're mistaken when you think that it "will call next in any way".

You seem to be missing it. koa-compose will stop processing middlewares once any of it returns a value, or a promise that doesn't depend on the next middleware. What koa-compose does itself is that it only runs a first middleware and passes it an optional next handler (pointing to the next middleware in line, or next after the last middleware), which may or may not be called by that middleware. It's not a loop! koa-compose does not run any middleware other than the first itself, nor does it call next.

why next is in the middlewares

middlewares here is the list of handlers associated with a currently matched route. next here is the next middleware in stack after koa-better-router (as passed by Koa). So when koa-better-route calls the corresponding route handler, it needs to support Koa's/koa-compose contract and pass the next middleware in stack to the currently called middleware handler in case if it decides to run it (which it most probably will not).

[tunnckoCore]

(huh, after 2 years [i totally forgot that])

Yup, agree.

[Ashenbacken]

I happen upon this problem my self today and was wondering why this happened. Found this thread and just want to ask is there any PR or update on this?