apaszke / tcp-ping

TCP ping utility for node.js
MIT License
123 stars 27 forks source link

Ping 'err' argument always undefined #6

Closed taras-d closed 6 years ago

taras-d commented 6 years ago

Ping err argument always undefined regardless address valid or not.

Example:

const tcpp = require('tcp-ping');

// Ping VALID address
tcpp.ping({ address: 'google.com', timeout: 1000, attempts: 1 }, (err, data) => {
    console.log(err); // log `undefined`
});

// Ping INVALID address
tcpp.ping({ address: 'node', timeout: 1000, attempts: 1 }, (err, data) => {
    console.log(err); // log `undefined`
});
taras-d commented 6 years ago

After exploring data.results array I noticed that every result item will have err property if error occurred.

To determine if error occured I need to check if there is at least one item with error:

const isError = data.results.some(item => item.err);
apaszke commented 6 years ago

Hmm yeah it seems that the only places where errors are put is in data.results. It's not obvious that you want to aggregate them in err, because the fact that one of 10 attempts failed, doesn't mean that you consider this ping as failed (you should still get a reasonable estimate).

taras-d commented 6 years ago

You're right. One failed attempt doesn't mean that ping failed. I think is better to check if all attempts failed - data.results.every(item => item.err).

apaszke commented 6 years ago

It's not even clear to me that failure of all attempts means that the whole ping was unsuccessful. Maybe you just expected the host to be down? There was no issue with setting up the connections, they were just rejected.

taras-d commented 6 years ago

I just want to check if site available by sending ping to ip address.

I was confused by err which always undefined. Now I understand that this is just convention/standard to use "error-first" callbacks in node world).

Thanks for your responses!