Skip to content

Commit

Permalink
feat(socket): propagate errors from destroy method (#1675)
Browse files Browse the repository at this point in the history
Calling `destroy` on the the response, with an error, not correctly propagates through the socket and then the request object. This aligns with Node's native functionality. 

Fixes: #1669
  • Loading branch information
Filippo Conti authored and mastermatt committed Aug 20, 2019
1 parent 2e56fb0 commit de9c40b
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 2 deletions.
2 changes: 1 addition & 1 deletion lib/request_overrider.js
Expand Up @@ -124,7 +124,7 @@ class InterceptedRequestHandler {
this.response.socket = this.response.client = this.response.connection =
req.socket

propagate(['timeout'], req.socket, req)
propagate(['error', 'timeout'], req.socket, req)

req.write = (...args) => this.handleWrite(...args)
req.end = (...args) => this.handleEnd(...args)
Expand Down
6 changes: 5 additions & 1 deletion lib/socket.js
Expand Up @@ -54,8 +54,12 @@ module.exports = class Socket extends EventEmitter {
).toString('base64')
}

destroy() {
destroy(err) {
this.destroyed = true
this.readable = this.writable = false
if (err) {
this.emit('error', err)
}
return this
}
}
43 changes: 43 additions & 0 deletions tests/test_destroy.js
@@ -0,0 +1,43 @@
'use strict'

const nock = require('..')
const http = require('http')
const { test } = require('tap')

require('./cleanup_after_each')()

test('req.destroy should emit error event if called with error', t => {
nock('http://example.test')
.get('/')
.reply(404)

http
.get('http://example.test/', res => {
if (res.statusCode !== 200) {
res.destroy(new Error('Response error'))
}
})
.once('error', err => {
t.type(err, Error)
t.equal(err.message, 'Response error')
t.end()
})
})

test('req.destroy should not emit error event if called without error', t => {
nock('http://example.test')
.get('/')
.reply(403)

http
.get('http://example.test/', res => {
if (res.statusCode === 403) {
res.destroy()
}

t.end()
})
.once('error', () => {
t.fail('should not emit error')
})
})

0 comments on commit de9c40b

Please sign in to comment.