read http response body before closing it

[erka]

Per golang documentation, it isn't enough just to close the response body.
It's the best to read it before closing. Otherwise, there could be the resource leakage.

More information at https://pkg.go.dev/net/http#Client.Do

[kevinmcconnell]

Thanks @erka, I agree it's better to read the response before closing here.

I don't think there was any resource leakage in the previous form, though. Closing the body should be sufficient. But reading the body before closing makes the underlying connection available for re-use via keep-alive. In the case of these healthcheck requests that probably won't make a huge difference, but still, it's better (and tidier!) to do so 👍

[erka]

@kevinmcconnell it's much better. Thank you.

I have a question about the checker. Is there a specific reason you use a shutdown channel instead of context.WithCancel for the health checker? Using context.WithCancel would allow passing a cancellable context to NewRequestWithContext instead of context.Background, potentially yielding earlier results in some cases.

[kevinmcconnell]

@erka that's a good point! Using the context for cancellation also means we can simplify the logic a bit, since we'll always know from the error whether it was a cancel or a timeout. And we won't have to worry about prioritizing the cancel in the select, because once it's cancelled any further requests would also be cancelled.

I've switched to this approach in bc8525d. Thanks for the suggestion!