Skip to content
This repository

connection.destroy() during http req setTimeout() causes TypeError #3231

Closed
s3u opened this Issue · 12 comments

6 participants

Subbu Allamaraju Isaac Z. Schlueter Brian White René v. Sweeden Ben Noordhuis Jeremie Miller
Subbu Allamaraju
s3u commented
var clientReq = http.request(options, function (clientRes) {
    ...
});
clientReq.setTimeout(1000, function () {
    clientReq.connection.destroy();
    ...
});

causes

TypeError: Cannot call method 'emit' of null
   at Socket.<anonymous> (http.js:1163:11)
   at Socket.emit (events.js:67:17)
   at Array.1 (net.js:301:14)
   at EventEmitter._tickCallback (node.js:192:40)

This is a regression in v0.6.17. This does not happen in v0.6.16.

Brian White
mscdex commented

This:

clientReq.setTimeout(1000, function () {
    clientReq.connection.destroy();
    ...
});

should be:

clientReq.setTimeout(function () {
    clientReq.connection.destroy();
    ...
}, 1000);
Subbu Allamaraju
s3u commented

Which setTimeout are you referring to?

Per http://nodejs.org/api/net.html#socket.setTimeout,

socket.setTimeout(timeout, [callback])
Brian White
mscdex commented

Ah ok, I didn't realize the order was reversed for socket setTimeout.

René v. Sweeden
renedx commented

Doesn't timeout already destroy the connection? (I know it says it doesn't in the documentation, haha)
https://github.com/joyent/node/blob/master/lib/http.js#L1518

[m@vm vm]# node run.http.js
callback: timeout
emit: error
emit: close

Because close gets emitted too.

Subbu Allamaraju
s3u commented

True, I missed that. I guess the refactoring that went into 0.6.17 makes this explicit.

Ben Noordhuis

@s3u Can you post a complete test case?

Jeremie Miller

I'm encountering this in production on heroku as well now, reverting to 0.6.16 in the interim.

Ben Noordhuis bnoordhuis referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
Ben Noordhuis

Failing test case added in e02af94.

Subbu Allamaraju
s3u commented

@bnoordhuis Thanks. Was just to submit one.

Isaac Z. Schlueter isaacs closed this in b4fbf6d
Ben Noordhuis

@s3u Can you check if b4fbf6d resolves your issue?

Subbu Allamaraju
s3u commented

@bnoordhuis it does. Thanks.

Ben Noordhuis

Great, thanks for testing.

Isaac Z. Schlueter isaacs referenced this issue from a commit
Isaac Z. Schlueter 2012.05.15 Version 0.6.18 (stable)
* windows: skip GetFileAttributes call when opening a file (Bert Belder)

* crypto: add PKCS12/PFX support (Sambasiva Suda)

* #3240: child_process: delete NODE_CHANNEL_FD from env in spawn (Ben Noordhuis)

* windows: add test for path.normalize with UNC paths (Bert Belder)

* windows: make path.normalize convert all slashes to backslashes (Bert Belder)

* fs: Automatically close FSWatcher on error (Bert Belder)

* #3258: fs.ReadStream.pause() emits duplicate data event (koichik)

* pipe_wrap: don't assert() on pipe accept errors (Ben Noordhuis)

* Better exception output for module load and process.nextTick (Felix Geisendörfer)

* zlib: fix error reporting (Ben Noordhuis)

* http: Don't destroy on timeout (isaacs)

* #3231: http: Don't try to emit error on a null'ed req object (isaacs)

* #3236: http: Refactor ClientRequest.onSocket (isaacs)
bb25001
Isaac Z. Schlueter isaacs referenced this issue from a commit
Isaac Z. Schlueter 2012.05.15 Version 0.6.18 (stable)
* windows: skip GetFileAttributes call when opening a file (Bert Belder)

* crypto: add PKCS12/PFX support (Sambasiva Suda)

* #3240: child_process: delete NODE_CHANNEL_FD from env in spawn (Ben Noordhuis)

* windows: add test for path.normalize with UNC paths (Bert Belder)

* windows: make path.normalize convert all slashes to backslashes (Bert Belder)

* fs: Automatically close FSWatcher on error (Bert Belder)

* #3258: fs.ReadStream.pause() emits duplicate data event (koichik)

* pipe_wrap: don't assert() on pipe accept errors (Ben Noordhuis)

* Better exception output for module load and process.nextTick (Felix Geisendörfer)

* zlib: fix error reporting (Ben Noordhuis)

* http: Don't destroy on timeout (isaacs)

* #3231: http: Don't try to emit error on a null'ed req object (isaacs)

* #3236: http: Refactor ClientRequest.onSocket (isaacs)
4bc1d39
jerbob92 jerbob92 referenced this issue from a commit in jerbob92/hallway-original
Jeremie Miller forcing older node to workaround joyent/node#3231 07125e8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.