setHeader method may be set incorrectly

[MikeMangum]

The method used to set the header won't work for mocked requests:

var setHeader = res.set ? http.OutgoingMessage.prototype.setHeader : res.setHeader

Should probably be something along the lines of:

var setHeader = res.setHeader ? res.setHeader : http.OutgoingMessage.prototype.setHeader

[dougwilson]

The issue is that if it is reversed it will break what the workaround is for. There are some frameworks with a broken / different res.set , so this uses the original instead when it is there. But they have a (broken in the same way) res.setHeader as well.

[dougwilson]

This is the most recent comment that made the current implementation for reference: https://github.com/pillarjs/cookies/commit/e3b12470ed7c6169c582bcd206325e64411c283e

I will see if I can dig up the original issue / PR as well when I get back to a computer.

[dougwilson]

PR: https://github.com/pillarjs/cookies/pull/19

[MikeMangum]

My concern is that this is a workaround for a problem with a specific implementation of setHeader function but it is not targeted to that specific implementation.
var setHeader = !(res.__proto__.app === undefined) ? http.OutgoingMessage.prototype.setHeader : res.setHeader

This would use the setHeader function of the passed in ServerResponse instance except in the specific case that it is express' implementation, which (if I understood the issue correctly) is the issue fixed in that PR.

[dougwilson]

I agree it is bad and I'm not sure i would have landed that pr if it were i, but it has been landed and is now part of the api. If you are ok with waiting a bit for me to get together a major release we can just remove it, otherwise just need a change that will not break folks .

[MikeMangum]

Sounds good.

[mrfitz42]

How goes the progress on this? I'm working on unit tests for a system we are upgrading from node 4.4.7 and have found that this is causing an exception when using node-mocks-http to create res.