Original response body cannot be re-read

[lukebjerring]

After executing client.Do, the response's body cannot be read by the caller in the case that some content has been dropped.

Being able to read the response again would allow for users to pick up dropped fields, in cases such as #1046

[gmlewis]

I don't think I understand the issue.
Can you please give a detailed example of what problem you are trying to solve?
What do you mean by in the case that some content has been dropped?
How did the content get dropped?

[lukebjerring]

So, hypothetically, there might be a field in the JSON body that is not defined in the Golang struct. That field will be ignored during json.Unmarshal (i.e. "dropped" during the conversion).

#1046 is an example, where I could have recovered the previous_filename field that I needed without having to handle all of the code already provided by go-github (building a client, providing the correct URL string for the API call, etc). However, I can't re-read the request.Body, so had to do the full request myself.

Does that make sense?

[gmlewis]

Ah, I get it. Thank you for the clarification, @lukebjerring!

I would consider that to be a bug of this repo that needs to be fixed.

I would rather not touch the Do method itself for things like this.
Whenever I need to debug problems like this, I just quickly add a httputil.DumpResponse in the Do method and then go update the library to support the new field.

I'll leave this issue open for a week or two for discussion from other repo users, but I recommend that we not add debugging code like this in such a heavily-used method.

[lukebjerring]

Sure; sounds reasonable to me.

[gauntface]

Is there a way we can make the raw response body available as a string?

[gmlewis]

This reminds me of #878. I would love to hear @dsymonds thoughts about the practicality of making the raw response body available as a string.

I suppose a string field could be added to the Response struct, but I'm a bit concerned about the space and time overhead of doing so. Granted, this is not a high throughput repo (based solely on the GitHub rate limiting), but I would hate to see the memory usage skyrocket for clients that deal regularly with multi-MB response payloads.

[dsymonds]

I'm a bit confused. If the problem is that there's some new field in the API response that isn't defined in the Go struct, why not just add that to the Go struct? That's what appeared to have happened in #1046; why is that a bad solution? Like @gmlewis said, that's simply a bug (or, more charitably, a missing feature) of this repo, and it's easy to fix when it arises.

Making the raw response body available is obviously possible, but without a compelling reason it just bloats the memory for every user of this API client.

[lukebjerring]

Sounds like we all agree it'd be overkill! Closing, since it's WAI and patches to the structs are better.