Permalink
Browse files

Fix: Handle pause() calls on destroyed uploads

This patch also improves the behavior for resume().

net.Stream currently does not guarantee 'data' to stop emitting after
pausing a stream. This makes the current throttling function
problematic and prone to race conditions. This patch will take of it
for now, but a further patch for node itself will make things even
smoother.
  • Loading branch information...
1 parent a6922ef commit 3988f82c177635cb7654ba5e8af07814f04153a9 @felixge felixge committed Jun 29, 2010
Showing with 56 additions and 4 deletions.
  1. +15 −2 lib/formidable/incoming_form.js
  2. +41 −2 test/simple/test-incoming-form.js
@@ -31,15 +31,28 @@ exports.IncomingForm = IncomingForm;
IncomingForm.prototype.parse = function(req, cb) {
this.pause = function() {
- req.pause();
+ try {
+ req.pause();
+ } catch (err) {
+ // the stream was destroyed
+ if (!this.ended) {
+ // before it was completed, crash & burn
+ this._error(err);
+ }
+ return false;
+ }
return true;
};
this.resume = function() {
try {
req.resume();
} catch (err) {
- this._error(err);
+ // the stream was destroyed
+ if (!this.ended) {
+ // before it was completed, crash & burn
+ this._error(err);
+ }
return false;
}
@@ -58,13 +58,41 @@ test(function parse() {
gently.expect(REQ, 'pause');
assert.strictEqual(form.pause(), true);
})();
-
+
+ (function testPauseCriticalException() {
+ form.ended = false;
+
+ var ERR = new Error('dasdsa');
+ gently.expect(REQ, 'pause', function() {
+ throw ERR;
+ });
+
+ gently.expect(form, '_error', function(err) {
+ assert.strictEqual(err, ERR);
+ });
+
+ assert.strictEqual(form.pause(), false);
+ })();
+
+ (function testPauseHarmlessException() {
+ form.ended = true;
+
+ var ERR = new Error('dasdsa');
+ gently.expect(REQ, 'pause', function() {
+ throw ERR;
+ });
+
+ assert.strictEqual(form.pause(), false);
+ })();
+
(function testResume() {
gently.expect(REQ, 'resume');
assert.strictEqual(form.resume(), true);
})();
- (function testResumeException() {
+ (function testResumeCriticalException() {
+ form.ended = false;
+
var ERR = new Error('dasdsa');
gently.expect(REQ, 'resume', function() {
throw ERR;
@@ -76,6 +104,17 @@ test(function parse() {
assert.strictEqual(form.resume(), false);
})();
+
+ (function testResumeHarmlessException() {
+ form.ended = true;
+
+ var ERR = new Error('dasdsa');
+ gently.expect(REQ, 'resume', function() {
+ throw ERR;
+ });
+
+ assert.strictEqual(form.resume(), false);
+ })();
(function testEmitError() {
var ERR = new Error('something bad happened');

0 comments on commit 3988f82

Please sign in to comment.