0

I'm playing around with promises to try and recursively walk a nested array structure. Here is my test code:

const Promise = require("Bluebird");

let allPaths = [
    "foo",
    "bar",
    [
        "baz",
        "bazzz",
        "bazzzzz",
        "xxx",
        [
            "never"
        ]
    ],
    "abc",
    [
        "tst",
        [
            "111",
            "222"
        ],
        "xxx"
    ],
    "zzz"
];

function processThisPath(path) {
    return new Promise(function(resolve) {
        if (typeof path === "string") {
            console.log(`Processing path ${path}`);
            return resolve(path);
        }
        else {
            return new Promise(function(resolve) {
                console.log("PROCESSING NEW LEVEL");
                return Promise.each(
                    path,
                    processThisPath
                );
            });
        }
    });
}

Promise.each(
    allPaths,
    function(currentPath) {
        return (
            processThisPath(currentPath)
            .catch(function(err) {
                console.log(`Cannot traverse '${currentPath}': ${err.message}`);
            })
        );
    }
);

It doesn't traverse the entire structure and I can't figure out why. The output is:

Processing path foo
Processing path bar
PROCESSING NEW LEVEL
Processing path baz
Processing path bazzz
Processing path bazzzzz
Processing path xxx
PROCESSING NEW LEVEL
Processing path never

Why does it not move on to "abc"? I'd have thought the initial Promise.each would move on to the next entry in the array, but it seems to have stopped when it got to the first array. Why?

Jez
  • 27,951
  • 32
  • 136
  • 233
  • Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! – Bergi Oct 11 '17 at 11:26
  • It stops when you never call `resolve` in the `new Promise(function(resolve) { console.log("PROCESSING NEW LEVEL"); … })` – Bergi Oct 11 '17 at 11:27
  • Adding `resolve();` doesn't change the output. – Jez Oct 11 '17 at 11:28
  • Also does the Promise constructor antipattern apply here? I don't have already-existing promises. If it does please reply with an answer and show how to fix it. – Jez Oct 11 '17 at 11:33
  • you certainly have multiple already-existing promises inside that `new Promise` call. The obvious one is the inner `new Promise`, the other is the return value of `Promise.each`. You should simply write `function processThisPath(path) { if (…) return Promise.resolve(…); else return Promise.each(paths, processThisPath); }` – Bergi Oct 11 '17 at 12:12
  • Ah, you also didn't call the `resolve()` of the outer promise in the `else` path, that's why it still stopped after the `each` now – Bergi Oct 11 '17 at 12:16
  • @Bergi Please give an example of how to fix this in an answer, I'm not really understanding your comments. – Jez Oct 11 '17 at 12:17
  • I think it's incorrect to imply that I am going to use the "already-existing" promises inside `processThisPath` (unless I am misunderstanding) - `return Promise.resolve()` still creates a new Promise and returns it, no? – Jez Oct 11 '17 at 12:33
  • But that's not inside a `new Promise` callback. Surely you have to create new promises inside `processThisPath`, but you should never put such inside a `new Promise`. Btw, where in your code does the actual asynchronous processing happen? Your example code doesn't actually need any promises. – Bergi Oct 11 '17 at 14:34
  • Bergi: I'm just structuring it for now. I want the "console.log" statements to be done asynchronously to simulate actual I/O work. And that's where I'm confused. This code: `if (typeof path === "string") { console.log(`Processing path ${path}`); return Promise.resolve(); }` will *immediately* execute the `console.log` and return the Promise, rather than executing the `console.log` as a promise callback. – Jez Oct 11 '17 at 14:51
  • To simulate actual IO, better use `new Promise(resolve => { console.log("Working"); setTimeout(resolve, 500); })` – Bergi Oct 11 '17 at 14:53
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/156481/discussion-between-jez-and-bergi). – Jez Oct 11 '17 at 14:53

1 Answers1

0

I was using the new Promise antipattern and I had forgotten that processThisPath was already running as a callback, called by Promise.each(...). Therefore it's OK to just do my simulation of the I/O operation directly:

function processThisPath(path) {
    if (typeof path === "string") {
        console.log(`Processing path ${path}`);
        return Promise.resolve();
    }
    else {
        console.log("PROCESSING NEW LEVEL");
        return Promise.each(path, processThisPath);
    }
}

Here, the Promise.resolve() which is returned is simulating the call to an async library that would be doing something like disk I/O, outside this thread. When using that library, the result of the async library function would be returned instead of Promise.resolve(). Note that although Promise.resolve() creates a Promise internally which is then returned, it avoids the antipattern by not explicitly using the new Promise constructor.

Jez
  • 27,951
  • 32
  • 136
  • 233