Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Everything returns 404 with Koa #174

Closed
kaelzhang opened this issue Mar 6, 2017 · 3 comments
Closed

Everything returns 404 with Koa #174

kaelzhang opened this issue Mar 6, 2017 · 3 comments

Comments

@kaelzhang
Copy link
Contributor

kaelzhang commented Mar 6, 2017

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

kaelzhang pushed a commit to kaelzhang/webpack-dev-middleware that referenced this issue Mar 6, 2017
kaelzhang pushed a commit to kaelzhang/webpack-dev-middleware that referenced this issue Mar 6, 2017
kaelzhang added a commit to kaelzhang/webpack-dev-middleware that referenced this issue Mar 9, 2017
kaelzhang added a commit to kaelzhang/webpack-dev-middleware that referenced this issue Mar 9, 2017
@javiertury
Copy link

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();
  }); 
});

@kaelzhang
Copy link
Contributor Author

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

@kaelzhang
Copy link
Contributor Author

Fixes in #175

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants