From 0c29193525748cd6cbde5eb02672964b8a88b69e Mon Sep 17 00:00:00 2001 From: Paul Melnikow Date: Thu, 6 Feb 2020 15:45:18 -0600 Subject: [PATCH 1/2] fix: Correct behavior when other libraries override http.get and http.request Thanks @kennethaasan for finding this fix! Fix #1836 Closes #1853 --- lib/common.js | 6 +++- tests/test_request_overrider.js | 63 +++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/lib/common.js b/lib/common.js index 59f075928..cbfb6fc38 100644 --- a/lib/common.js +++ b/lib/common.js @@ -99,7 +99,11 @@ function overrideRequests(newRequest) { } // https://nodejs.org/api/http.html#http_http_get_options_callback module.get = function(input, options, callback) { - const req = module.request(input, options, callback) + const req = newRequest(proto, overriddenGet.bind(module), [ + input, + options, + callback, + ]) req.end() return req } diff --git a/tests/test_request_overrider.js b/tests/test_request_overrider.js index 5fe90bbbf..9c230636d 100644 --- a/tests/test_request_overrider.js +++ b/tests/test_request_overrider.js @@ -684,3 +684,66 @@ test('Request with `Expect: 100-continue` triggers continue event', t => { req.end(exampleRequestBody) }) }) + +// https://github.com/nock/nock/issues/1836 +test( + 'when http.get and http.request have been overridden before nock overrides them, http.get calls through to the expected method', + async t => { + // TODO Investigate why this is needed when it's also in the `afterEach()` + // hook in ./setup. + t.on('end', () => { + nock.restore() + sinon.restore() + }) + + // Obtain the original `http.request()` and stub it out, as a library might. + nock.restore() + const overriddenRequest = sinon.stub(http, 'request').callThrough() + const overriddenGet = sinon.stub(http, 'get').callThrough() + + // Let Nock override them again. + nock.activate() + + const server = http.createServer((request, response) => { + response.writeHead(200) + response.end() + }) + await new Promise(resolve => server.listen(resolve)) + + const req = http.get(`http://localhost:${server.address().port}`) + expect(overriddenGet).to.have.been.calledOnce() + expect(overriddenRequest).not.to.have.been.called() + + req.abort() + server.close() + } +) + +// https://github.com/nock/nock/issues/1836 +test( + 'when http.get and http.request have been overridden before nock overrides them, http.request calls through to the expected method', + async t => { + t.on('end', () => { + nock.restore() + sinon.restore() + }) + + // Obtain the original `http.request()` and stub it out, as a library might. + nock.restore() + const overriddenRequest = sinon.stub(http, 'request').callThrough() + const overriddenGet = sinon.stub(http, 'get').callThrough() + + // Let Nock override them again. + nock.activate() + + const req = http.request({ + host: 'localhost', + path: '/', + port: 1234 + }) + expect(overriddenRequest).to.have.been.calledOnce() + expect(overriddenGet).not.to.have.been.called() + + req.abort() + } +) From 32a8999d2e0fb16e37e873d3ec40f43a123cd7fe Mon Sep 17 00:00:00 2001 From: Paul Melnikow Date: Thu, 6 Feb 2020 15:51:45 -0600 Subject: [PATCH 2/2] Reformat --- tests/test_request_overrider.js | 94 +++++++++++++++------------------ 1 file changed, 44 insertions(+), 50 deletions(-) diff --git a/tests/test_request_overrider.js b/tests/test_request_overrider.js index 9c230636d..9ca868ce7 100644 --- a/tests/test_request_overrider.js +++ b/tests/test_request_overrider.js @@ -686,64 +686,58 @@ test('Request with `Expect: 100-continue` triggers continue event', t => { }) // https://github.com/nock/nock/issues/1836 -test( - 'when http.get and http.request have been overridden before nock overrides them, http.get calls through to the expected method', - async t => { - // TODO Investigate why this is needed when it's also in the `afterEach()` - // hook in ./setup. - t.on('end', () => { - nock.restore() - sinon.restore() - }) - - // Obtain the original `http.request()` and stub it out, as a library might. +test('when http.get and http.request have been overridden before nock overrides them, http.get calls through to the expected method', async t => { + // TODO Investigate why this is needed when it's also in the `afterEach()` + // hook in ./setup. + t.on('end', () => { nock.restore() - const overriddenRequest = sinon.stub(http, 'request').callThrough() - const overriddenGet = sinon.stub(http, 'get').callThrough() + sinon.restore() + }) - // Let Nock override them again. - nock.activate() + // Obtain the original `http.request()` and stub it out, as a library might. + nock.restore() + const overriddenRequest = sinon.stub(http, 'request').callThrough() + const overriddenGet = sinon.stub(http, 'get').callThrough() - const server = http.createServer((request, response) => { - response.writeHead(200) - response.end() - }) - await new Promise(resolve => server.listen(resolve)) + // Let Nock override them again. + nock.activate() + + const server = http.createServer((request, response) => { + response.writeHead(200) + response.end() + }) + await new Promise(resolve => server.listen(resolve)) - const req = http.get(`http://localhost:${server.address().port}`) - expect(overriddenGet).to.have.been.calledOnce() - expect(overriddenRequest).not.to.have.been.called() + const req = http.get(`http://localhost:${server.address().port}`) + expect(overriddenGet).to.have.been.calledOnce() + expect(overriddenRequest).not.to.have.been.called() - req.abort() - server.close() - } -) + req.abort() + server.close() +}) // https://github.com/nock/nock/issues/1836 -test( - 'when http.get and http.request have been overridden before nock overrides them, http.request calls through to the expected method', - async t => { - t.on('end', () => { - nock.restore() - sinon.restore() - }) - - // Obtain the original `http.request()` and stub it out, as a library might. +test('when http.get and http.request have been overridden before nock overrides them, http.request calls through to the expected method', async t => { + t.on('end', () => { nock.restore() - const overriddenRequest = sinon.stub(http, 'request').callThrough() - const overriddenGet = sinon.stub(http, 'get').callThrough() + sinon.restore() + }) - // Let Nock override them again. - nock.activate() + // Obtain the original `http.request()` and stub it out, as a library might. + nock.restore() + const overriddenRequest = sinon.stub(http, 'request').callThrough() + const overriddenGet = sinon.stub(http, 'get').callThrough() - const req = http.request({ - host: 'localhost', - path: '/', - port: 1234 - }) - expect(overriddenRequest).to.have.been.calledOnce() - expect(overriddenGet).not.to.have.been.called() + // Let Nock override them again. + nock.activate() - req.abort() - } -) + const req = http.request({ + host: 'localhost', + path: '/', + port: 1234, + }) + expect(overriddenRequest).to.have.been.calledOnce() + expect(overriddenGet).not.to.have.been.called() + + req.abort() +})