Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

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

Closed
s3u opened this issue May 7, 2012 · 12 comments
Closed

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

s3u opened this issue May 7, 2012 · 12 comments
Labels

Comments

@s3u
Copy link

s3u commented May 7, 2012

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.

@mscdex
Copy link

mscdex commented May 7, 2012

This:

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

should be:

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

@s3u
Copy link
Author

s3u commented May 7, 2012

Which setTimeout are you referring to?

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

socket.setTimeout(timeout, [callback])

@mscdex
Copy link

mscdex commented May 7, 2012

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

@rdzar
Copy link

rdzar commented May 7, 2012

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.

@s3u
Copy link
Author

s3u commented May 7, 2012

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

@bnoordhuis
Copy link
Member

@s3u Can you post a complete test case?

@quartzjer
Copy link

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

bnoordhuis added a commit that referenced this issue May 7, 2012
@bnoordhuis
Copy link
Member

Failing test case added in e02af94.

@s3u
Copy link
Author

s3u commented May 7, 2012

@bnoordhuis Thanks. Was just to submit one.

@isaacs isaacs closed this as completed in b4fbf6d May 7, 2012
@bnoordhuis
Copy link
Member

@s3u Can you check if b4fbf6d resolves your issue?

@s3u
Copy link
Author

s3u commented May 8, 2012

@bnoordhuis it does. Thanks.

@bnoordhuis
Copy link
Member

Great, thanks for testing.

isaacs added a commit that referenced this issue May 14, 2012
* 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)
isaacs added a commit that referenced this issue May 15, 2012
* 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)
jerbob92 pushed a commit to jerbob92/hallway-original that referenced this issue Nov 10, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants