Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
nktnet1 opened this issue Oct 30, 2023 · 6 comments
Open

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

nktnet1 opened this issue Oct 30, 2023 · 6 comments
Labels

Comments

@nktnet1
Copy link

nktnet1 commented Oct 30, 2023

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 :)).

@danielkrainas
Copy link
Owner

@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.

@nktnet1
Copy link
Author

nktnet1 commented Nov 6, 2023

@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.

@danielkrainas
Copy link
Owner

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

@danielkrainas
Copy link
Owner

@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.

@nktnet1
Copy link
Author

nktnet1 commented Nov 6, 2023

(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?


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

@danielkrainas
Copy link
Owner

  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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants