cloud-health icon indicating copy to clipboard operation
cloud-health copied to clipboard

PingCheck's `path` parameter requires leading slash

Open tomi-bigpi opened this issue 3 years ago • 0 comments

The path argument for http.request needs to have the leading slash or the network request is invalid. The current code is misleading and results in errors, for example, in nginx logs similar to the following:

"GET health HTTP/1.1" 400 157 "-" "-" "-"

Currently PingCheck adds an extra slash to both the error message and call to super on the following lines:

  • https://github.com/CloudNativeJS/cloud-health/blob/master/src/healthcheck/checks/PingCheck.ts#L40
  • https://github.com/CloudNativeJS/cloud-health/blob/master/src/healthcheck/checks/PingCheck.ts#L44

This makes it seem like you shouldn't add the leading slash.

Additionally, as you can see from the nginx log output, a 400 error is returned in the situation I encountered, but this results in success in the current code path. I guess you could argue that a Ping check should just check connectivity, but it would at least be good to document the behavior in these types of error situations.

tomi-bigpi avatar Jul 07 '21 20:07 tomi-bigpi