Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fixing double callback firing. node 0.5 is much better about calling …

…errors on the client object which, when aborting on timeout, predictable emits an error which then triggers a double callback.
  • Loading branch information...
commit a2e5d6e30d3e101f8c5a034ef0401fdde8608ccf 1 parent 248e9d6
@mikeal mikeal authored
Showing with 72 additions and 60 deletions.
  1. +11 −0 main.js
  2. +61 −60 tests/test-timeout.js
View
11 main.js
@@ -90,6 +90,17 @@ Request.prototype.getAgent = function (host, port) {
}
Request.prototype.request = function () {
var self = this
+
+ // Protect against double callback
+ if (!self._callback && self.callback) {
+ self._callback = self.callback
+ self.callback = function () {
+ if (self._callbackCalled) return // Print a warning maybe?
+ self._callback.apply(self, arguments)
+ self._callbackCalled = true
+ }
+ }
+
if (self.url) {
// People use this property instead all the time so why not just support it.
self.uri = self.url
View
121 tests/test-timeout.js
@@ -11,76 +11,77 @@ var remainingTests = 5;
s.listen(s.port, function () {
// Request that waits for 200ms
-s.on('/timeout', function (req, resp) {
- setTimeout(function(){
- resp.writeHead(200, {'content-type':'text/plain'})
- resp.write(expectedBody)
- resp.end()
- }, 200);
-});
-
-// Scenario that should timeout
-var shouldTimeout = {
- url: s.url + "/timeout",
- timeout:100
-}
-
-request(shouldTimeout, function (err, resp, body) {
- assert.equal(err.code, "ETIMEDOUT");
- checkDone();
-})
+ s.on('/timeout', function (req, resp) {
+ setTimeout(function(){
+ resp.writeHead(200, {'content-type':'text/plain'})
+ resp.write(expectedBody)
+ resp.end()
+ }, 200);
+ });
+ // Scenario that should timeout
+ var shouldTimeout = {
+ url: s.url + "/timeout",
+ timeout:100
+ }
-// Scenario that shouldn't timeout
-var shouldntTimeout = {
- url: s.url + "/timeout",
- timeout:300
-}
-request(shouldntTimeout, function (err, resp, body) {
- assert.equal(err, null);
- assert.equal(expectedBody, body)
- checkDone();
-})
+ request(shouldTimeout, function (err, resp, body) {
+ assert.equal(err.code, "ETIMEDOUT");
+ checkDone();
+ })
-// Scenario with no timeout set, so shouldn't timeout
-var noTimeout = {
- url: s.url + "/timeout"
-}
-request(noTimeout, function (err, resp, body) {
- assert.equal(err);
- assert.equal(expectedBody, body)
- checkDone();
-})
+ // Scenario that shouldn't timeout
+ var shouldntTimeout = {
+ url: s.url + "/timeout",
+ timeout:300
+ }
-// Scenario with a negative timeout value, should be treated a zero or the minimum delay
-var negativeTimeout = {
- url: s.url + "/timeout",
- timeout:-1000
-}
+ request(shouldntTimeout, function (err, resp, body) {
+ assert.equal(err, null);
+ assert.equal(expectedBody, body)
+ checkDone();
+ })
-request(negativeTimeout, function (err, resp, body) {
- assert.equal(err.code, "ETIMEDOUT");
- checkDone();
-})
+ // Scenario with no timeout set, so shouldn't timeout
+ var noTimeout = {
+ url: s.url + "/timeout"
+ }
-// Scenario with a float timeout value, should be rounded by setTimeout anyway
-var floatTimeout = {
- url: s.url + "/timeout",
- timeout: 100.76
-}
+ request(noTimeout, function (err, resp, body) {
+ assert.equal(err);
+ assert.equal(expectedBody, body)
+ checkDone();
+ })
-request(floatTimeout, function (err, resp, body) {
- assert.equal(err.code, "ETIMEDOUT");
- checkDone();
-})
+ // Scenario with a negative timeout value, should be treated a zero or the minimum delay
+ var negativeTimeout = {
+ url: s.url + "/timeout",
+ timeout:-1000
+ }
+
+ request(negativeTimeout, function (err, resp, body) {
+ assert.equal(err.code, "ETIMEDOUT");
+ checkDone();
+ })
+
+ // Scenario with a float timeout value, should be rounded by setTimeout anyway
+ var floatTimeout = {
+ url: s.url + "/timeout",
+ timeout: 100.76
+ }
+
+ request(floatTimeout, function (err, resp, body) {
+ assert.equal(err.code, "ETIMEDOUT");
+ checkDone();
+ })
-function checkDone() {
- if(--remainingTests == 0) {
- s.close();
- console.log("All tests passed.");
+ function checkDone() {
+ if(--remainingTests == 0) {
+ s.close();
+ console.log("All tests passed.");
+ }
}
-}
})
Please sign in to comment.
Something went wrong with that request. Please try again.