Getting new error using Async with Node 6.2.2 "Callback already called"

[shelleyp]

What version of async are you using?

3.9.5

Which environment did the issue occur in (Node version/browser version)

Node 6.2.2 and 5.0.71.52

What did you do? Please include a minimal reproducable case illustrating issue.

This app

var fs = require('fs'),
    async = require('async'),
    _dir = './data/';

var writeStream = fs.createWriteStream('./log.txt',
      {'flags' : 'a',
       'encoding' : 'utf8',
       'mode' : 0666});

async.waterfall([
   function readDir(callback) {
      fs.readdir(_dir, function(err, files) {
         callback(err,files);
      });
   },
   function loopFiles(files, callback) {
      files.forEach(function (name) {
         callback (null, name);
      });
   },

   function checkFile(file, callback) {
      fs.stat(_dir + file, function(err, stats) {
         callback(err, stats, file);
      });
   },
   function readData(stats, file, callback) {
      if (stats.isFile())
         fs.readFile(_dir + file, 'utf8', function(err, data){
           callback(err,file,data);
       });
   },
   function modify(file, text, callback) {
      var adjdata=text.replace(/somecompany\.com/g,'burningbird.net');
      callback(null, file, adjdata);
   },
   function writeData(file, text, callback) {
       fs.writeFile(_dir + file, text, function(err) {
          callback(err,file);
       });
   },

   function logChange(file, callback) {
       writeStream.write('changed ' + file + '\n', 'utf8',
                       function(err) {
          callback(err);
      });
   }
], function (err) {
         if (err) {
            console.log(err.message);
         } else {
            console.log('modified files');
         }
});

What did you expect to happen?

That I wouldn't get an error

What was the actual result?

I'm getting

/home/examples/public_html/learnnode2-examples/chap3/node_modules/async/dist/async.js:837
if (fn === null) throw new Error("Callback was already called.");
^

Error: Callback was already called.

The callback function in the following section of the code

function loopFiles(files, callback) {
files.forEach(function (name) {
callback (null, name);
});
},

Is lost on the second run.

I've run this code successfully through several versions of Node. I believe I did test this when Node 6 was first released.

In the second iteration, the function isn't null, but something is happening in the Async code resulting in this error.

[ex1st]

Because you called callback many times in here:

function loopFiles(files, callback) {
    files.forEach(function(name) {
        callback(null, name);
    });
},

Please use async.forEach for this case.

[shelleyp]

Interesting. I have had no problems with previous versions of Node, and in fact, just ran this successfully in Node 6.0.0 in my Windows machine.

So, something has changed between 6.0.0 and 6.2.2 that causes Async to break with the use of built-in forEach.

[shelleyp]

Just an FYI, async.forEach doesn't work. It doesn't crash, but it doesn't work.

   function loopFiles(files, callback) {
      async.forEach(files, function (name, callback) {
         callback (null, name);
      });
   },

And frankly, I'm not sure it's the forEach causing the problem. If I'm using it incorrectly, do let me know.

[shelleyp]

OK, I give up.

I updated my Windows machine to 6.2.2, and this code works. But it doesn't work on my Linux machine. Both are built on the same V8 engine. Both are using same version of Async. Both have same example file directories.

In Linux, error is thrown. In Windows, not.

[megawac]

@shelleyp your waterfall is structured improperly for multiple files. I think you want to separate it into two waterfalls or have each step expect an array.

The reason your async.forEach change didn't work is because you're not calling the loopFiles callback. That function isn't doing what you want it to anyway, if you only want to process the firstFile change loopFiles to (files, callback) => callback(null, files[0]) otherwise you're going to need to have checkFiles, readData and modify expect arrays (or create a second waterfall)

[megawac]

I'm not going to fix all your code, but this is how I would change your checkFiles function

async.waterfall([
   function readDir(callback) {
      fs.readdir(_dir, callback);
   },

   function checkFile(files, callback) {
      async.map(files, (file, cb) => {
         fs.stat(_dir + file, function(err, stats) {
            cb(err, {stat: stats, file: file});
         });
      }, (err, stats) => {
         callback(err, stats);
      });
   },

....

[shelleyp]

That's fine.

Just know that this worked until 6.2.2, so whatever caused my code not to work may cause other people's code not to work.

[megawac]

Odd the only thing that should make that code work is if the readdir lists
one file (it is also broken for 0 files).

Do you mind double checking the output of readdir in both versions of nod
On Jun 27, 2016 5:55 PM, "Shelley Powers" notifications@github.com wrote:

That's fine.

Just know that this worked until 6.2.2, so whatever caused my code not to
work may cause other people's code not to work.

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#1199 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ADUIEAfvkwCmdjmkIZIjipFGYgPQ6SzYks5qQEbpgaJpZM4I_Nsg
.

[shelleyp]

This code has always worked with multiple files. Well, until 6.2.2 on Linux.

But just checked on Windows. Yup, worked with four files.

Output of readdir is [ 'data1.txt', 'data2.txt', 'data3.txt', 'data4.txt' ]

input to loopFiles is the same.

At that point in time, with forEach, each callback gets an individual file name.

If I had ever run into a brick wall with the array, I wouldn't have used this approach. But it always worked.

No biggie, I can always update the code to remove the array handling. The real issue is it works in one environment but not another. Yet Node, V8, and Async are the same versions.

[megawac]

I just ran slightly modified code in Ubuntu 16.04 in node 6.0.0 and 6.2.2. In both cases I got the same output:

• data1.txt, callback for the first item (data1.txt)
• Error: callback was already called

Modified script:

var fs = require('fs'),
    async = require('async'),
    _dir = './data/';

async.waterfall([
   function readDir(callback) {
      fs.readdir(_dir, callback);
   },
   function loopFiles(files, callback) {
      files.forEach(function (name) {
         callback (null, name);
      });
   },
   console.log.bind(console)
], console.error)

$ ls data
> data1.txt  data2.txt

[shelleyp]

But you didn't try it in Windows.

[megawac]

Sure, I'm closing it as not an async issue.

On Mon, Jun 27, 2016 at 8:20 PM, Shelley Powers notifications@github.com
wrote:

But you didn't try it in Windows.

—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
#1199 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ADUIEDNwRmcEjcAnVmSCHBBIHvdM6DL3ks5qQGiwgaJpZM4I_Nsg
.