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

fix: InterceptedRequestRouter.handleWrite arity issue #2371

Merged
merged 5 commits into from Jun 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions .eslintrc.yml
Expand Up @@ -27,6 +27,9 @@ rules:
# TODO These are included in standard and should be cleaned up and turned on.
n/no-deprecated-api: 'off' # we still make heavy use of url.parse

# TODO remove once we drop support for Node 10
node/no-deprecated-api: 'off'

# Nock additions.
strict: ['error', 'safe']
no-console: 'error'
Expand Down
8 changes: 7 additions & 1 deletion lib/intercepted_request_router.js
Expand Up @@ -143,8 +143,11 @@ class InterceptedRequestRouter {

// from docs: When write function is called with empty string or buffer, it does nothing and waits for more input.
// However, actually implementation checks the state of finished and aborted before checking if the first arg is empty.
handleWrite(buffer, encoding, callback) {
handleWrite(...args) {
debug('request write')

let [buffer, encoding] = args

const { req } = this

if (req.finished) {
Expand All @@ -171,6 +174,9 @@ class InterceptedRequestRouter {
}
this.requestBodyBuffers.push(buffer)

// writable.write encoding param is optional
// so if callback is present it's the last argument
const callback = args.length > 1 ? args[args.length - 1] : undefined
// can't use instanceof Function because some test runners
// run tests in vm.runInNewContext where Function is not same
// as that in the current context
Expand Down
62 changes: 62 additions & 0 deletions tests/test_request_overrider.js
Expand Up @@ -89,6 +89,40 @@ describe('Request Overrider', () => {
})
})

it('write callback called when encoding is not supplied', done => {
const scope = nock('http://example.test')
.filteringRequestBody(/mia/, 'nostra')
.post('/', 'mamma nostra')
.reply()

const reqWriteCallback = sinon.spy()

const req = http.request(
{
host: 'example.test',
method: 'POST',
path: '/',
port: 80,
},
res => {
expect(reqWriteCallback).to.have.been.calledOnce()
expect(res.statusCode).to.equal(200)
res.on('end', () => {
scope.done()
done()
})
// Streams start in 'paused' mode and must be started.
// See https://nodejs.org/api/stream.html#stream_class_stream_readable
res.resume()
}
)

req.write('mamma mia', () => {
reqWriteCallback()
req.end()
})
})

it('write callback is not called if the provided chunk is undefined', done => {
const scope = nock('http://example.test').post('/').reply()

Expand Down Expand Up @@ -117,6 +151,34 @@ describe('Request Overrider', () => {
req.end()
})

it("write doesn't throw if invoked w/o callback", done => {
const scope = nock('http://example.test').post('/').reply()

const reqWriteCallback = sinon.spy()

const req = http.request(
{
host: 'example.test',
method: 'POST',
path: '/',
},
res => {
expect(res.statusCode).to.equal(200)
res.on('end', () => {
expect(reqWriteCallback).to.not.have.been.called()
scope.done()
done()
})
// Streams start in 'paused' mode and must be started.
// See https://nodejs.org/api/stream.html#stream_class_stream_readable
res.resume()
}
)

req.write(undefined)
req.end()
})

it('end callback called', done => {
const scope = nock('http://example.test')
.filteringRequestBody(/mia/, 'nostra')
Expand Down