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(socketDelay): support options.timeout #1848

Merged
merged 6 commits into from Feb 10, 2020
Merged
Show file tree
Hide file tree
Changes from 4 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
7 changes: 7 additions & 0 deletions lib/intercepted_request_router.js
Expand Up @@ -31,6 +31,13 @@ class InterceptedRequestRouter {
this.interceptors = interceptors

this.socket = new Socket(options)

// support setting `timeout` using request `options`
// https://nodejs.org/docs/latest-v12.x/api/http.html#http_http_request_url_options_callback
if (options.timeout) {
this.socket.setTimeout(options.timeout)
gurpreetatwal marked this conversation as resolved.
Show resolved Hide resolved
}

this.response = new IncomingMessage(this.socket)
this.playbackStarted = false
this.requestBodyBuffers = []
Expand Down
25 changes: 23 additions & 2 deletions tests/test_socketdelay.js
Expand Up @@ -35,7 +35,7 @@ describe('`socketDelay()`', () => {
})
})

it('emits a timeout', done => {
it('emits a timeout - with setTimeout', done => {
nock('http://example.test')
.get('/')
.socketDelay(10000)
Expand All @@ -56,7 +56,28 @@ describe('`socketDelay()`', () => {
req.end()
})

it('emits a timeout if not idle for long enough', done => {
it('emits a timeout - with options.timeout', done => {
nock('http://example.test')
.get('/')
.socketDelay(10000)
.reply(200, 'OK')

const onEnd = sinon.spy()

const req = http.request('http://example.test', { timeout: 5000 }, res => {
res.setEncoding('utf8')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this setEncoding have an affect on the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know to be fully honest. I copied the test case from above and made the minimal number of changes to make it work for options.timeout. I can take a look into it and adjust it for both tests cases if needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mastermatt any thoughts?

res.once('end', onEnd)
})

req.on('timeout', function() {
expect(onEnd).not.to.have.been.called()
done()
})

req.end()
})

it('does not emit a timeout when timeout > socketDelay', done => {
const responseText = 'okeydoke!'
const scope = nock('http://example.test')
.get('/')
Expand Down