koajs / koa

Expressive middleware for node.js using ES2017 async functions
https://koajs.com
MIT License
35.2k stars 3.23k forks source link

Keep receiving 404 "Not Found" with promises in url handler #1305

Closed zefirka closed 5 years ago

zefirka commented 5 years ago

My trouble is pretty similiar to #886 and #881, but I'm using async function creator.

function wrapCall(requestCreator) {
    return async function handler(ctx) {
        try {
            const response = await requestCreator(ctx);
            ctx.status = 200;
            ctx.body = {
                ...response,
                ok: true,
            };

        } catch (error) {
            ctx.throw(error.code, error.message);
        }
    };
}

api.get('/operators/', koaBody, wrapCall((ctx) => {
    return adapter.pull('operators', apiModel.getAviableOperatorsList()); // returns promise
}));

I'm setting status=200 but koa sends 404 Not found. I'm wrapping function that returns promise so I'm expecting that wrapped function will be correctly works in resulting handler.

What am I doing wrong?

ehsankhfr commented 5 years ago

Your example looks more like Express.js! You should think differently while using KoaJs:

const Koa = require('koa')
const bodyParser = require('koa-bodyparser')
const Router = require('koa-router')
const app = new Koa()

const router = new Router()

app.use(bodyParser())

function wrapCall(requestCreator) {
    return async (ctx) => {
        try {
            const response = await requestCreator(ctx)
            ctx.status = 200
            ctx.body = {
                ...response,
                ok: true,
            }

        } catch (error) {
            ctx.throw(error.code, error.message)
        }
    }
}

router.get('/operators/', wrapCall((ctx) => {
    return Promise.resolve({
        message: "hello world!"
    })
}))

app
    .use(router.routes())
    .use(router.allowedMethods())

app.listen(3000, (err) => {
    if (err) {
        throw err
    }

    console.log('Server started on port 3000')
})
fl0w commented 5 years ago

I'm in a weird spot so I can't try your example, but are you sure the route matches? Can you achieve expected results without the wrapper function?

fl0w commented 5 years ago

To expand @ehsankhfr's answer, I wouldn't wrap each mw but instead create a mw that handles response as the promise resolves. Here's a quick and dirty example of how that would work:

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

const app = new Koa()
const router = new Router()

async function responseHandler (ctx, next) {  
  try {
    await next()
  } catch (err) {
    ctx.throw(err.code, err.message)
  }

  // Ignore post-processing if body was already set
  if (!ctx.state.response || ctx.body) return

  ctx.status = 200
  ctx.body = {
    ...ctx.state.response,
    ok: true
  }
}

router.get('/operators', async ctx => {
  ctx.state.response = await Promise.resolve({
    message: "hello world!"
  })
})

app
  .use(responseHandler)
  .use(router.routes())
  .use(router.allowedMethods())

app.listen(process.env.PORT, err => {
  if (err) throw err
  console.log(`Server started on port ${process.env.PORT}`)
})

Note that we're not relying on the returned promise value, instead we're setting it to context aware state object.

I'm going to close this for now but if you're still experiencing issue feel free to re-open this.