Promise chain not then-able

Question

Having trouble figuring out why the 'describeDir' promise chain is not then-able.. Anyone have an idea what I messed up here? All of the code seems to execute, but any promise api function such as then or finally never gets executed. Two of the top level functions are shown below. The github repo is located at https://github.com/PhoenixContactUSA/pcworx-doc-gen

function updateDescriptor(fileloc, wsName, outdir){
  console.log('Updating descriptor file for: ' + wsName);
  return new Promise(function(resolve, reject){
    return getDescriptor(outdir).then(
      (value) => {
        let descriptorFile = value;

        var comments = getComments(fileloc);
        var variables = getVariables(fileloc);

        //wait until both are completed before continuing
        return Promise.all([comments, variables]).then((values) => {
          //var descriptor = new Object();
          //console.log(JSON.stringify(descriptor));
          descriptorFile[wsName] = new Object();
          //console.log(JSON.stringify(descriptor));

          //var worksheet = new Object();
          descriptorFile[wsName].comments = values[0];
          descriptorFile[wsName].variables = values[1];

          //save the file
          return saveDescriptor(descriptorFile, outdir).then((value) => {
            console.log('Completed ' + wsName + ' ' + value);
            resolve(value);
          }, (reason) => {console.log(reason)})

        }, (reason) => {
          console.log(reason);
        }

        )


      },
      (reason) => {console.log(reason)}
    )



  })



}


function describeDir(filedir, outdir){

  var files = findFilesInDir(filedir, '.XML');
  for (var k=0;k<files.length;k++){
    if ((files[k].indexOf('@HW') !== -1) || (files[k].indexOf('@LIBS') !== -1) || (files[k].indexOf('@ROOT') !== -1) || (files[k].indexOf('run') !== -1)) {
      files.splice(k,1);
    }
  }

  return Promise.each(files, function(file){
      return updateDescriptor(file, path.basename(file), outdir);
  });

}

Then I call the functions here. The code seems to execute just fine, but the then() is never called. Please note that I'm using bluebird in this latest revision.

  //generate the output files, then copy them to the destination
    docProcessor.describeDir(folder, path.join(__dirname, '..')).then((value)=>{
      console.log('docProcessor then entered: ' + value);
    });

Could you include or indicate the line on which you are attempting to `then` the promise chain? — TheHans255, Jul 28 '17 at 23:28
No errors.. It's node based and half the issue is that I don't get any debug info at the moment. — zmink1, Jul 28 '17 at 23:34
@JasonGoemaat - no errors, its node based and not getting any debug information besides the occasional console.log I've attempted to follow the flow with. — zmink1, Jul 28 '17 at 23:37
@TheHansinator later on in the code, I call updateDescriptor().then() in order to perform some last minute operations — zmink1, Jul 28 '17 at 23:37
@MinusFour that's what I thought. I was stabbing at thin air there. — zmink1, Jul 28 '17 at 23:38
I'm guessing you're getting an error somewhere. Basic code worked for me writing the unshown function definitions as "return Promise.resolve('xxx')". When you catch in a promise you'll have to bubble that error up if you want it to be received. For instance, if `saveDescriptor` throws an error you will never call `resolve` and the top-level promise will never get resolved or rejected. You should probably call `reject` in all those catch blocks. — Jason Goemaat, Jul 28 '17 at 23:50
Or if you just return the getDescriptor call instead of creating your own promise and change `resolve(value)` to `return value` that would work too. — Jason Goemaat, Jul 29 '17 at 00:03
@JasonGoemaat thanks for checking that. Totally goofed and realized that that function wasn't the source of the issue. I have added the believed source of the issue to an updated post. Somehow the describeDir function isn't then-able and I believe it has to do with how I'm writing the promise/iterator there — zmink1, Jul 29 '17 at 01:01
`describeDir` returns a `Promise.each` - which resolves to `files` by the way - so is "thenable" - try a `.catch` on `docProcessor.describeDir` to see if there's a rejection happening — Jaromanda X, Jul 29 '17 at 01:40
the for loop with `splice` is a minefield for problems, by the way, because you will "miss" testing entry `k+1` every time you splice - use filter instead — Jaromanda X, Jul 29 '17 at 01:46
Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! — Bergi, Jul 29 '17 at 02:40

Jaromanda X · Answer 1 · 2017-07-29T02:13:31.770

Firstly, to check if there's a rejection, try

docProcessor.describeDir(folder, path.join(__dirname, '..'))
.then(value => console.log('docProcessor then entered:', value))
.catch(reason => console.error('error', reason);

A potential problem in describeDir is the loop you use to filter out the files with @HW @LIBS @ROOT or run in the name

When you splice the files array on k, the k++ still gets executed, therefore you will skip testing the next file

i.e.

array = [a, b, c, d];
k == 1 // testing "b"
array.splice(k, 1);
now array = [a, c, d]
k++; // == 2
next iteration checks "d"

Therefore, if ever there are two files in a row with one of those strings in them, you will skip "removing" it - this could be the issue?

You want to use filter instead

function describeDir(filedir, outdir) {
    var files = findFilesInDir(filedir, '.XML')
    .filter(file => 
        file.indexOf('@HW') == -1 && 
        file.indexOf('@LIBS') == -1 && 
        file.indexOf('@ROOT') == -1 && 
        file.indexOf('run') == -1
    );

    return Promise.each(files, file => updateDescriptor(file, path.basename(file), outdir));
}

or neater

function describeDir(filedir, outdir) {
    var files = findFilesInDir(filedir, '.XML')
    .filter(file => !/@HW|@LIBS|@ROOT|run/.test(file));

    return Promise.each(files, file => updateDescriptor(file, path.basename(file), outdir));
}

As a bonus, the following is the updateDescriptor function cleaned up and flattened out and modernised with latest ES2015+ coding features (with your comments intact)

function updateDescriptor(fileloc, wsName, outdir) {
    console.log('Updating descriptor file for: ' + wsName);
    return getDescriptor(outdir)
    .then(value => Promise.all([getComments(fileloc), getVariables(fileloc), value]))
    .then(([comments, variables, descriptorFile]) => {
        //var descriptor = new Object();
        //console.log(JSON.stringify(descriptor));
        //console.log(JSON.stringify(descriptor));

        //descriptorFile[wsName] = new Object();

        //var worksheet = new Object();
        descriptorFile[wsName] = {comments, variables};
        //save the file
        return saveDescriptor(descriptorFile, outdir)
    }).then((value) => {
        console.log('Completed ' + wsName + ' ' + value);
        return value
    })
}

Note the lack of catch code, as you want the errors to persist down the chain

a truly compact version of updateDescriptor is

const updateDescriptor = (fileloc, wsName, outdir) => getDescriptor(outdir)
    .then(value => Promise.all([getComments(fileloc), getVariables(fileloc), value]))
    .then(([comments, variables, descriptorFile]) => 
        saveDescriptor(Object.assign(descriptorFile, { 
            [wsName] : { comments, variables }
        }), outdir)
    );

Thanks for the feedback, this is a great improvement on my code. Believe it or not, I'm still not getting a catch or then execution when describeDir is called, though the code is still executed. Perhaps there is a bug somewhere deeper in my code that Node is eating? I'll keep investigating before posting something more useful — zmink1, Jul 29 '17 at 03:19

Promise chain not then-able

1 Answers1