Return Close() errors instead of logging

[spenczar]

Issue #, if available:
This is one half of #4 . The other half is to fix servers to never log.

Description of changes:

It's best not to call the standard library's logger default logger, so this changes clients to return errors encountered when calling resp.Body.Close() instead of logging them.

If the client encounters a different error, any error from Close() will be ignored. If Close() returns an error, we'll call it a second time due to the defer, but this should be harmless.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[danp]

How about _ = body.Close(), possibly with the comment from the next line?

[danp]

Or, sorry, maybe it's done this way to get around errcheck -blank? I've used _ = body.Close() or similar before to appease errcheck but maybe -blank is a mode I'm not familiar with.

[spenczar]

Correct, this is done to dodge errcheck -blank, which is "errcheck, and I don't want any funny business like assigning to _".

Which is exactly what we're doing, of course. But I think this is simpler than telling an opinionated user to change their linter setup to not invoke errcheck -blank.

[rhysh]

For clients, this helper doesn't seem to pay its way. It's used in two places in the client code, and the needs of the server code are now different. Instead of modifying the helper, how about phasing it out (first for client code, later for server code)?

I propose changing doProtobufRequest and doJSONRequest to use a named return and to replace their use of defer closebody(resp.Body) with deferring a closure like this:

defer func() {
  cerr := resp.Body.Close()
  if err == nil {
    err = clientError("failed to close response body", cerr)
  }
}()

[spenczar]

@rhysh I'm basically just paranoid of messing up the whole "named return + defer override" trick. I've seen it go awry too many times in too-subtle ways.

Maybe we can do it here though if we do really really really careful code review and tests :) I'll take a crack at it and see how it feels.

[spenczar]

@rhysh Thanks for the prompt, this is better with the defer structure.