Pinger timeout should return an error

[wbollock]

If the pinger reaches timeout, no error is returned from OnFinish() and it makes it seem like the probe finished successfully, even if nothing was returned from the ping job.

pro-bing/ping.go

Lines 538 to 539 in 0999adf

	case <-timeout.C:
	return nil

I believe this should return an error indicating the timeout was reached before the ping job finished fully. Happy to make the PR if this sounds good

[lllama]

Adding a +1 for this. Given that an unreachable host will block on .Run() without a timeout, it would be great to have some way to determine whether the pinger timed out.

[fkr]

indeed. Especially since one specifically has to set the timeout (which I initially did not do and wondered why my pinger go routine sometimes would not properly return).
Does it make sense to also set a timeout by default when the pinger is initialized?

[fuomag9]

indeed. Especially since one specifically has to set the timeout (which I initially did not do and wondered why my pinger go routine sometimes would not properly return). Does it make sense to also set a timeout by default when the pinger is initialized?

I tried running with pinger.Timeout = 1000 but the result is that it exits and returns like it responded to the ping (which is not true)

[jmkng]

I may not understand, but a RunWithContext function exists, I'm using that to check for a timeout.

ctx, _ := context.WithTimeout(context.Background(), deadline)
err = client.RunWithContext(ctx)
	if err != nil {
		if errors.Is(err, context.DeadlineExceeded) {
			// Timeout
		}
		...
	}

Does this not work for your situation?