danielzzz / node-ping

a poor man's ping library (using udp scanning) for node
MIT License
332 stars 105 forks source link

missing error messages from stderr #158

Open wonderfulShrineMaidenOfParadise opened 1 year ago

wonderfulShrineMaidenOfParadise commented 1 year ago

Reproduction steps

Ping something which returns errors.

Expected behavior

Messages from stderr are included in the responses.

{
  inputHost: 'p.p',
  host: 'unknown',
  alive: false,
  output: 'ping: permission denied (are you root?)\n',
  time: 'unknown',
  times: [],
  min: 'unknown',
  max: 'unknown',
  avg: 'unknown',
  stddev: 'unknown',
  packetLoss: 'unknown'
}
{
  inputHost: '1.1.1.1',
  host: '1.1.1.1',
  alive: false,
  output: 'PING 1.1.1.1 (1.1.1.1): 56 data bytes\nping: permission denied (are you root?)\n',
  time: 'unknown',
  times: [],
  min: 'unknown',
  max: 'unknown',
  avg: 'unknown',
  stddev: 'unknown',
  packetLoss: 'unknown',
  numeric_host: '1.1.1.1)'
}

Actual Behavior

The responses don't have messages from stderr included.

Relevant issue: louislam/uptime-kuma#2896

{
  inputHost: 'p.p',
  host: 'unknown',
  alive: false,
  output: '',
  time: 'unknown',
  times: [],
  min: 'unknown',
  max: 'unknown',
  avg: 'unknown',
  stddev: 'unknown',
  packetLoss: 'unknown'
}
{
  inputHost: '1.1.1.1',
  host: '1.1.1.1',
  alive: false,
  output: 'PING 1.1.1.1 (1.1.1.1): 56 data bytes\n',
  time: 'unknown',
  times: [],
  min: 'unknown',
  max: 'unknown',
  avg: 'unknown',
  stddev: 'unknown',
  packetLoss: 'unknown',
  numeric_host: '1.1.1.1)'
}

Operating System and Arch

Alpine 3.17 armv7

Ping

BusyBox v1.35.0

NodeJS Version

18.14.2

mondwan commented 1 year ago

Hello, errors reported here are too generic. Please include how you run the library, actual responses, and expected responses.

As mentioned in louislam/uptime-kuma/issues/2896, please also include how your system behaved while running the system ping if you believed the errors are coming from the system instead.

wonderfulShrineMaidenOfParadise commented 1 year ago

I updated the thread with outputs. We can know that output is empty if there is nothing in stdout. We should also include stderr for more info for debugging issues.

mondwan commented 1 year ago

Hopefully, #159 should resolve this bug

mondwan commented 1 year ago

Try release 0.4.4

wonderfulShrineMaidenOfParadise commented 1 year ago
{
  inputHost: 'p.p',
  host: 'bad',
  alive: false,
  output: "ping: bad address 'p.p'\n",
  time: 'unknown',
  times: [],
  min: 'unknown',
  max: 'unknown',
  avg: 'unknown',
  stddev: 'unknown',
  packetLoss: 'unknown',
  numeric_host: 'ddres'
}
{
  inputHost: '1.1.1.1',
  host: '1.1.1.1',
  alive: false,
  output: 'PING 1.1.1.1 (1.1.1.1): 56 data bytes\n' +
    'ping: permission denied (are you root?)\n',
  time: 'unknown',
  times: [],
  min: 'unknown',
  max: 'unknown',
  avg: 'unknown',
  stddev: 'unknown',
  packetLoss: 'unknown',
  numeric_host: '1.1.1.1)'
}

Thank you for fixing!

mondwan commented 1 year ago

That's great.

Just wondering. Hopefully, you are using BusyBox or something like that. Can you also show me the value of "os.platform()"?

The value in the numeric host field is bad. But, it is probably due to picking a wrong parser. The library should pick a Mac base parser for your case

On Sat, 11 Mar 2023, 22:13 Raymond Hackley, @.***> wrote:

{ inputHost: 'p.p', host: 'bad', alive: false, output: "ping: bad address 'p.p'\n", time: 'unknown', times: [], min: 'unknown', max: 'unknown', avg: 'unknown', stddev: 'unknown', packetLoss: 'unknown', numeric_host: 'ddres' } { inputHost: '1.1.1.1', host: '1.1.1.1', alive: false, output: 'PING 1.1.1.1 (1.1.1.1): 56 data bytes\n' + 'ping: permission denied (are you root?)\n', time: 'unknown', times: [], min: 'unknown', max: 'unknown', avg: 'unknown', stddev: 'unknown', packetLoss: 'unknown', numeric_host: '1.1.1.1)' }

Thank you for fixing!

— Reply to this email directly, view it on GitHub https://github.com/danielzzz/node-ping/issues/158#issuecomment-1465035596, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA5YBIZQBLZTHPVQO7YKMVDW3T2I7ANCNFSM6AAAAAAVTNQMSI . You are receiving this because you commented.Message ID: @.***>

wonderfulShrineMaidenOfParadise commented 1 year ago

Can you also show me the value of "os.platform()"?

linux. I don't really understand. Do you need to know something specific?

mondwan commented 1 year ago

I mean I would like to know the value from the module in node js os.platform().

In your captured sample, the value of the field numeric_host is 1.1.1.1), the ending bracket is something unexpected. Hopefully, we can fix this buggy value by picking a correct parser in the first place

Given that the format of ping's output depends on the underlying platform, there are 3 kinds of parsers in this library: MAC / LINUX / WINDOW. This library will pick the corresponding parser depends on the value from os.platform().

The problematic numeric_host value can be resolved as long as a MAC based parser being picked. That's why I would like to know the value of os.platform() for your underlying system. Then, I can fix the factory accordingly.

mondwan commented 1 year ago

Otherwise, I need to fix the linux parser accordingly. lol

wonderfulShrineMaidenOfParadise commented 1 year ago

Yes os.platform() returns linux and it seems to be correct.