Infinite recursion

[KidkArolis]

I just had a problem with when.js + jquery's deferreds.

when.map(someBackboneModels, function (model) {
  return model.save(); // this returns a jquery's ajax result
}).then(
  function () {
    console.log('success');
  },
  function () {
    console.log('error');
  }
);

In case of success, everything works as expected. In case of the error, when.js starts calling it's internal functions recursively and kills the browser..

I will try to reproduce/identify the problem, but do you spot anything wrong with the above?

[KidkArolis]

Here is an example of Backbone + jQuery + when.js when some infinite recursion happens:

http://jsfiddle.net/3th8H/3/

[KidkArolis]

So, actually, here is a shorter version of how this fails. I thought I've used this before and it used to work, strange.

var success = function () {
    console.log('success');
}
var error = function () {
    console.log('error');
}

var request = $.get("/will_fail");​​

// both of these fail, as in they do the 100% CPU thing on my Chrome 
when(request).then(success, error);
​when(request, success, error);

[briancavalier]

Very strange! I did a little investigating in your original fiddle, added some different cases, and tried it in Firefox and Safari as well. The results are pretty weird. In FF 12, all the cases work just fine, and no CPU spike. In Safari and Chrome, all the cases actually work correctly--that is, they produce the expected output to the console. However, the model.save() version also causes a CPU spike.

Here's my version of the fiddle

It's interesting that simply doing a $.get causes the CPU spike too. That makes me think that there's something weird about $.get, or the interaction between when.js and $.Deferred, or jsfiddle, and/or WebKit. This is a silly question, but I have to ask: Have you seen this CPU spike in real application code, and not just in jsfiddle?

I'll try your simplified example and see what I can find. If you find any more clues, let me know!

[briancavalier]

Here's another fiddle that shows using dojo.xhrGet works correctly, but $.get() causes some sort of CPU spike or an XHR lockup in both FF 12 and WebKit.

It's starting to seem like some sort of problem with $.get + $.Deferred + jsfiddle + http error. I'll keep looking.

[briancavalier]

FWIW, I haven't actually witnessed infinite recursion. I see the CPU spike, but it seems to happen after execution leaves the when.js code due to the XHR promise rejection (in Chrome), or due to the XHR locking up (in FF). Have you been able to trace the recursion in a debugger and actually see it, or have you gotten a "maximum stack depth exceeded" type error from Chrome?

Might be another clue ...

[briancavalier]

Ok, I think I did find some sort of infinite recursion by using Firebug's profiler. Still not sure exactly what's going on, but I'll spend some time on this a bit later today and post more info.

[KidkArolis]

I've seen this outside of jsfiddle first. I've put console.logs in a bunch of when.js internal functions and I saw all of those logs happen in Chrome's console over and over, thus my point about something recursing in when.js.

[briancavalier]

Cool, thanks for the info. I am def seeing some sort of recursion, but also not seeing the callstack grow infinitely, it's as if the recursion is actually happening across multiple turns of the javascript event loop. And most of the time in FF, I don't see it at all--it works correctly. I'll keep you posted, and let me know if you find anything else!

[briancavalier]

I found the issue. It's a rather odd interaction between $.Deferred and when.js, due to several factors:

1. the particular $.Deferred returned by $.get uses itself as the rejection value when it rejects.
2. when.js currently forwards a previous value when callbacks or errbacks return undefined
3. The success and fail callbacks in the fiddle return undefined

So, what happened was that the $.get $.Deferred rejected with itself as the rejection value, which was passed to the fail() callback. That callback returns undefined, so when.js forwarded the previous value, the already-rejected $.Deferred to a new when.js (because when.js transforms foreign promises into trusted, when.js promises). That already-resolved $.Deferred caused the new when.js promise to immediately be rejected ... with ... you guessed it, the same already-rejected $.Deferred (since that $.Deferred uses itself as its own rejection value!)

From there, the cycle just continued forever.

One potential workaround is to ensure that your promise callbacks and errbacks return something (other than the $.Deferred). Without knowing your application, though, I can't say for certain that will solve the problem.

I've pushed a potential fix to the issue-43 branch. Can you try it and let me know if it fixes the problem? Also, here is an html file that uses the issue-43 branch. You can save and run it locally to test:

<!DOCTYPE HTML>
<html lang="en-US">
<head>
    <meta charset="UTF-8">
    <title></title>
    <script src="http://code.jquery.com/jquery.js"></script>
    <script src="https://raw.github.com/cujojs/when/issue-43/when.js"></script>

    <script type="text/javascript">
        function success(val) { console.log('success', val); }
        function fail(e) { console.error('fail', e); }

        // Each of these should work without causing a cpu spike
        when($.get('fake.html'), success, fail);
//      when($.get('fake.html')).then(success, fail);
//      when.map(['1', '2', '3'], function(url) {
//          return $.get(url);
//      }).then(success, fail);
    </script>
</head>
<body>

</body>
</html>

[KidkArolis]

The issue-43 branch works perfectly with this test html you provided as well as in my app,
where it was failing previously.

As far as I understood you simply removed that forwarding of the previous value feature in case the callbacks return undefined? When would that feature be important?

I think some kind of fix for this problem is definitely needed as I'm sure someone else will run into the same problem when mixing when and jquery. when($.get('..')) is like the simplest thing you can use when.js for :)

Another possible way to solve this problem might be not converting foreign promises into when.js promises automagically. What if there was a function like when.convert that does that. So then you would do:

when.map(someBackboneModels, function (model) {
  // we know .save returns jquery deferred, so need to convert to when.js deferred
  return when.convert(model.save());
})

This would mean less magic and make developers more aware of those differences in promises. I'm not even sure this would solve this particular issue.

Anyways, the fix you made also seems to be working, so I would be happy to see that go live. Any doubts about it?

Thanks for the quick responses.

[briancavalier]

Cool, glad that works for you, and yep, you're right that the change was to stop forwarding the previous value when a callback returns undefined. That "feature" was implemented pretty early-on in when.js, and I've come to realize it's just plain wrong :)

So, I have no doubts that this change represents the correct behavior :) In fact, issue #31 is already tracking it, and it had already been implemented in the dev-20 branch. Right now, it's slated for when.js 2.0.0, but I think you're right that compatibility with $.get is important enough that it needs to go live sooner rather than later.

It's not entirely clear to me, based on semantic versioning rules, whether this change could be released in a 1.3.0 version, or whether it would require a 2.0.0 bump. It'll go out in the next release, be it numbered 1.3.0 or 2.0.0.

Since we know the answer to the specific problem, and #31 is already tracking the change, I'm closing this issue, but feel free to reopen if you run into any additional related problems.

[briancavalier]

Hey @KidkArolis, just wanted to let you know v1.3.0 is released and includes this fix. Cheers!