koajs / koa

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

don't set header when `respond=false` and header sent #1044

Closed dead-horse closed 5 years ago

dead-horse commented 7 years ago

many people using next.js or migrate there app from express will set respond=false, but lots of koa's middleware will set header like:

async (ctx, next) => {
    await next();
    ctx.set('foo', 'bar');
}

it will cause Can't set headers after they are sent error if we already sent response body before. Maybe we can make ctx.set() invalid when respond=false and header sent. (We can log something when NODE_ENV is develop to hint developer this unexpected behavior)

related issues:

Any thoughts?

nordsimon commented 7 years ago

I just saw your reference @dead-horse. From my point of view, respond.false is very useful and sometimes neccessary in specific implementations. I would go for logging a warning in develop (or via debug) module mode but still allow and fix the behaviour. Maybe add some more info what side effects respond false has ?

nordsimon commented 7 years ago

And yes, I see that this should actually live in koa and not in koa-session, I will close my PR in order to find a solution here

dead-horse commented 5 years ago

this is already fixed in https://github.com/koajs/koa/pull/1137