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

matchHeader doesn't work in Node 8.x #925

Closed
panva opened this issue May 31, 2017 · 12 comments
Closed

matchHeader doesn't work in Node 8.x #925

panva opened this issue May 31, 2017 · 12 comments

Comments

@panva
Copy link

panva commented May 31, 2017

matchHeader no longer functions as expected, neither when providing value as a string or a function. The culprit seems to be options.getHeader returning undefined for every header name because request._headers (v9.0.13) is not set anymore.

@kevinoid
Copy link

I'm seeing this issue as well. It looks like this is a result of nodejs/node#10941 (nodejs/node@b377034359a specifically) which made the ._headers property of OutgoingMessage return a copy of the headers object. This copy is assigned to in setHeaders, which has no lasting effect.

Is there a reason why request.setHeader (OutgoingMessage.prototype.setHeader) isn't being called here?

@jhuliano
Copy link

jhuliano commented Jun 7, 2017

Replacing the matchHeader function with nock(host, { reqheaders: {"header":"content"} }), seems to do the trick for now.
I'm using Nock 9.0.13 and Node 8 (on Official Docker Image)

@jdx
Copy link

jdx commented Jun 7, 2017

that requires you to specify all the headers though, right?

@jhuliano
Copy link

jhuliano commented Jun 7, 2017

@dickeyxxx doesn't look like it does, I don't mock all my headers and the tests are working just fine with the mentioned change.
Also the header assertions are working as expected (tests will fail if the supplied header content doesn't match the expectation).

@jdx
Copy link

jdx commented Jun 7, 2017

good to know! thank you!

@acofer
Copy link

acofer commented Jun 8, 2017

#930

thomasconner pushed a commit to Kinvey/js-sdk that referenced this issue Jun 9, 2017
voxpelli added a commit to voxpelli/webpage-micropub-to-github that referenced this issue Jun 16, 2017
Nock is failing at Node 8 right now, see eg: nock/nock#925
kirederik added a commit to kirederik/node-concourse that referenced this issue Jun 17, 2017
@shahata
Copy link

shahata commented Jul 8, 2017

any plans of fixing this soon?

thomasconner pushed a commit to Kinvey/js-sdk that referenced this issue Jul 8, 2017
@0xcaff
Copy link

0xcaff commented Jul 14, 2017

It looks like this is fixed in the latest commit to master: 35c76e3

@ianwsperber
Copy link
Contributor

Will be released end of weekend

@ianwsperber
Copy link
Contributor

v9.0.14 out with @martinkuba's fix. Huge thanks to him for doing the work. Closing this issue. Will re-open if we get reports that issue persists.

@panva
Copy link
Author

panva commented Jul 17, 2017

resolved, @ianwsperber @martinkuba thanks!

thomasconner pushed a commit to Kinvey/js-sdk that referenced this issue Aug 3, 2017
kevinoid added a commit to kevinoid/appveyor-status that referenced this issue Jun 29, 2018
9.0.14 includes nock/nock#929 which fixes nock/nock#922 and
nock/nock#925 which prevented nock from working on Node 8 and later.
Now test everything on all supported Node versions.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
@lock
Copy link

lock bot commented Sep 13, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue and add a reference to this one if it’s related. Thank you!

@lock lock bot locked as resolved and limited conversation to collaborators Sep 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants