improve docs for response handlers #164
Conversation
client.go
Outdated
| // If an error is returned, the client's retry policy will be used to determine whether to retry the whole request. | ||
| // If an error is returned, the client's retry policy will be used to determine | ||
| // whether to retry the whole request (including this handler). | ||
| // NOTE: It only runs if the initial part of the request was successful. |
There was a problem hiding this comment.
"initial part of the request" and "successful" might be hard to follow. I would say:
The ResponseHandlerFunc is called when the HTTP client successfully receives a response and the CheckRetry function indicates that a retry of the base request is not necessary.
There was a problem hiding this comment.
Also just a nitpick - I think it's fine to just flow these like a normal sentence in the description rather than marking them specifically as NOTE:.
client.go
Outdated
| // whether to retry the whole request (including this handler). | ||
| // NOTE: It only runs if the initial part of the request was successful. | ||
| // Make sure to check status codes! Even if the request was "successful" it may have a non-2xx status code. | ||
| // NOTE: If there is a response body, users should make sure to close it to avoid a memory leak. |
There was a problem hiding this comment.
There will always be a response body to close when this function is called. That doesn't necessarily mean that the handler has to close it - that could also be done by the caller in certain cases. Maybe:
The response body is not automatically closed. It must be closed either by the ResponseHandlerFunc or by the caller out-of-band. Failure to do so will result in a memory leak.
The new "retryable response handler" functionality can be confusing to use, so this PR