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
BUG: Nested fibers, callback invocation in try block #9
Comments
wait.for is 70 LOC (and heavily commented), It's just a wrapper over node-fibers (or ES6-generators). All the hard work is done in node-fibers Question 1) do you really need an "inner" fiber? Wait.for proper usage is to launch a fiber to attend a request. Ideally here: var server = http.createServer(
function(req, res){
console.log('req!');
wait.launchFiber(handler,req,res); //handle in a fiber
// keep node spinning
}).listen(8000); then,at function handler(req,res) and every function you call from there, you'll be able to use wait.for(ayncFn... Normally, you don't need to launch "inner" fibers. why do you launch a fiber as the first thing in function inner? you can use wait.for inside function inner without launching another fiber. Can you give me more info about your project, so I can understand why you're launching inner fibers? |
Well, technically I don't need nested fibers, it was just coincidence. But other people might stumble upon this, that's why I am posting it :-) I am working on medium size project which is split into several modules and every module is offering traditional callback interface to the outer world. It happened that both lower layer module and the module using it were launching fibers. I have found the reason for this behaviour. Node-fibers documentation says:
So when resumeCallback is called again it restarts the whole fiber. I think it is enough to modify wait.for code in this way:
|
ok great! I've added your if statement. Thxs |
Hello,
thanks for this great library. I have found a interesting issue with it. I totally understand that it might not be a good idea to place callback invocations inside a
try
block, because then you might catch exceptions you never intended to. But anyway, we are not using try catch blocks in pure node.js. But inwait.for
we are, so we have to be careful about it.Here is short example in pure node.js style:
The exception is caught in the try block and this invokes the callback again. When the exception is thrown for the second time the callback invocation is no longer in the try block and we end up with uncaught exception as expected.
So how does this relates to wait.for? Let's try...
Again, the exception is caught, which is not surprising. The problem starts when the callback is invoked for the second time - the whole outer fiber is restarted! And then the exception is thrown again....
I have spent several hours debugging my code and this is the minimal example where it works. If you put away the inner fiber or
wait.for(setImmediate);
it works as it should - no strange restarts.I am just guessing - wait.for can't find the correct resume callback any more so it goes back to the beginning of the whole fiber. It should throw an error instead.
The text was updated successfully, but these errors were encountered: