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: response.complete must be true after res.end #1601

Merged
merged 1 commit into from
Jul 15, 2019
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
4 changes: 4 additions & 0 deletions lib/request_overrider.js
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,8 @@ function RequestOverrider(req, options, interceptors, remove, cb) {
})
responseBody.on('end', function() {
response.push(null)
// https://nodejs.org/dist/latest-v10.x/docs/api/http.html#http_message_complete
response.complete = true
})
responseBody.on('error', function(err) {
response.emit('error', err)
Expand Down Expand Up @@ -556,6 +558,8 @@ function RequestOverrider(req, options, interceptors, remove, cb) {
} else {
debug('ending response stream')
response.push(null)
// https://nodejs.org/dist/latest-v10.x/docs/api/http.html#http_message_complete
response.complete = true
interceptor.scope.emit('replied', req, interceptor)
}
})
Expand Down
27 changes: 27 additions & 0 deletions tests/test_request_overrider.js
Original file line number Diff line number Diff line change
Expand Up @@ -429,3 +429,30 @@ test("mocked requests have 'method' property", t => {
})
req.end()
})

// https://github.com/nock/nock/issues/1493
test("response have 'complete' property and it's true after end", t => {
const scope = nock('http://example.test')
.get('/')
.reply(200, 'Hello World!')

const req = http.request(
{
host: 'example.test',
method: 'GET',
path: '/',
port: 80,
},
res => {
res.on('end', () => {
t.is(res.complete, true)
scope.done()
t.end()
})
// Streams start in 'paused' mode and must be started.
// See https://nodejs.org/api/stream.html#stream_class_stream_readable
res.resume()
}
)
req.end()
})
37 changes: 37 additions & 0 deletions tests/test_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,43 @@ test('pause response after data', t => {
}, 0)
})

// https://github.com/nock/nock/issues/1493
test("response have 'complete' property and it's true after end", t => {
const response = new stream.PassThrough()
const scope = nock('http://example.test')
.get('/')
// Node does not pause the 'end' event so we need to use a stream to simulate
// multiple 'data' events.
.reply(200, response)

http.get(
{
host: 'example.test',
path: '/',
},
res => {
let hasData = false

res.on('data', () => {
hasData = true
})

res.on('end', () => {
t.true(hasData)
t.is(res.complete, true)
scope.done()
t.end()
})
}
)

// Manually simulate multiple 'data' events.
response.emit('data', 'one')
setTimeout(() => {
response.end()
}, 0)
})

// TODO Convert to async / got.
test('response pipe', t => {
const dest = (() => {
Expand Down