Why can't I use then on a resolved promise?

Question

I have my function whose job is to go over a number of files (that use the values from the array as building blocks for file names) and download them using a reduce. It's more of a hack as of now but the Promise logic should work. Except it doesn.t

Here's my code:

function import_demo_files(data) {
    /**
     * Make a local copy of the passed data.
     */
    let request_data = $.extend({}, data);

    const get_number_of_files_1 = Promise.resolve({
        'data' : {
            'number_of_files' : 2
        }
    });

    return new Promise((resolve, reject) => {
        let import_files = get_number_of_files_1.then(function(response) {
            new Array(response.data.number_of_files).fill(request_data.step_name).reduce((previous_promise, next_step_identifier) => {
                let file_counter = 1;

                return previous_promise.then((response) => {
                    if( response !== undefined ) {
                        if('finished_import' in response.data && response.data.finished_import === true || response.success === false) {
                            return import_files;
                        }
                    }
                    const recursively_install_step_file = () => import_demo_file({
                        demo_handle: request_data.demo_handle,
                        'step_name': request_data.step_name,
                        'file_counter': file_counter

                    }).call().then(function(response) {
                        file_counter++;

                        if('file_counter' in response.data && 'needs_resume' in response.data) {
                            if(response.data.needs_resume === true) {
                                file_counter = response.data.file_counter;
                            }
                        }
                        return response.data.keep_importing_more_files === true ? recursively_install_step_file() : response
                    });
                    return recursively_install_step_file();
                }).catch(function(error) {
                    reject(error);
                });
            }, Promise.resolve())
        }).catch(function(error) {
            reject(error);
        });
        resolve(import_files);
    });
}

Now, when I do:

const import_call = import_demo_files({ 'demo_handle' : 'demo-2', 'step_name' : 'post' });
console.log(import_call);

The console.log gives me back that import_call is, in fact a promise and it's resolved. I very much like the way return allows me to bail out of a promise-chain, but I have no idea how to properly resolve my promise chain in there, so clearly, it's marked as resolved when it isn't.

I would like to do import_call.then(... but that doesn't work as of now, it executes this code in here before it's actually done because of the improper handling in import_demo_files.

That promise will resolve immediately because the `resolve()` callback handler is called outside of anything async. basically `new Promise((resolve, reject) => { let import_files = /* some promise exp */; resolve(import_files); })` actually looks like a promise that returns a promise — Wilhelmina Lohan, Jun 25 '19 at 15:10
Looks like you could do `import_call.then(request => request).then(...` — Wilhelmina Lohan, Jun 25 '19 at 15:13
I think you're right @WilliamLohan but doesn't that seem like a hack? I'm trying to find the underlying cause for this not working. — coolpasta, Jun 25 '19 at 15:15
I believe the problem is the line new Array(response.data.number_of_files).fill... you wanted to return that result as: return new Array(response.data.number_of_files).fill... — Alvaro Ccatamayo, Jun 25 '19 at 15:17
Why wrap in a new Promise, why not just `return get_number_of_files_1.then(...);` — Wilhelmina Lohan, Jun 25 '19 at 15:18
@Alvaro Ccatamayo is right there is no return statement in side the callback passed to `get_number_of_files_1.then(...)` so its like a "fire and forget" Promise the immediately resolves `undefined` — Wilhelmina Lohan, Jun 25 '19 at 15:27
@AlvaroCcatamayo This would just return the `Array` and nothing else. — coolpasta, Jun 25 '19 at 15:35
@coolpasta that line doenst return an array, its a reduce call on the array that returns a promise. Currently, said promise its not being returned and is treated as a fire and forget — Alvaro Ccatamayo, Jun 25 '19 at 16:07
Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! — Bergi, Jun 25 '19 at 16:42
It's not clear what data you want `import_demo_files()` to deliver. — Roamer-1888, Jun 25 '19 at 18:03
@Roamer-1888 It delivers a `response` object that always has the `success` and `data` keys when it's done. — coolpasta, Jun 25 '19 at 19:40
@Bergi I understand why it's bad but I have no clue on how to re-write it. What you typed is, I can tell, extremely valuable but have no idea how to make use of it. If you could put it into the answer, I promise to make good use of it and pay it back once I get better myself. — coolpasta, Jun 25 '19 at 19:42

Roamer-1888 · Accepted Answer · 2019-06-26T01:15:48.560

An asynchronous recursion inside a reduction isn't the simplest of things to cut your teeth on, and it's not immediately obvious why you would want to given that each iteration of the recursion is identical to every other iteration.

The reduce/recurse pattern is simpler to understand with the following pulled out, as outer members :

1. the `recursively_install_step_file()` function
1. the `new Array(...).fill(...)`, as `starterArray`
1. the object passed repeatedly to `import_demo_file()`, as `importOptions`)

This approach obviates the need for the variable file_counter, since importOptions.file_counter can be updated directly.

function import_demo_files(data) {
    // outer members
    let request_data = $.extend({}, data);
    const importOptions = {
        'demo_handle': request_data.demo_handle,
        'step_name': request_data.step_name,
        'file_counter': 1
    };
    const starterArray = new Array(2).fill(request_data.step_name);
    function recursively_install_step_file() {
        return import_demo_file(importOptions).then((res) => {
            if('file_counter' in res.data && 'needs_resume' in res.data && res.data.needs_resume) {
                importOptions.file_counter = res.data.file_counter; // should = be += ?
            } else {
                importOptions.file_counter++;
            }
            return res.data.keep_importing_more_files ? recursively_install_step_file() : res;
        });
    }

    // the reduce/recurse pattern
    return starterArray.reduce((previous_promise, next_step_identifier) => { // next_step_identifier is not used?
        let importOptions.file_counter = 1; // reset to 1 at each stage of the reduction?
        return previous_promise.then(response => {
            if(response && ('finished_import' in response.data && response.data.finished_import || !response.success)) {
                return response;
            } else {
                return recursively_install_step_file(); // execution will drop through to here on first iteration of the reduction
            }
        });
    }, Promise.resolve());
}

May not be 100% correct but the overall pattern should be about right. Be prepared to work on it some.

Incredible and much more terse. I was blindly following the "write it the JS way" when I needed to see clearly what's happening and this helps. **But I still don't understand why this works.** Any chance at explaining? — coolpasta, Jun 27 '19 at 01:10
I selected the answer because it works and I can tell how much better it is also in terms of legibility, but please add some explanations for my original question, I'd be forever grateful. — coolpasta, Jun 27 '19 at 03:08
@coolpasta, original code has multiple issues and I confess that I gave up trying to understand the decribed behaviour. I'm horrendously busy right now but will see if I can get back to this when things quieten down. — Roamer-1888, Jun 27 '19 at 14:54
In the original code, the main thing appears to be that in `previous_promise.then((response) {...})`, `response` is tested but not returned. It doesn't help that member name `response` is reused. Note that in my version, one `response` disappears completely and, for clarity, another has been changed to `res`. — Roamer-1888, Jun 27 '19 at 15:04

Why can't I use then on a resolved promise?

1 Answers1