fix some more zalgo corner cases and add more tests

lib/suspend.js

@@ -145,15 +145,29 @@ suspend.join = function join() {
  * interactions with a suspend-wrapped generator.
  */
 function Suspender(generator, callback) {
+	var self = this;
+
 	this.generator = generator;
 	// initialized in start()
 	this.iterator = null;
-	// (optionally) initialized in start()
-	this.callback = callback;
+	// flag to keep track of whether or not the entire generator completed.
+	// See start() for state tracking.
+	this.syncComplete = true;
+	// makes sure to not unleash zalgo: https://github.com/jmar777/suspend/pull/21
+	this.callback = callback && function() {
+		if (self.syncComplete) {
+			var args = Array.prototype.slice.call(arguments);
+			setImmediate(function() {
+				callback.apply(this, args);
+			});
+		} else {
+			callback.apply(this, arguments);
+		}
+	};
 	// flag indicating whether or not the iterator has completed
 	this.done = false;
 	// flag to keep track of whether or not we were resumed synchronously.
-	// See next() for state tracking.
+	// See nextOrThrow() for state tracking.
 	this.syncResume = false;
 	// flag indicating whether or not the values passed to resume() should be
 	// treated as raw values, or handled per the error-first callback convention
@@ -175,6 +189,7 @@ function Suspender(generator, callback) {
 Suspender.prototype.start = function start(ctx, args) {
 	this.iterator = this.generator.apply(ctx, args);
 	this.nextOrThrow();
+	this.syncComplete = false;
 };
 
 /**
@@ -183,7 +198,9 @@ Suspender.prototype.start = function start(ctx, args) {
 Suspender.prototype.handleYield = function handleYield(ret) {
 	if (ret.done) {
 		this.done = true;
-		this.callback && this.callback.call(null, null, ret.value);
+		if (this.callback) {
+			this.callback.call(null, null, ret.value);
+		}
 		return;
 	}
 
@@ -210,14 +227,16 @@ Suspender.prototype.handleYield = function handleYield(ret) {
  * resumed synchronously.
  */
 Suspender.prototype.nextOrThrow = function next(val, isError) {
+	var self = this;
+
 	this.syncResume = true;
 	setActiveSuspender(this);
 	var ret;
 	try {
 		ret = isError ? this.iterator.throw(val) : this.iterator.next(val);
 	} catch (err) {
-		var self = this;
-		setImmediate(function () {
+		// prevents promise swallowing: https://github.com/jmar777/suspend/pull/21
+		setImmediate(function() {
 			if (self.callback) {
 				return self.callback(err);
 			} else {

test/suspend.async.js

@@ -123,6 +123,35 @@ describe('suspend.async(fn*)()', function() {
 			done();
 		});
 	});
+
+
+	it('should not unleash zalgo on synchronous completion', function(done) {
+		var x = 41;
+
+		async(function*() {
+			return;
+		})(function() {
+			assert.strictEqual(42, x);
+			done();
+		});
+
+		// this should run before the callback
+		x += 1;
+	});
+
+	it('should not unleash zalgo on synchronously thrown errors', function(done) {
+		var x = 41;
+
+		async(function*() {
+			throw new Error();
+		})(function() {
+			assert.strictEqual(42, x);
+			done();
+		});
+
+		// this should run before the callback
+		x += 1;
+	});
 });
 
 // functions used for test cases

test/suspend.promise.js

@@ -109,6 +109,34 @@ describe('suspend.promise(fn*)()', function() {
 			done();
 		});
 	});
+
+	it('should not unleash zalgo on synchronous completion', function(done) {
+		var x = 41;
+
+		suspend.promise(function*() {
+			return;
+		})().then(function() {
+			assert.strictEqual(42, x);
+			done();
+		});
+
+		// this should run before the promise fulfillment
+		x += 1;
+	});
+
+	it('should not unleash zalgo on synchronously thrown errors', function(done) {
+		var x = 41;
+
+		suspend.promise(function*() {
+			throw new Error();
+		})().then(noop, function() {
+			assert.strictEqual(42, x);
+			done();
+		});
+
+		// this should run before the promise fulfillment
+		x += 1;
+	});
 });
 
 // functions used for test cases

test/suspend.run.js

@@ -84,6 +84,34 @@ describe('suspend.run(fn*, cb)', function() {
 			done();
 		});
 	});
+
+	it('should not unleash zalgo on synchronous completion', function(done) {
+		var x = 41;
+
+		run(function*() {
+			return;
+		}, function() {
+			assert.strictEqual(42, x);
+			done();
+		});
+
+		// this should run before the callback
+		x += 1;
+	});
+
+	it('should not unleash zalgo on synchronously thrown errors', function(done) {
+		var x = 41;
+
+		run(function*() {
+			throw new Error();
+		}, function() {
+			assert.strictEqual(42, x);
+			done();
+		});
+
+		// this should run before the callback
+		x += 1;
+	});
 });
 
 // functions used for test cases

test/yielding-promises.js

@@ -22,56 +22,27 @@ describe('yielded promises', function() {
 	});
 
 	it('should not swallow exception', function(done) {
-		// test uncaught exception
-		// http://stackoverflow.com/questions/9025095/how-can-i-test-uncaught-errors-in-mocha
-		var errorHandler = process.listeners('uncaughtException').pop();
-		process.removeListener('uncaughtException', errorHandler);
-
-		var recordedError = null;
-		function newHandler(error) {
-			recordedError = error;
-		}
-		process.on('uncaughtException', newHandler);
+		captureNextUncaughtException(function(err) {
+			assert.strictEqual(err.message, 'oops');
+			done();
+		});
 
 		run(function*() {
-			yield asyncError(); // this is uncaught exception
+			yield asyncError();
 		});
-
-		setTimeout(function() {
-			process.removeListener('uncaughtException', newHandler);
-			process.on('uncaughtException', errorHandler);
-
-			assert(recordedError !== null);
-			assert(recordedError.message === 'oops');
-			done();
-		}, 40);
 	});
 
-	it('should not swallow exception with callback', function(done) {
-		// http://stackoverflow.com/questions/9025095/how-can-i-test-uncaught-errors-in-mocha
-		var errorHandler = process.listeners('uncaughtException').pop();
-		process.removeListener('uncaughtException', errorHandler);
-
-		var recordedError = null;
-		function newHandler(error) {
-			recordedError = error;
-		}
-		process.on('uncaughtException', newHandler);
+	it('should not swallow exceptions in callback', function(done) {
+		captureNextUncaughtException(function(err) {
+			assert.strictEqual(err.message, 'from callback');
+			done();
+		});
 
 		run(function*() {
 			yield asyncError();
-		}, function (err) {
-			if (err) throw err; // this is uncaught exception
+		}, function(err) {
+			if (err) throw new Error('from callback');
 		});
-
-		setTimeout(function() {
-			process.removeListener('uncaughtException', newHandler);
-			process.on('uncaughtException', errorHandler);
-
-			assert(recordedError !== null);
-			assert(recordedError.message === 'oops');
-			done();
-		}, 40);
 	});
 
 	it('should behave correctly when multiple generators run in parallel', function(done) {
@@ -122,3 +93,15 @@ function asyncError() {
 		setTimeout(reject.bind(null, new Error('oops')), 20);
 	});
 }
+
+function captureNextUncaughtException(cb) {
+	var mochaListener = process.listeners('uncaughtException')[0];
+	process.removeListener('uncaughtException', mochaListener);
+	var newListener = function(err) {
+		// restore mocha's listener
+		process.removeListener('uncaughtException', newListener);
+		process.on('uncaughtException', mochaListener);
+		cb(err);
+	}
+	process.on('uncaughtException', newListener);
+}