1
\$\begingroup\$

When is it better to instantiate the sftp client globally vs inside a function as shown below? What considerations favor one method over the other?

For a long running service that polls indefinitely, would it better to instantiate globally so I don't need to instantiate a new client on each run?

Also, is there any difference between using Bluebird's Promise.delay(10000) as opposed to wrapping setTimeout like so:

const timeout = async (ms) => new Promise(resolve => setTimeout(resolve, ms));

Finally, my polling function sleeps at the beginning of the function instead of at the end so my finally block frees up the sftp connection without having to wait 10000ms. Seems okay with the current recursive setup but I'm curious if there are "better" ways to write a recurring task.

Instantiate globally

const Client = require('ssh2-sftp-client');
const Promise = require('bluebird');

const sftp = new Client();

const listFiles = async (directory) => sftp.list(`/${directory}`);

const pollSftpServer = async () => {
  try {
    await Promise.delay(10000);
    await sftp.connect({
      host: '127.0.0.1',
      port: '2222',
      username: 'foo',
      password: 'pass',
    });
    const files = await listFiles('upload');
    console.log(files);
    return pollSftpServer();
  } catch (e) {
    console.error('caught error', e);
    return pollSftpServer();
  } finally {
    sftp.end();
  }
};

(() => {
  pollSftpServer();
})();

Instantiating inside a function

const Client = require('ssh2-sftp-client');
const Promise = require('bluebird');

const listFiles = async (sftp, directory) => sftp.list(`/${directory}`);

const pollSftpServer = async () => {
  // Instantiate here?
  const sftp = new Client();
  try {
    await Promise.delay(10000);
    await sftp.connect({
      host: '127.0.0.1',
      port: '2222',
      username: 'foo',
      password: 'pass',
    });
    const files = await listFiles(sftp, 'upload');
    console.log(files);
    return pollSftpServer();
  } catch (e) {
    console.error('caught error', e);
    return pollSftpServer();
  } finally {
    sftp.end();
  }
};

(() => {
  pollSftpServer();
})();
\$\endgroup\$
2
  • \$\begingroup\$ These examples look a bit contrived. Code Review questions should contain real code from a project — see the help center and How to Ask. What do you intend to do with the resulting file lists? Can you clarify the intended behavior when listFiles() fails? \$\endgroup\$ Sep 17, 2019 at 4:32
  • \$\begingroup\$ @200_success, this function's end goal would be to poll an Sftp server and download any new files. So once I get the file list, I would check the last modified time of each file and decide what to do from there. If listFiles() failed, it should enter the catch block and poll the sftp server again after waiting another 10s. \$\endgroup\$ Sep 17, 2019 at 4:58

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.