3

I am working with Nodejs. I have a forEach which is async as I have to wait for a result inside the forEach. As a result, I need to wait for the forEach to finish and then carry on with the result of the loop. I found several solutions for waiting for the forEach, one of them is using Promises. I did though, and these promises are created, however, the code after the forEach (and therefore the promises) are finished, is never actually executed (console.log is not printed). And the NodeJS function just ends without any errors.

Here is my Code:

var Client = require('ssh2').Client;

// eslint-disable-next-line no-undef
var csv = require("csvtojson");
// eslint-disable-next-line no-undef
var fs = require("fs");
// eslint-disable-next-line no-undef
const config = require('./config.json');
// eslint-disable-next-line no-undef
const os = require('os');
let headerRow = [];
let sumTxAmount = 0;

const filenameShortened = 'testFile';

let csvLists = [];
let csvFile;

const options = {
    flags: 'r',
    encoding: 'utf8',
    handle: null,
    mode: 0o664,
    autoClose: true
}

var conn = new Client();

async function start() {
    const list = await getCSVList();
    let content = fs.readFileSync('./temp.json', 'utf8');
    content = JSON.parse(content);
    var promises = list.map(function(entry) {
        return new Promise(async function (resolve, reject) {
            if (!content['usedFiles'].includes(entry.filename)) {
                const filename = entry.filename;
                csvFile = await getCsv(filename);
                csvLists.push(csvFile);
                console.log('here');
                resolve();
            } else {
                resolve();
            }
        })
    });
    console.log(promises)
    Promise.all(promises)
        .then(function() {
            console.log(csvLists.length, 'length');
        })
        .catch(console.error);
}

start();

The "here" is printed once (not 8 times as the arrays length is 8), but there are 8 promises created. The lower part where I am printing the length of the array is not executed.

Can anyone tell me what I am doing wrong? Am I using Promises and forEach falsely as I have to do an await inside the forEach?

Note: getCSVList() and getCsv() are functions to get Csvs from an sftp server:

function getCSVList() {
    return new Promise((resolve, reject) => {
            conn.on('ready', function () {
                conn.sftp(function (err, sftp) {
                        if (err) throw err;
                        sftp.readdir(config.development.pathToFile, function (err, list) {
                            if(err) {
                                console.log(err);
                                conn.end();
                                reject(err);
                            } else {
                                console.log('resolved');
                                conn.end();
                                resolve(list);
                            }
                        })
                })
            }).connect({
                host: config.development.host,
                port: config.development.port, // Normal is 22 port
                username: config.development.username,
                password: config.development.password
                // You can use a key file too, read the ssh2 documentation
            });
    })
}

function getCsv(filename) {
    return new Promise((resolve, reject) => {
        conn.on('ready', function () {
        conn.sftp(function (err, sftp) {
            if (err) reject(err);
            let csvFile = sftp.createReadStream(`${config.development.pathToFile}/${filename}`, options);
            // console.log(csvFile);
            conn.end();
            resolve(csvFile);
        })
    }).connect({
        host: config.development.host,
        port: config.development.port, // Normal is 22 port
        username: config.development.username,
        password: config.development.password
        // You can use a key file too, read the ssh2 documentation
    });
});
} 

The output in my console from all the console logs is:

`➜ node server.js
resolved
[ Promise { <pending> },
  Promise { <pending> },
  Promise { <pending> },
  Promise { <pending> },
  Promise { <pending> },
  Promise { <pending> },
  Promise { <pending> },
  Promise { <pending> } ]
here`
threxx
  • 1,213
  • 1
  • 31
  • 59
  • Probably `getCsv` would be more interesting than `getCSVList`, because this seems to be where it's blocked. – JeffRSon Jun 19 '19 at 09:23
  • @JeffRSon I added it now – threxx Jun 19 '19 at 09:24
  • I think you're not really reading from the stream. You should subscribe to its 'data' event or pipe it to a writable stream. Probably before ending the connection. – JeffRSon Jun 19 '19 at 09:30
  • Where is the var `csvLists` defined ? I tried to reproduce your code locally by declaring a `const csvLists = []` inside the `start()` function scope and it worked just fine... – BJRINT Jun 19 '19 at 09:30
  • @BJRINT I added it now. It was outside, I did not add that code – threxx Jun 19 '19 at 09:32
  • @JeffRSon what do you mean by "subscribe to its data event"? Where would I do that? It is actually a readable string, I printed the result BUT the method is only called once, then I get the "here" and then the node script ends – threxx Jun 19 '19 at 09:33
  • Please try to minimize your example to the necessary bits to reproduce the problem. See https://stackoverflow.com/help/minimal-reproducible-example – k0pernikus Jun 19 '19 at 09:40
  • 1
    @k0pernikus I did, but then users asked for more code. So I added it – threxx Jun 19 '19 at 09:41
  • I don't know which sftp package you're actually using, but if I'm not mistaken, csvFile is a readableStream, not the content of the file. So you may get the content from `csvFile.on('data', ...)` – JeffRSon Jun 19 '19 at 09:53
  • The example @k0pernikus was asking for should be *reproducible*. The code you posted wasn't self contained, so it wasn't reproducible. – JeffRSon Jun 19 '19 at 09:55
  • @JeffRSon can you post a link where you found that? I am using ssh2 (https://github.com/mscdex/ssh2-streams/blob/master/SFTPStream.md) but I couldn't find that in their documentation – threxx Jun 19 '19 at 10:10
  • This page reads: "Returns a new readable stream for path" - how you work with readable streams can be found in the official docs: https://nodejs.org/api/stream.html#stream_readable_streams – JeffRSon Jun 19 '19 at 10:36
  • but the stream is good, I need the stream later. I later work with the stream: csv({ noheader: true, output: "csv" }) .fromStream(csvFile) .then((rows) => { ... }) So I need the stream. Or cant I use the Stream like that? – threxx Jun 19 '19 at 10:36
  • I don't see any `forEach` in your code? – Bergi Jun 19 '19 at 10:40
  • [Never pass an `async function` as the executor to `new Promise`](https://stackoverflow.com/q/43036229/1048572)! – Bergi Jun 19 '19 at 10:40
  • Didn't know that. maybe it'll work. But you close the connection? – JeffRSon Jun 19 '19 at 10:41
  • @Bergi sorry it's not a "forEach" its a "map"now - Had to change it. Do you have an idea of how I could rewrite my code then? I do understand the problem but I have no clue how to fix that with what I want to achieve – threxx Jun 19 '19 at 10:46
  • Regarding promises, you seem to be doing mostly fine. Logging the array of promises is working as expected, but you probably don't want that: instead log the results of the `Promise.all(promises)` promises in its `then` callback. That you are getting only a single "here" probably means that the other 7 files are already included in the `usedFiles`. – Bergi Jun 19 '19 at 16:35

2 Answers2

1

Promise.all is a method that will return a promise object, but you are not waiting for your start method to execute.

function getCSVList() {
  return new Promise((resolve, reject) => {
    setTimeout(() => {
      resolve([1, 2, 3, 4]);
    }, 1000);
  });
}

function getCsv(params) {
  return new Promise((resolve, reject) => {
    setTimeout(() => {
      resolve(params);
    }, 1000);
  });
}

async function start() {
  const list = await getCSVList();
  const promises = list.map(item => {
    return new Promise(async function (resolve, reject) {
      const csvFile = await getCsv(item);
      console.log('here');
      resolve(csvFile);
    });
  });

  return Promise.all(promises);
}

start().then(res => {
  console.log(res);
});


mintsweet
  • 11
  • 4
  • Thanks for the idea. I tried that with my code, but "res" is never printed then – threxx Jun 19 '19 at 09:28
  • You can try putting csvFile in the resolve method.like this 'resolve(csvFile)' – mintsweet Jun 19 '19 at 09:33
  • You don't need to wrap `test` inside a new promise. It's already an async function. You could do `list.map(test)`. And then you could directetly `return Promise.all(list.map(test))`. – k0pernikus Jun 19 '19 at 09:36
1

Break up your problem into pieces, confirming they work along the way.

You are not using the stream correctly, among other things.

I made a working example with ssh2-sftp-client so you can maybe use it as a starting point.


Working example :

var fs = require('fs'); var _ = require('underscore');
var SFTPClient = require('ssh2-sftp-client');
const CONFIG = {
 "SSH_CONN_OPTS":{"host":"XXXXXXXX","port":22,"username":"XXXXXXXX","password":"XXXXXXXX"},
 "CSV_DIRECTORY":"/var/www/html"
}
//---------------
//.:The order-logic of the script is here
function StartScript(){
 console.log("[i] SSH Connection")
 LoadValidationFile(()=>{
  InitializeSFTP(()=>{ console.log("[+] SSH Connection Established")
   ListRemoteDirectory((list)=>{ console.log(`[i] Total Files @ ${CONFIG.CSV_DIRECTORY} : ${list.length}`)
    //console.log(list) //:now you have a 'list' of file_objects, you can iterate over to check the filename
    var csvFileList = [] //store the names of the files you will request after
    _.each(list,(list_entry)=>{ console.log(list_entry)
     if(!CONFIG.USED_FILES.includes(list_entry.name)){ csvFileList.push(list_entry.name) }
    }) 
    //:now loop over the new final list of files you have just validated for future fetch 
    GenerateFinalOutput(csvFileList)
   })
  })
 })
}
//.:Loads your validation file
function LoadValidationFile(cb){
 fs.readFile(__dirname+'/temp.json','utf8',(err,data)=>{ if(err){throw err}else{
  var content = JSON.parse(data)
  CONFIG.USED_FILES = content.usedFiles
  cb()
 }})
}
//.:Connects to remote server using CONFIG.SSH_CONN_OPTS
function InitializeSFTP(cb){
 global.SFTP = new SFTPClient();
 SFTP.connect(CONFIG.SSH_CONN_OPTS)
 .then(()=>{cb()})
 .catch((err)=>{console.log("[!] InitializeSFTP :",err)})
}
//.:Get a list of files from a remote directory
function ListRemoteDirectory(cb){
 SFTP.list(`${CONFIG.CSV_DIRECTORY}`)
     .then((list)=>{cb(list)})
     .catch((err)=>{console.log("[!] ListRemoteDirectory :",err)})
}
//.:Get target file from remote directory
function GetRemoteFile(filename,cb){
 SFTP.get(`${CONFIG.CSV_DIRECTORY}/${filename}`)
     .then((data)=>{cb(data.toString("utf8"))}) //convert it to a parsable string
     .catch((err)=>{console.log("[!] ListRemoteDirectory :",err)})
}
//-------------------------------------------
var csvLists = []
function GenerateFinalOutput(csv_files,current_index){ if(!current_index){current_index=0}
 if(current_index!=csv_files.length){ //:loop
  var csv_file = csv_files[current_index]
  console.log(`[i] Loop Step #${current_index+1}/${csv_files.length} : ${csv_file}`)
  GetRemoteFile(csv_file,(csv_data)=>{
   if(csv_data){csvLists.push(csv_data)}
   current_index++
   GenerateFinalOutput(csv_files,current_index)
  })
 }else{ //:completed
  console.log("[i] Loop Completed")
  console.log(csvLists)
 }
}
//------------
StartScript()

Good luck!

Community
  • 1
  • 1
EMX
  • 6,066
  • 1
  • 25
  • 32