koajs / bodyparser

Koa body parsing middleware
MIT License
1.31k stars 116 forks source link

Does not play well with koa-compose #95

Closed yadwinderpaul closed 5 years ago

yadwinderpaul commented 6 years ago

App always sends 404 Not found when koa-bodyparser is wrapped inside koa-compose

{
    "koa": "^2.4.1",
    "koa-bodyparser": "^2.5.0",
    "koa-compose": "^4.0.0"
}
const Koa = require('koa')
const compose = require('koa-compose')
const bodyparser = require('koa-bodyparser')

const app = new Koa()

// app.use(bodyparser()) // works
app.use(compose([bodyparser()])) // send 404

app.use((ctx) => {
  ctx.body = 'Hello World!'
})

app.listen(3000)
dead-horse commented 6 years ago

sorry, I can't get reproduction with your code. :(

yadwinderpaul commented 6 years ago

I am running:

macOS 10.13.2
Node v8.9.0

Even doesn't runs on:

Ubuntu 16.04.3 LTS
Node v8.1.4
codinggirl commented 6 years ago

When meet this, we should check other middleware's calling of next().

If a middleware is a common function, we should call like:

return next()

But not:

next()

The solution is add keyword return and let the middleware returns the promise.

I struggle this for several hours, so write it down, may it helps.

Finally, see this article for more detail.

sergeysova commented 5 years ago

I have the same problem. And I was posted PR #102

dead-horse commented 5 years ago

@codinggirl koa-bodyparser's middleware is async function, so await next() is the same as return next() in this situation.

sergeysova commented 5 years ago

@dead-horse but in your case middleware return promise with undefined

dead-horse commented 5 years ago

I don't understand why middleware should return promise in this koa-compose case.

fl0w commented 5 years ago

This issue is invalid.

@yadwinderpaul You're using an old version of koa-bodyparse which is not compliant with Koa 2 (and thus compose 4 and 3 which is what Koa is using currently).

@codinggirl It makes no sense to break the promise flow with synchronous middleware currently. And not calling next is a completely valid use case. Having a guard in this repo (even though one could assume next is always called after this specific middleware) can be very confusing.