koajs / koa

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

ctx.body=null may make some confusing things #1317

Closed JhinDeng closed 5 years ago

JhinDeng commented 5 years ago

I found some code in the test case and added a line to describe the problem I encountered.

it('should respond 204 with status=200', async () => {
      const app = new Koa();

      app.use(ctx => {
        ctx.status = 200;
        ctx.body = null;

        // When I try to set ctx.body repeatedly, 
        // even if the value is not Null, it will still return 204
        ctx.body = { hello: 'world' }; 
      });

      const server = app.listen();

      const res = await request(server)
        .get('/')
        .expect(204)
        .expect('');

      assert.equal(res.headers.hasOwnProperty('content-type'), false);
    });

the reason is https://github.com/koajs/koa/blob/master/lib/response.js/#L83-L92 https://github.com/koajs/koa/blob/master/lib/response.js/#L134-L145

This problem may be good to avoid, but if you don't pay attention, it will cause troublesome problems. So I posted the problem I encountered here and it might help others save some time.

fl0w commented 5 years ago

I’m on a business trip but this is a duplicate. This issue is understood but the problem is that changing this behavior is a major server version (breaking)

JhinDeng commented 5 years ago

@fl0w Thanks for the reply, if the official support is the best.

fl0w commented 5 years ago

So the thing here is that once you change status, Koa doesn't overwrite it. It only sets status if missing or if you're using ctx.(throw|assert). This isn't a bug - but rather a side effect and perhaps a docs issue. Once you understand how Koa reasons about this, it shouldn't be a problem.

JhinDeng commented 5 years ago

ok, I've got it. thanks.