mscdex / node-ftp

An FTP client module for node.js
MIT License
1.13k stars 243 forks source link

Not seeing error on close #86

Open animedbz16 opened 10 years ago

animedbz16 commented 10 years ago

I have a script that attempts queues up FTP connections to download a config file from 1000+ devices and runs them in parallel with async.parallelLimit.

My issue is that some of these devices may be offline or may not have DNS entries set up, but I am able to detect those in my logs on the 'error' event. Even though it detects the error "ENOTFOUND", calling c.destroy() does not call trigger the close even with an error.

c.on('error', function(err){
  if (err){
    log.error({host: host}, "FTP error: "+err);
  } else {
    log.error({host: host}, "FTP error: Unknown Error");
    myError = "FTP error Unknown Error";
  }
  c.destroy();
});
c.on('close', function(hadErr) {
  log.debug({host: host, hadErr: hadErr}, "FTP Connection Closed");
  if(hadErr){
    callback(null, {host: host, err: myError});
  } else {
    callback(null, {host: host});
  }
});

Host A - No DNS entry Host B - DNS Entry, but device is offline Host C - Normal Device - pulls down config file fine

"host":"A","msg":"FTP error: Error: getaddrinfo ENOTFOUND" "host":"A","hadErr":false,"msg":"FTP Connection Closed" "host":"B","msg":"FTP error: Error: Timeout while connecting to server" "host":"C","hadErr":false,"msg":"FTP Connection Closed "host":"B","msg":"FTP error: Error: connect ETIMEDOUT" "host":"B","hadErr":true,"msg":"FTP Connection Closed" results":[{"host":"A"},{"host":"B","err":{"code":"ETIMEDOUT","errno":"ETIMEDOUT","syscall":"connect"}},{"host":"C"}]

Host A doesnt trigger the haddErr: true Host B appears to time out and then will trigger the error Host C works properly.

Is this an improper use of c.destory when an error occurs? Or can it not properly call c.destroy when a connection has not been made since it could not resolve the DNS?