-
-
Notifications
You must be signed in to change notification settings - Fork 34.2k
Always call callbacks in lib/_http_outgoing.js #1143
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
Changes from all commits
2dd0bc2
d733196
8f20ed6
6d4bd06
e712b26
10d5da8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,6 +70,27 @@ function OutgoingMessage() { | |
| this._header = null; | ||
| this._headers = null; | ||
| this._headerNames = {}; | ||
|
|
||
| var self = this; | ||
|
|
||
| // Call remaining callbacks if stream closes prematurely | ||
| this.once('close', function() { | ||
| if (self.output.length === 0) return; | ||
|
|
||
| var err = new Error('stream closed prematurely'); | ||
|
|
||
| var outputCallbacks = self.outputCallbacks; | ||
|
|
||
| self.output = []; | ||
| self.outputEncodings = []; | ||
| self.outputCallbacks = []; | ||
|
|
||
| for (var i = 0; i < outputCallbacks.length; i++) { | ||
| var callback = outputCallbacks[i]; | ||
| if (typeof callback === 'function') | ||
|
||
| callback(err); | ||
| } | ||
| }); | ||
| } | ||
| util.inherits(OutgoingMessage, Stream); | ||
|
|
||
|
|
@@ -159,12 +180,8 @@ OutgoingMessage.prototype._writeRaw = function(data, encoding, callback) { | |
|
|
||
| // Directly write to socket. | ||
| return connection.write(data, encoding, callback); | ||
| } else if (connection && connection.destroyed) { | ||
| // The socket was destroyed. If we're still trying to write to it, | ||
| // then we haven't gotten the 'close' event yet. | ||
| return false; | ||
| } else { | ||
| // buffer, as long as we're not destroyed. | ||
| // buffer, as long as we didn't get the "close" event | ||
| return this._buffer(data, encoding, callback); | ||
| } | ||
| }; | ||
|
|
@@ -407,9 +424,10 @@ Object.defineProperty(OutgoingMessage.prototype, 'headersSent', { | |
|
|
||
| OutgoingMessage.prototype.write = function(chunk, encoding, callback) { | ||
| var self = this; | ||
| var err; | ||
|
|
||
| if (this.finished) { | ||
| var err = new Error('write after end'); | ||
| err = new Error('write after end'); | ||
|
||
| process.nextTick(function() { | ||
| self.emit('error', err); | ||
| if (callback) callback(err); | ||
|
|
@@ -432,10 +450,13 @@ OutgoingMessage.prototype.write = function(chunk, encoding, callback) { | |
| throw new TypeError('first argument must be a string or Buffer'); | ||
| } | ||
|
|
||
|
|
||
| // If we get an empty string or buffer, then just do nothing, and | ||
| // signal the user to keep writing. | ||
| if (chunk.length === 0) return true; | ||
| if (chunk.length === 0) { | ||
| if (typeof callback === 'function') | ||
| process.nextTick(callback); | ||
| return true; | ||
| } | ||
|
|
||
| var len, ret; | ||
| if (this.chunkedEncoding) { | ||
|
|
@@ -511,11 +532,24 @@ OutgoingMessage.prototype.end = function(data, encoding, callback) { | |
| throw new TypeError('first argument must be a string or Buffer'); | ||
| } | ||
|
|
||
| var self = this; | ||
|
|
||
| if (this.finished) { | ||
| if (data && data.length > 0) { | ||
| //If the user tried to write data, tell him it failed | ||
|
||
| var err = new Error('write after end'); | ||
| process.nextTick(function() { | ||
| self.emit('error', err); | ||
| if (callback) callback(err); | ||
| }); | ||
| } else { | ||
| //If he only wanted to end the stream anyways, lucky for him | ||
| if (callback) | ||
|
||
| process.nextTick(callback); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| var self = this; | ||
| function finish() { | ||
| self.emit('finish'); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,10 +15,27 @@ var numRequests = 20; | |
| var done = 0; | ||
|
|
||
| var server = http.createServer(function(req, res) { | ||
| res.end('ok'); | ||
|
|
||
| var callbackCalled = false; | ||
|
|
||
| res.end('ok', function() { | ||
| assert.ok(!callbackCalled); | ||
|
||
| callbackCalled = true; | ||
| }); | ||
|
|
||
| // We might get a socket already closed error here | ||
| // Not really a surpise | ||
| res.on('error', function() {}); | ||
|
||
|
|
||
| res.on('close', function() { | ||
| process.nextTick(function() { | ||
| assert.ok(callbackCalled, "end() callback should've been called"); | ||
|
||
| }); | ||
| }); | ||
|
|
||
| // Oh no! The connection died! | ||
| req.socket.destroy(); | ||
|
|
||
|
||
| if (++done == numRequests) | ||
| server.close(); | ||
| }); | ||
|
|
@@ -31,5 +48,7 @@ for (var i = 0; i < numRequests; i++) { | |
| 'Host: some.host.name\r\n'+ | ||
| '\r\n\r\n'); | ||
| } | ||
|
|
||
| client.end(); | ||
|
|
||
| client.pipe(process.stdout); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason you cache the variable and reset it rather than using it raw in the
forloop and resetting it afterwards?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I worry the callback function over which we have no control might do something fishy and mess up our state while we're working with it. If such things are nothing we are supposed to be guarding against, then I'll change it of course.