webpack/webpack-dev-middleware

Everything returns 404 with Koa

kaelzhang opened this issue · 3 comments

Reproduce

// To reproduce this problem, I made this tiny module 
// to convert express middleware to koa2
const convert = require('express-to-koa')

const Koa = require('koa')
const app = new Koa
app.use(convert(devMiddleware))

Then all response statuses are 404.

Reason

We should not set statusCode explicitly, see here

// Express automatically sets the statusCode to 200, but not all servers do (Koa).
res.statusCode = res.statusCode || 200;

Actually, koa automatically set the statusCode to 404 by default (see koa2, and koa), but the default setting of statusCode does not change the this._explicitStatus, so ctx.body = something will set the statusCode to 200 (here).

But doing res.statusCode = res.statusCode || 200 makes all responses 404, because it will change this._explicitStatus to true.

Conclusion

  • We should delete this line.
  • Http status should not be handled by this middleware, but the converter instead. Because this is an express middleware, not a koa one. The converter which converts express middlewares into koa middlewares should handle this common requirement.

Related Issue

  • #148
  • #94, this problem should not be solved like this, which leads to further problems.
  • #30

Here is an ugly but functional wrapper to workaround the issue if you are using Koa.

Be careful, you must set ctx.res.statusCode, not ctx.status or ctx.res.status or anything else. This is the best way to stop it from interfering with the rest of middlewares and routes.

Depending on your setup you could still come across some strange behavior, specially if using on of those complex webpack hot middleware setups. An example of strange behavior may be that some of your assets being served(response body set) but with a 404 response code.

koa.use(async function (ctx, next) {
  let prevStatus = ctx.res.statusCode;
  ctx.res.statusCode = 200;
  await c2k(devMiddleware)(ctx, function() {
    ctx.res.statusCode = prevStatus;
    return next();
  }); 
});

@javiertury Yes, it is a temporary workaround. And we also need a final solution.

Fixes in #175