0

In a public library spex that I wrote awhile ago and that proved to be very reliable, I've been struggling to locate the reason why Bluebird is warning me with Warning: a promise was created in a handler but was not returned from it.

After trying to nail it time and again, I've almost given up, and now ready to place the maximum bounty on a solution.

Complete test application:

'use strict';

var promise = require('bluebird');
var spex = require('spex')(promise);

function factory(index) {
    if (index < 2) {
        return promise.resolve(index);
    }
}

spex.sequence(factory)
    .then(data=> {
        console.log("success");
    })
    .catch(error=> {
        console.log("error");
    });

Complete console output:

Warning: a promise was created in a handler but was not returned from it
    at Object.factory (D:\NodeJS\tests\test4.js:8:24)
    at loop (D:\NodeJS\tests\node_modules\spex\lib\utils.js:69:44)
    at Object.resolve (D:\NodeJS\tests\node_modules\spex\lib\utils.js:90:9)
    at loop (D:\NodeJS\tests\node_modules\spex\lib\ext\sequence.js:106:28)
    at next (D:\NodeJS\tests\node_modules\spex\lib\ext\sequence.js:160:25)
    at $utils.resolve.call.reject.index (D:\NodeJS\tests\node_modules\spex\lib\ext\sequence.js:144:25)
    at loop (D:\NodeJS\tests\node_modules\spex\lib\utils.js:86:17)
    at D:\NodeJS\tests\node_modules\spex\lib\utils.js:80:25
    at processImmediate [as _immediateCallback] (timers.js:383:17)
From previous event:
    at loop (D:\NodeJS\tests\node_modules\spex\lib\utils.js:77:22)
    at Object.resolve (D:\NodeJS\tests\node_modules\spex\lib\utils.js:90:9)
    at loop (D:\NodeJS\tests\node_modules\spex\lib\ext\sequence.js:106:28)
    at D:\NodeJS\tests\node_modules\spex\lib\ext\sequence.js:184:9
From previous event:
    at promise (D:\NodeJS\tests\node_modules\spex\lib\index.js:96:24)
    at Object.sequence (D:\NodeJS\tests\node_modules\spex\lib\ext\sequence.js:100:12)
    at Object.sequence (D:\NodeJS\tests\node_modules\spex\lib\ext\sequence.js:193:29)
    at Object.<anonymous> (D:\NodeJS\tests\test4.js:12:6)
    at Module._compile (module.js:409:26)
    at Object.Module._extensions..js (module.js:416:10)
    at Module.load (module.js:343:32)
    at Function.Module._load (module.js:300:12)
    at Function.Module.runMain (module.js:441:10)
    at startup (node.js:139:18)
    at node.js:968:3

success
  • SPEX library version - any
  • Bluebird version - any 3.X, with DEBUG mode on.
  • Node.js version - any
  • OS - any
vitaly-t
  • 24,279
  • 15
  • 116
  • 138
  • https://github.com/petkaantonov/bluebird/blob/master/docs/docs/warning-explanations.md – Amit Jun 15 '16 at 22:08
  • @Amit, thank you, but I've read the entire Bluebird documentation 10 times over, including the cases where the same warning was reported. I've done a very good research prior to publishing my question. Normally, I would have found it by now, as I know promises very well, but i'm damn stuck on this one... – vitaly-t Jun 15 '16 at 22:09
  • 1
    And did you try returning `null` as suggested? – Amit Jun 15 '16 at 22:11
  • @Amit in a few places, yes, here and there, trying not to break the library's logic. – vitaly-t Jun 15 '16 at 22:13
  • 1
    Then start by returning `null` from every `onFulfilled` function (either "inline" anonymous or "standard" function) that doesn't return anything and see if that helps (that's the suggested resolution). You can start with the "low hanging fruit" - the `.then(data=> { ... })` function that doesn't return anything. – Amit Jun 15 '16 at 22:15
  • @Amit, monuments to be raised in your name, I've just found one place where returning `null` finally killed off the warnings. God bless! – vitaly-t Jun 15 '16 at 22:19
  • Your code appears fine, the problem seems to be in the `spex` library, most like it's `loop` function. You should file a bug there. – Bergi Jun 15 '16 at 22:32
  • @Bergi there are no bugs there, It's been tested thoroughly. The answer has already been published, and it is the correct one. – vitaly-t Jun 15 '16 at 22:36
  • @vitaly-t: The answer makes no sense - that particular callback does not create any promises. Either there's a bug in spex or in bluebird itself, there should be no warning even without the `return null`. – Bergi Jun 15 '16 at 22:38
  • @Bergi I believe this is a shortcoming in Bluebird, which I wanted to get rid off somehow. Those warnings aren't very reliable, that's a fact, and my case was one example where the Bluebird warning was wrong. – vitaly-t Jun 15 '16 at 22:41
  • @vitaly-t: Those warnings aren't always useful if the calls are deeply nested, but by quickly looking through the spex sources I've already found 4 places where functions that create promises internally but don't return them are called from promise callbacks. https://github.com/vitaly-t/spex/blob/master/lib/ext/sequence.js#L131 https://github.com/vitaly-t/spex/blob/master/lib/ext/sequence.js#L164 https://github.com/vitaly-t/spex/blob/master/lib/utils.js#L80 https://github.com/vitaly-t/spex/blob/master/lib/utils.js#L83 There are probably more. You should report a bug. – Bergi Jun 15 '16 at 22:53
  • @vitaly-t: Oh wait, I just realised it's your own library. You should fix them, in that case. – Bergi Jun 15 '16 at 22:54
  • @Bergi, yeah, but every promise I create there is settled implicitly, which Bluebird fails to pick up, and throws massive warnings. It is especially weak understanding things when one uses sudo-recursion, which is what I am using there. Anyway, I'm just happy at least to get rid of one of the annoying warnings. – vitaly-t Jun 15 '16 at 22:56
  • @vitaly-t: Well I have no idea what you mean by sudo-recursion, but you should try using promises like they were designed to be used :-) At least put `return null` statements in the lines I pointed out to prevent Bluebird from picking up those cases. If you are doing so many problematic things at once, Bluebird is having a hard time tracking down where the probable mistake was. – Bergi Jun 15 '16 at 23:03
  • @Bergi I know the promises spec quite well, but never came across anything saying that one is supposed to return `null` from a `.then` when the returned value isn't needed. Any reference, please? Or when you are saying `using promises like they were designed to be used` - do you mean "the way Bluebird was designed"? – vitaly-t Jun 15 '16 at 23:07
  • @vitaly-t: Reading the spec apparently didn't help with understanding what the best practises are :-) No, I really meant [how *promises* were designed](https://blog.domenic.me/youre-missing-the-point-of-promises/), and their monadic nature. Bluebird is just trying to prevent you from making [such mistakes](https://pouchdb.com/2015/05/18/we-have-a-problem-with-promises.html) by throwing warnings when it detects weird patterns in your code (and admittedly, your code has a lot of them). The `return null` is bluebird's way to opt out from the warning when you are intentionally doin something weird – Bergi Jun 15 '16 at 23:14
  • @Bergi I asked a very specific question, and you forward me to someone's subjective blog? And calling promise usage `weird` doesn't help either. – vitaly-t Jun 15 '16 at 23:32
  • @vitaly-t: No, I linked to the blog entry of one of the designers of the influential Promises/A+ specification that everyone follows today. There is no official reference about how promises should be used - it's just best practises and commons sense. I could've linked [my rules of thumb](http://stackoverflow.com/a/25756564/1048572) as well. Regarding "weird", your whole `sequence` function uses the [`Promise` constructor antipattern](http://stackoverflow.com/q/23803743/1048572), but I don't know whether that's intentional (for performance?), accidental, or even necessary in parts. – Bergi Jun 15 '16 at 23:41

1 Answers1

1

The solution (as suggested here) is to return null from onFulfilled functions that don't return anything else.

For example:

.then(data=> {
    console.log("success");
})

Should probably be:

.then(data=> {
    console.log("success");
    return null;
})
Amit
  • 45,440
  • 9
  • 78
  • 110