node-netstat icon indicating copy to clipboard operation
node-netstat copied to clipboard

[Feature Request]: Throw Error if net-tools is not installed

Open nktnet1 opened this issue 1 year ago • 6 comments

Hello,

node-netstat currently does nothing if the command netstat is not found (i.e. net-tools is not installed). For example:

node-netstat does nothing

code source (click to view)
/**
 * index.js
 */

const netstat = require('node-netstat');
netstat(
  { filter: { protocol: 'tcp' }, limit: 1 },
  (data) => console.log(data)
);

Commands and outputs:

node-netstat$ which netstat
netstat not found
node-netstat$ node index.js 
node-netstat$ sudo pacman -S --noconfirm net-tools
resolving dependencies...
looking for conflicting packages...

Packages (1) net-tools-2.10-2

Total Installed Size:  0.51 MiB

:: Proceed with installation? [Y/n] 
(1/1) checking keys in keyring                                                              [------------------------------------------------------] 100%
(1/1) checking package integrity                                                            [------------------------------------------------------] 100%
(1/1) loading package files                                                                 [------------------------------------------------------] 100%
(1/1) checking for file conflicts                                                           [------------------------------------------------------] 100%
(1/1) checking available disk space                                                         [------------------------------------------------------] 100%
:: Processing package changes...
(1/1) installing net-tools                                                                  [------------------------------------------------------] 100%
:: Running post-transaction hooks...
(1/1) Arming ConditionNeedsUpdate...
node-netstat$ node index.js                       
{
  protocol: 'tcp',
  local: { port: 6600, address: null },
  remote: { port: NaN, address: null },
  state: 'LISTEN',
  pid: 1564
}
node-netstat$ which netstat
/usr/bin/netstat

This can be difficult to debug. On netstat's manual page, we have

Note

This program is obsolete. Replacement for netstat is ss. Replacement for netstat -r is ip route. Replacement for netstat -i is ip -s > link. Replacement for netstat -g is ip maddr.

so it's not uncommon for docker images and linux distributions do not ship with net-tools.

Would it be possible to simply throw an Error with a helpful message telling the user to install net-tools?

Thank you :)).

nktnet1 avatar Oct 30 '23 09:10 nktnet1

@nktnet1 do you currently receive an error from node-netstat when netstat is missing? I would guess something like an ENOENT error? Telling the user they must install some package is outside the scope of this project but I can certainly make sure it errors cleanly if netstat is missing or fails to run.

danielkrainas avatar Nov 06 '23 14:11 danielkrainas

@danielkrainas I don't think there's any error if net-tools is not installed - it just fails silently, like in my attached image, second command in the terminal.

nktnet1 avatar Nov 06 '23 14:11 nktnet1

@nktnet1 understood - I will look to make sure it errors cleanly if netstat isn't installed or in PATH.

danielkrainas avatar Nov 06 '23 14:11 danielkrainas

@nktnet1 I see the issue - you need to add a done handler to your netstat options and check the error returned. Here's an example:

netstat.parsers.dummy = 'dummy';
netstat.commands.dummy = { cmd: 'dummy', args:['test']};
netstat({ platform:'dummy',done: (err) => console.log('done: err=', err)})
done: err= Error: spawn dummy ENOENT
    at Process.ChildProcess._handle.onexit (node:internal/child_process:283:19)
    at onErrorNT (node:internal/child_process:478:16)
    at processTicksAndRejections (node:internal/process/task_queues:83:21) {
  errno: -2,
  code: 'ENOENT',
  syscall: 'spawn dummy',
  path: 'dummy',
  spawnargs: [ 'test' ]
}

aside - I do see there's a bug where the done handler is called twice, first time with the error and second time without but I'll fix this.

danielkrainas avatar Nov 06 '23 22:11 danielkrainas

(1) I'm using the synchronous version (somehow missed this in the original image of the issue, my bad) - is there a way to avoid the done handler and simply have the Error thrown?

Currently my usage is something like this (slightly simplified below):

const getNetstat = (port: number, host: string): SyncResult => {
  let results: SyncResult;
  netstat({
    sync: true,
    filter: { local: { port, host } },
    limit: 1,
  }, (ret) => {
    results = ret;
  });
  return results;
};

(2) Actually, on this note, it'll be awesome if the synchronous version can be written without the callback - can look into creating a PR if you think it's a good idea. For example,

const getNetstat = (port: number, host: string): SyncResult => {
  // synchronously return the result (instead of undefined) or throw an Error as appropriate
  return netstat({
    sync: true,
    filter: { local: { port, host } },
    limit: 1,
  });
};

or maybe export a netstatSync function and remove the sync option.


(3) Sorry off-topic again, but SyncResult from the function above (typescript @types/node-netstat) is aliased to type any - any chance we can fix this too?

  • https://github.com/DefinitelyTyped/DefinitelyTyped/blob/fbc33dd3590c31aa653079082d22500c9a191e65/types/node-netstat/index.d.ts#L53

Happy to make new issues/PRs for (2) and (3) to not conflate issues if you'd like. Thanks @danielkrainas!

nktnet1 avatar Nov 06 '23 23:11 nktnet1

  1. I'm going to have to say no for now and I wouldn't accept a PR change for that at the moment. It would be too much of a breaking change but I'm likely to go that route with v2
  2. Same as 1, I wouldn't want to add such a breaking change in this version.
  3. any would be correct since the sync version would return whatever your done handler returns as a value or void if you don't pass a done handler.

I don't use this project in anything personally and only maintain it as a courtesy to the community that finds it useful. That said, the time to finally sit down and write V2 I think is long past due - with the renewed activity on this project, I will see what I can do to make it happen. Thank you for your interest in the project!

danielkrainas avatar Nov 07 '23 02:11 danielkrainas