Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Response body not being closed #401

Open
miquella opened this issue Jan 4, 2022 · 0 comments
Open

Response body not being closed #401

miquella opened this issue Jan 4, 2022 · 0 comments

Comments

@miquella
Copy link

miquella commented Jan 4, 2022

The Go documentation states:

If the returned error is nil, the Response will contain a non-nil Body which the user is expected to close. If the Body is not both read to EOF and closed, the Client's underlying RoundTripper (typically Transport) may not be able to re-use a persistent TCP connection to the server for a subsequent "keep-alive" request.
https://pkg.go.dev/net/http#Client.Do

However, the CodeFresh Go SDK is failing to close the response body in many instances.

For example, the workflow Get method makes a request (via (*codefresh).requestAPI) and then decodes it (via (*codefresh).decodeResponseInto), but never closes the response's Body.

Unfortunately, the way the code is currently structured will require that most calls to (*codefresh).requestAPI remember to defer resp.Body.Close(), so it's likely to continue to be missed in many places.

It may be worthwhile to extend *codefresh with an additional abstraction that fully consumes the body, including taking the responsibility of closing it. That way it won't continue to be missed. Perhaps the (*codefresh).decodeResponseInto and similar methods could be repurposed? I haven't dug through the code enough to know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant