How to pause between two asynchronous actions?

Question

I need to make pause between "/system/backup/save" and "/file/print". Because otherwise, the backup will not be completed before the contents of the "/file" directory are displayed. Now the code is performing a backup, but it gives me a list of files in which there is no backup yet.

const RouterOSAPI = require("node-routeros").RouterOSAPI;
const sleep = require('util').promisify(setTimeout);
var hosts = require('./config.json');

async function backup() {
    return new Promise(function (resolve, reject) {

        for (let elem of hosts) {
            const conn = new RouterOSAPI({
                host: elem.host,
                user: elem.user,
                password: elem.password
            })
            conn.connect()
                .then((client) => {
                    return conn.write(["/system/backup/save",]).then((data) => {
                        resolve('COMPLETE - OK');
                    }).catch((err) => {
                        reject('ERROR!');
                    });
                    sleep(5000);
                }).then(() => {
                return conn.write("/file/print").then((data2) => {
                    console.log(data2)
                    resolve('CHECK - OK');
                    conn.close();
                }).catch((err) => {
                    reject('ERROR!');
                });

            }).catch((err) => {
                reject('ERROR CONNECT TO ' + elem.name);
            });
        }
    });
}

backup();

Rather than sleep. Use a promise to await for the action to be completed. — John, Dec 28 '20 at 16:09
Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! The `resolve()` and `reject()` calls will never work properly with that loop. — Bergi, Dec 28 '20 at 16:16
Is there no way for the other end to tell you when the backup is complete? And separately: If you have an `async` function, you don't need to create the promise explicitly, and you usually don't want to use `.then` and such to attach handlers (use `await` instead). — T.J. Crowder, Dec 28 '20 at 16:16
What does `sleep(5000)` do? And it creates a promise, you must `return` that! Although @John is correct, you shouldn't be sleeping there at all but just wait for the write to complete. — Bergi, Dec 28 '20 at 16:18

T.J. Crowder · Answer 1 · 2020-12-28T16:36:15.157

Generally, using a delay to wait for completion of an asynchronous process is an anti-pattern you'll always end up either not waiting long enough or waiting an unnecessarily long time. The former is of course a bigger problem than the latter, but both are problems. If you have any means of having the other end report completion of the backup, that would be your best bet. Looking at the documentation, it seems like conn.write's promise shouldn't be fulfilled until the operation is complete, but I only skimmed the docs so maybe that's not the case.

Other than that:

Don't create the promise explicitly, your async function automatically creates a promise (but you may not want an async function here anyway)
Don't mix using .then/.catch handlers with async functions; use await.

For instance, here's a version that runs the backups and such in parallel and returns an array giving success/failure via allSettled:

const RouterOSAPI = require("node-routeros").RouterOSAPI;
const sleep = require('util').promisify(setTimeout);
var hosts = require('./config.json');

async function backup() {
    // Run the hosts in parallel
    return await Promise.allSettled(hosts.map(async (host) => {
        let conn;
        try {
            const c = new RouterOSAPI({
                host: elem.host,
                user: elem.user,
                password: elem.password
            })
            const client = await c.connect();
            conn = c;
            await conn.write(["/system/backup/save",]);
            await sleep(5000); // Only if really unavoidable because the 
                               // backup continues even after the promise
                               // from `write` is fulfilled
            await conn.write("/file/print");
            conn = null;
            c.close();
        } catch (e) {
            if (conn) {
                try {
                    conn.close();
                } catch {} // Don't let errors in close shadow previous errors
            }
            throw e;
        }
    }));
}

backup()
.then(results => {
    // Check for status = "rejected" entries in results and report the errors
});

But note that since that function just returns the promise from allSettled, you might not want an async function at all:

const RouterOSAPI = require("node-routeros").RouterOSAPI;
const sleep = require('util').promisify(setTimeout);
var hosts = require('./config.json');

function backup() {
    // Run the hosts in parallel
    return Promise.allSettled(hosts.map(async (host) => {
        let conn;
        try {
            const c = new RouterOSAPI({
                host: elem.host,
                user: elem.user,
                password: elem.password
            })
            const client = await c.connect();
            conn = c;
            await conn.write(["/system/backup/save",]);
            await sleep(5000); // Only if really unavoidable because the 
                               // backup continues even after the promise
                               // from `write` is fulfilled
            await conn.write("/file/print");
            conn = null;
            c.close();
        } catch (e) {
            if (conn) {
                try {
                    conn.close();
                } catch {} // Don't let errors in close shadow previous errors
            }
            throw e;
        }
    }));
}

backup()
.then(results => {
    // Check for status = "rejected" entries in results and report the errors
});

(There's a subtle difference between those two around what happens if hosts.map throws an error — perhaps because hosts isn't an array — but it's probably not important. The former returns a rejected promise, the latter throws a synchronous error.)

How to pause between two asynchronous actions?

1 Answers1