NodeJS - Looping through Array Sequentially with Timeout between each Element in Array

Question

I have a list of commands in an array that I need to run in order:

const commands = [
  `git clone https://github.com/EliLillyCo/${repo}.git`,
  `cd ${repo}`, `git checkout -b ${branch}`,
  'cp ../codeql-analysis.yml .github/workflows/',
  'git add .github/workflows/codeql-analysis.yml',
  `git push --set-upstream origin ${branch}`,
  'cd ../',
  `rm -r  ${repo}`,
];

They need to be ran in order as the commands rely on the previous command being ran.

Also, each command needs to have a 3 second wait before running the next command, because sometimes commands take time, especially command 1 and command 5.

I am using a standard for loop which is then using setTimeout() that calls a function to run the commands, as such:


const a = require('debug')('worker:sucess');
const b = require('debug')('worker:error');

const { exec } = require('child_process');

function execCommand(command) {
  exec(command, (error, stdout, stderr) => {
    if (error) {
      b(`exec error: ${error}`);
      return;
    }
    a(`stdout: ${stdout}`);
    b(`stderr: ${stderr}`);
  });
}

const commands = [
   `git clone https://github.com/EliLillyCo/${repo}.git`,
   `cd ${repo}`, `git checkout -b ${branch}`,
   'cp ../codeql-analysis.yml .github/workflows/',
   'git add .github/workflows/codeql-analysis.yml',
   `git push --set-upstream origin ${branch}`,
   'cd ../',
   `rm -r  ${repo}`,
 ];

for (let i = 0; i < commands.length; i++) {
  setTimeout(execCommand(commands[i]), 3000);
}

But there is something wrong with the setTimeout() as it's returning this:

  worker:error TypeError [ERR_INVALID_CALLBACK]: Callback must be a function. Received undefined

What is the best way to approach the problem of looping through an array sequentially, whilst using a timeout?

The correct syntax is either `setTimeout( execCommand, 3000)` or `setTimeout( () => { execCommand(commands[i]) } , 3000) ` — Jeremy Thille, Jan 27 '21 at 10:42

T.J. Crowder · Accepted Answer · 2021-01-27T11:14:01.637

2

I'd make execCommand return a promise so you know when it's done; you can't rely on timeouts (what if the task takes more than three seconds?) and since most of those commands will complete much faster than that, the timeouts hold things up unnecessarily.

Here's execCommand returning a promise:

function execCommand(command) {
    return new Promise((resolve, reject) => {
        exec(command, (error, stdout, stderr) => {
            if (error) {
                b(`exec error: ${error}`);
                reject(error);
                return;
            }
            a(`stdout: ${stdout}`);
            b(`stderr: ${stderr}`);
            resolve();
        });
    });
}

Then if you have top-level await available (modern Node.js and ESM modules):

// If you have top-level `await` available
try {
    for (const commmand of commands) {
        await execCommand(command);
    }
} catch (error) {
    // ...report/handle error...
}

If you don't, wrap it in an async IIFE:

(async () => {
    for (const commmand of commands) {
        await execCommand(command);
    }
})().catch(error => {
    // ...report/handle error...
});

Alternatively, you could use util.promisify on exec directly if you wanted to separately the execution from the handling of stdout/stderr, but doing them together was the minimal change to what you had, so that's what I stuck with.

edited Jan 27 '21 at 11:14

answered Jan 27 '21 at 10:45

T.J. Crowder

1,031,962
187
1,923
1,875

Is `await execCommand(commands);` meant to be: `await execCommand(command);` – user3180997 Jan 27 '21 at 10:56
It still isn't waiting for the previous command to finish. The commands still seem to be running before the last one has finished? – user3180997 Jan 27 '21 at 11:08
@user3180997 - Yes, `execCommand(commands)` was meant to be `execCommand(command)`, sorry about that. According to [the documentation](https://nodejs.org/api/child_process.html#child_process_child_process_exec_command_options_callback), `exec` doesn't call its callback until the process exits. So if you think the promise is being settled before that, it suggests one of three things: 1. You've not quite implemented the above correctly (with apologies :-) ), or 2. The command *is* complete and what you're seeing has a different explanation, or ... – T.J. Crowder Jan 27 '21 at 11:13
... 3. The process you're calling just *starts* another thing that doesn't prevent the process you're creating from terminating, and continues its work in the background. But the `execCommand` above will definitely wait for the process it starts to exit, at least according to the Node.js documentation. – T.J. Crowder Jan 27 '21 at 11:13
No worries, just wanted to double check. Well when I put `cd ../` as my first command and `pwd` as my second command, the `pwd` prints out the directory that I am currently in and not the directory that `cd ../` should put me in. So I think it either isn't running `cd ../` or `pwd` is coming first. – user3180997 Jan 27 '21 at 11:16
@user3180997 - I didn't even look at the commands. :-) `cd` only changes the directory for the process in which it runs, it has no effect on *subsequent* processes you run. If you want to set the directory for those processes, you need to use the `cwd` option to `exec` to tell it what directory to use when spawning those processes. Or alternatively put those commands in a batch file and just execute the shell telling it to execut the batch, which also handles doing them in sequence. – T.J. Crowder Jan 27 '21 at 11:22

score 0 · Answer 2 · answered Jan 27 '21 at 10:53

0

Currenty you can't guarantee that the previous command will be completed when calling the next one. You call the next one automatically after 3000ms, but the previous one can take longer than expected and not be over yet.

You should add a mechanism to await each command, then launch the next one. Here's how using async/await :

const util = require('util');
const exec = util.promisify(require('child_process').exec);

const commands = [ ... ];

const execCommand = async (command) => {
    try {
        await exec(command)
    } catch (error) {
        b(`exec error: ${error}`);
        return;
    }
    a(`stdout: ${stdout}`);
    b(`stderr: ${stderr}`);
}

(async () => {
    for (let command of commands) {
        await execCommand(command);
    }
})();

answered Jan 27 '21 at 10:53

Jeremy Thille

26,047
12
43
63

Thanks for the response .It still isn't waiting for the previous command to finish. The commands still seem to be running before the last one has finished? – user3180997 Jan 27 '21 at 11:08
Uh, I don't see how because everything returns a Promise and is `await`ed in order – Jeremy Thille Jan 27 '21 at 11:10
I know, I have added a `cd ../` and `pwd` one after another and the pwd is printing out before the cd has happened :/ – user3180997 Jan 27 '21 at 11:12

NodeJS - Looping through Array Sequentially with Timeout between each Element in Array

2 Answers2