Repository: google/go-containerregistry
Pull request #739 (2020-07-07 20:09:18)

transport: preserve unexpected json errors
Fixes https://github.com/google/go-containerregistry/issues/737

codecov-commenter: # [Codecov](https://codecov.io/gh/google/go-containerregistry/pull/739?src=pr&el=h1) Report
> Merging [#739](https://codecov.io/gh/google/go-containerregistry/pull/739?src=pr&el=desc) into [master](https://codecov.io/gh/google/go-containerregistry/commit/72edbad9993e722466d28ff308cf2cdba4b675c5&el=desc) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/google/go-containerregistry/pull/739/graphs/tree.svg?width=650&height=150&src=pr&token=wG7iiDCdZ4)](https://codecov.io/gh/google/go-containerregistry/pull/739?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master     #739   +/-   ##
=======================================
  Coverage   79.39%   79.39%           
=======================================
  Files         102      102           
  Lines        4683     4683           
=======================================
  Hits         3718     3718           
  Misses        534      534           
  Partials      431      431           
```


| [Impacted Files](https://codecov.io/gh/google/go-containerregistry/pull/739?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/v1/remote/transport/error.go](https://codecov.io/gh/google/go-containerregistry/pull/739/diff?src=pr&el=tree#diff-cGtnL3YxL3JlbW90ZS90cmFuc3BvcnQvZXJyb3IuZ28=) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/go-containerregistry/pull/739?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/google/go-containerregistry/pull/739?src=pr&el=footer). Last update [72edbad...8037955](https://codecov.io/gh/google/go-containerregistry/pull/739?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pkg/v1/remote/transport/error.go b/pkg/v1/remote/transport/error.go
index a215e794..0f0e92af 100644
--- a/pkg/v1/remote/transport/error.go
+++ b/pkg/v1/remote/transport/error.go
@@ -166,10 +166,14 @@ func CheckError(resp *http.Response, codes ...int) error {
 
 	// https://github.com/docker/distribution/blob/master/docs/spec/api.md#errors
 	structuredError := &Error{}
-	if err := json.Unmarshal(b, structuredError); err != nil {
-		structuredError.rawBody = string(b)
-	}
+
+	// This can fail if e.g. the response body is not valid JSON. That's fine,
+	// we'll construct an appropriate error string from the body and status code.
+	_ = json.Unmarshal(b, structuredError)
+
+	structuredError.rawBody = string(b)
 	structuredError.StatusCode = resp.StatusCode
 	structuredError.request = resp.Request
+
 	return structuredError
 }


Test patch:
diff --git a/pkg/v1/remote/transport/error_test.go b/pkg/v1/remote/transport/error_test.go
index 85bd1279..ededc509 100644
--- a/pkg/v1/remote/transport/error_test.go
+++ b/pkg/v1/remote/transport/error_test.go
@@ -89,6 +89,11 @@ func TestCheckErrorNotError(t *testing.T) {
 		code: http.StatusBadRequest,
 		body: "",
 		msg:  "unsupported status code 400",
+	}, {
+		code: http.StatusUnauthorized,
+		// Valid JSON, but not a structured error -- we should still print the body.
+		body: `{"details":"incorrect username or password"}`,
+		msg:  `unsupported status code 401; body: {"details":"incorrect username or password"}`,
 	}, {
 		code: http.StatusUnauthorized,
 		body: "Not JSON",