Koa: Restrict koa from using non-promise middlewares

[atassis]

Hello there. In koa@2 when you use the mix of normal and async functions as middlewares, the normal function behave unproperly if it doesn't return a promise. I propose to add a verification if function returns promise, so no other developers will be confused.

[fl0w]

This has been discussed in many issues, a function that doesn't return a promise is valid.

const Koa = require('koa')
const app = new Koa
app.use(ctx => {
  ctx.body = 'I am valid'
})

[SergProduction]

Yes. But it breaks previous async middlewares

[atassis]

@fl0w in addition to @SergProduction, I want to suggest some type of strict mode, when normal functions are restricted. Current implementation of Koa breaks async functions.

[sergeysova]

@fl0w Maybe restrict sync middlewares that have arity 2

Example:

// Show warning
app.use((ctx, next) => {})

// All right
app.use(async (ctx, next) => {})

// Too
app.use(ctx => {})
app.use(async ctx => {})

@atassis @SergProduction

[danwkennedy]

Sounds like this would be best done as a linter rule instead of part of the framework. It could then be configured as a warning or an error depending on your preference.

[fl0w]

@danwkennedy that was pretty much what others was saying as well in https://github.com/koajs/compose/pull/73

@SergProduction @atassis sounds like what https://github.com/koajs/koa/pull/955 does, but simplified maybe. forceAsync would be an option. Koa maintainers has not reflected opinions about it yet though - and that PR is somewhat borked.

[sergeysova]

@danwkennedy I think, framework should check that to avoid stupid mistakes, like forgotten async

[fl0w]

@LestaD Koa is not a framework - and I strongly disagree with guarding against what's basically erroneous javascript (not specific to Koa).

Edit though my opinion doesn't matter :) just adding my two cents.

[danwkennedy]

@LestaD linters will already warn you about these kinds of things:
http://eslint.org/docs/rules/require-await

[atassis]

@fl0w still, it is not obvious for even matured developers how the code from koajs.com site works. It is an individual case of using middlewares. And yes- it is not about JS itself, it is about Koa.
And, btw, it is said nearly everywhere, that Koa is a framework. Worth to mention=)

[atassis]

@danwkennedy not everyone is using this kind of linters as well as it mustn't be covering a case with function returning promises. Or is it?

[danwkennedy]

You don't have to return a promise for the middleware to be valid. Execution simply stops when the function returns and goes back up the stack. If you expect your middleware to passthrough, you do have to await or return the result of next(). So then the solution can't simply be a check for a returned promise. The solution explored in https://github.com/koajs/compose/pull/73 concluded with this is mainly just a user error that would be unperformant and hacky to check for.

[SergProduction]

@danwkennedy I think it needs to be written in the documentation so people do not puzzle why their code does not working

working async

app.use(async (ctx, next) => {
  const result = await new Promise( resolve => {
    setTimeout(() => {resolve('YES!!!') }, 1000)   
  })
  ctx.body = result
});

app.use(async (ctx, next) => {
  ctx.body = 'Hello'
  await next()
})

not working async

app.use(async (ctx, next) => {
  const result = await new Promise( resolve => {
    setTimeout(() => {resolve('YES!!!') }, 1000)   
  })
  ctx.body = result
});

app.use((ctx, next) => {
  ctx.body = 'Hello'
  next()
})

[danwkennedy]

@SergProduction That's fair. I looked over the docs website and the http://koajs.com/#cascading section could make this behavior more explicit. Either that or listing possible structures for middleware functions and what would happen when they're called.

@fl0w would you have any suggestions/comments on that? I'd be willing to PR it if you'd like.

[fl0w]

@danwkennedy I'm +0, wouldn't be opposed having a section to reference, as this issue seems to crop up.

Though I can't speak for what the maintainers wants the docs to include. If you feel like you have the time, do a PR and await (pun intended) a review - or, let this simmer a bit until the members chip in with grand opinions :)

[SergProduction]

OR

[fl0w]

@danwkennedy https://github.com/koajs/koa/issues/954 is marked as documentation, I would assume a PR would be appreciated.

[fl0w]

I'm going to close this due to inactivity. This is more of a general javascript "gotchya". As I've understood the consensus, this is at best a documentation issue and thus overlaps with #954 which is already tagged accordantly.