1

This is my first time i am working with promises , So i have use bluebird library to achieve my task, In below code i have string input from client that i have to search in fileSystem. So i have to loop through dir and check if that string matched with any file push that line to results so once forEach finish i want to send results to client. How can i achieve that task using promises ? error with below code

Error: Unhandled rejection TypeError: expecting an array or an iterable object but got [object Null]

app.js

   searchFileService.readFile(searchTxt, logFiles, function(lines, err) {
     console.log('Logs', lines);
     if (err)
         return res.send();
     res.json(lines);
 })

promises.js

function readFile(str,logFiles,callback){
   searchStr = str;
    return Promise.map(logFiles, function(logfile) {

        return new Promise(function(resolve, reject) {
             fs.readFile('logs/dit/' + logfile.filename, 'utf8', function(err, data) {
                               if (err) {
                                   callback(null,err);
                                   return; // **** Make sure not to run the code below!
                               }
                               var lines = data.split('\n'); // get the lines
                               lines.forEach(function(line) { // for each line in lines
                                   if (line.indexOf(searchStr) != -1) { // if the line contain the searchSt
                                       results.push(line);
                                   }
                               });
                            });
        })
    });
//}
}
Promise
    .all(readFile())
    .then(function() {
        console.log('done');
        callback(results);
    });
hussain
  • 6,587
  • 18
  • 79
  • 152
  • You forgot to add arguments when you are calling readFile()! – Paulin Trognon Mar 01 '17 at 15:50
  • those params i am getting from client , how can i pass it to promise it will throw undefined when i will start application. – hussain Mar 01 '17 at 15:51
  • Personally, I find promises are ugly as hell. [Async.js](https://github.com/caolan/async) is way cleaner to read and understand. – Mikey Mar 01 '17 at 15:52
  • hmm I was using Async but i could not resolve issue with that , see here question asked http://stackoverflow.com/questions/42519121/callback-is-already-called-using-async?noredirect=1#comment72175283_42519121 – hussain Mar 01 '17 at 15:55
  • @Mikey error handling in nodebacks is even uglier than hell, and there's nothing in async.js that relieves us of them. – Bergi Mar 01 '17 at 16:03
  • @hussain That's because every [answer](http://stackoverflow.com/a/42519195/1022914) is calling the `callback` incorrectly. You probably won't see my edit, but the arguments are supposed to be used as `callback(err);` and `callback(null, results);`. – Mikey Mar 01 '17 at 16:07

3 Answers3

2

Update

I posted some general advice and then some answers were posted with code examples that are quite complicated so I'd like to add that using the right modules your readFiles() function can be quite compact:

function readFiles(str, files) {
  return Promise.all(files.map(
    file => rsp(fs.readFile(`logs/dit/${file}`, 'utf-8'))
       .split('\n').filter(l => l.includes(str))))
       .then(a => [].concat(...a));
}

The right modules being mz and rsp here (disclaimer, I'm the author of the rsp module so I'm clearly biased here). The mz module lets you use promised versions of fs methods and the rsp module lets you invoke methods on promise results in the future.

You need to use those modules as:

let fs = require('mz/fs');
let rsp = require('rsp');

The readFiles() function above can be used as:

readFiles('line 3', ['file1.txt', 'file2.txt', 'file3.txt'])
  .then(lines => console.log(lines.join('\n')))
  .catch(err => console.log('Error:', err));

and it actually works - I tested it with a bunch of files the the logs/dit directory.

Full example

let fs = require('mz/fs');
let rsp = require('rsp');

function readFiles(str, files) {
  return Promise.all(files.map(
    file => rsp(fs.readFile(`logs/dit/${file}`, 'utf-8'))
       .split('\n').filter(l => l.includes(str))))
       .then(a => [].concat(...a));
}

readFiles('line 3', ['file1.txt', 'file2.txt', 'file3.txt'])
  .then(lines => console.log(lines.join('\n')))
  .catch(err => console.log('Error:', err));

Summary

As you can see no loops are needed, no explicit callbacks, no new Promise() construction etc.

Original answer

If you want to use Promises with file system access in Node then use on of those modules:

Or use Bluebird's promisyfyAll:

to promisify the fs module in Node.

For example with mz you will be able to do things like:

return Promise.map(logFiles, file => fs.readFile('logs/dit/' + file, 'utf-8'));

to return a promise of the array of all those files contents in one line of code.

You can chain in further with:

return Promise.map(...).then(files => ...);

to get all files and work with them, or you can use something like:

return Promise.map(logFiles, file => fs.readFile('logs/dit/' + file, 'utf-8').then(content => {...});

and have a promise of an array of files contents after transformations.

rsp
  • 107,747
  • 29
  • 201
  • 177
0

You forgot to call "resolve", and you forgot to pass arguments, and it seems that you are using "results" as a global variable, which is bad.

Here's my correction of the code:

function readFile(str,logFiles){
   searchStr = str;
    return Promise.map(logFiles, function(logfile) {

        return new Promise(function(resolve, reject) {
             fs.readFile('logs/dit/' + logfile.filename, 'utf8', function(err, data) {
                               if (err) {
                                   reject(err);
                                   return; // **** Make sure not to run the code below!
                               }
                               var results = [];
                               var lines = data.split('\n'); // get the lines
                               lines.forEach(function(line) { // for each line in lines
                                   if (line.indexOf(searchStr) != -1) { // if the line contain the searchSt
                                       results.push(line);
                                   }
                               });
                               resolve(results);
                            });
        })
    });
}

Promise
    .all(readFile(searchTxt, logFiles))
    .then(function(results) {
        console.log('done');
        callback(results);
    });
Paulin Trognon
  • 762
  • 6
  • 17
0

The error TypeError: expecting an array or an iterable object but got [object Null] comes from passing no logFiles to your readFile() call, Promise.map will bitch about its first argument being undefined.

Also you must not use Promise.all(readFile(…)), as Promise.all does take an array of promises not a promise (which the Promise.map in your readFile function already returns).

But indeed, even if you fixed those, your code would still not work because you're never settling the promises you create. You need to actually call reject and resolve, not some callback:

function readFile(filename) {
    return new Promise(function(resolve, reject) {
        fs.readFile(filename, 'utf8', function(err, data) {
            if (err) reject(err); // <--
            else resolve(data); // <--
        });
    });
}

Now you can use that function in your loop:

function findFileLines(searchStr, logFiles) {
    return Promise.map(logFiles, function(logfile) {
        return readFile('logs/dit/' + logfile.filename).then(function(data) {
            var lines = data.split('\n'); // get the lines
            var result = lines.filter(function(line) {
                return line.indexOf(searchStr) != -1
            });
            return result;
        });
    })
}

findFileLines("…", [{filename:"…"}, …]).then(function(results) {
    console.log('done');
    callback(results);
});
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • so i should export `findFileLines` for app.js and call it from there – hussain Mar 01 '17 at 16:57
  • @hussain Yes, try that. – Bergi Mar 01 '17 at 17:17
  • @Bergi Having seen a lot of your answers about promises I'd love you to comment on the code in [my answer](http://stackoverflow.com/questions/42536130/how-to-use-promises-using-nodejs/42536416#42536416) to the same question. – rsp Mar 01 '17 at 17:24