uncaught exceptions in nested generators won't throw in callback fn, co(gen)(fn) #92

Closed
matthewmueller opened this Issue Mar 7, 2014 · 7 comments

3 participants

@matthewmueller

Here's how to reproduce it:

var co = require('co');

co(function *() {
  yield function *() {
    req + 'hi'; // ReferenceError: req is not defined
    return req;
  }
})(function(err) {
  throw err; // error doesn't throw, it simply won't print anything after this
  console.log(err); // if I remove throw err, it will log [ReferenceError: req is not defined]
  console.log('hi');
});

This is inconsistent with the non-nested example:

var co = require('co');

co(function *() {
  req + 'hi';
})(function(err) {
  throw err; // properly throws
});
@matthewmueller

this is really problematic for things like @juliangruber's co-streams, particularly read streams.

@jonathanong

haaaahaaaha the internal try/catch's are catching it. quickest fix i can think of is always wrapping the callback in a setImmediate, which wouldn't be too bad. can be reduced to:

var co = require('..');
var assert = require('assert');

describe('bugs', function(){
  it('#92', function(done){
    co(function *() {
      yield function (done) {
        done(new Error('lol'))
      }
    })(function(err) {
      throw err;
      done();
    });
  })
})
@jonathanong

i think the correct way to do all of this is to always setImmediate between every yield, but that decreases performance a lot if you don't yield* everywhere

@matthewmueller

that sounds like an awful hack, haha.

@jonathanong

for now, you can do:

if (err) setImmediate(function() { throw err });

lol

@tj
Owner
tj commented Mar 7, 2014

hmm yeah not good that I'll try and take a look today

@matthewmueller

yah, that's pretty true, i discovered it when i was testing something. i think for CLI apps this could be a problem though.

@jonathanong jonathanong added a commit that closed this issue Mar 14, 2014
@jonathanong jonathanong fix #92 19ada57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment