client: improve handling of JSON error-responses with incorrect schema
Before this patch, an API response that's valid JSON, but not the right
schema would be silently discarded by the CLI. For example, due to a bug
in Docker Desktop's API proxy, the "normal" (not JSON error) response
would be returned together with a non-200 status code when using an
unsupported API version;
curl -s -w 'STATUS: %{http_code}\n' --unix-socket /var/run/docker.sock 'http://localhost/v1.99/version'
{"Platform":{"Name":"Docker Desktop 4.38.0 (181016)"},"Version":"","ApiVersion":"","GitCommit":"","GoVersion":"","Os":"","Arch":""}
STATUS: 400
Before this patch, this resulted in no output being shown;
DOCKER_API_VERSION=1.99 docker version
Client:
Version: 27.5.1
API version: 1.99 (downgraded from 1.47)
Go version: go1.22.11
Git commit: 9f9e405
Built: Wed Jan 22 13:37:19 2025
OS/Arch: darwin/arm64
Context: desktop-linux
Error response from daemon:
With this patch, an error is generated based on the status:
DOCKER_API_VERSION=1.99 docker version
Client:
Version: 27.5.1
API version: 1.99 (downgraded from 1.47)
Go version: go1.22.11
Git commit: 9f9e405
Built: Wed Jan 22 13:37:19 2025
OS/Arch: darwin/arm64
Context: desktop-linux
Error response from daemon: API returned a 400 (Bad Request) but provided no error-message
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn authored on 2025/01/29 19:46:38Showing 2 changed files
| ... | ... |
@@ -234,8 +234,35 @@ func (cli *Client) checkResponseErr(serverResp serverResponse) error {
|
| 234 | 234 |
if err := json.Unmarshal(body, &errorResponse); err != nil {
|
| 235 | 235 |
return errors.Wrap(err, "Error reading JSON") |
| 236 | 236 |
} |
| 237 |
- daemonErr = errors.New(strings.TrimSpace(errorResponse.Message)) |
|
| 237 |
+ if errorResponse.Message == "" {
|
|
| 238 |
+ // Error-message is empty, which means that we successfully parsed the |
|
| 239 |
+ // JSON-response (no error produced), but it didn't contain an error |
|
| 240 |
+ // message. This could either be because the response was empty, or |
|
| 241 |
+ // the response was valid JSON, but not with the expected schema |
|
| 242 |
+ // ([types.ErrorResponse]). |
|
| 243 |
+ // |
|
| 244 |
+ // We cannot use "strict" JSON handling (json.NewDecoder with DisallowUnknownFields) |
|
| 245 |
+ // due to the API using an open schema (we must anticipate fields |
|
| 246 |
+ // being added to [types.ErrorResponse] in the future, and not |
|
| 247 |
+ // reject those responses. |
|
| 248 |
+ // |
|
| 249 |
+ // For these cases, we construct an error with the status-code |
|
| 250 |
+ // returned, but we could consider returning (a truncated version |
|
| 251 |
+ // of) the actual response as-is. |
|
| 252 |
+ // |
|
| 253 |
+ // TODO(thaJeztah): consider adding a log.Debug to allow clients to debug the actual response when enabling debug logging. |
|
| 254 |
+ daemonErr = fmt.Errorf(`API returned a %d (%s) but provided no error-message`, |
|
| 255 |
+ serverResp.statusCode, |
|
| 256 |
+ http.StatusText(serverResp.statusCode), |
|
| 257 |
+ ) |
|
| 258 |
+ } else {
|
|
| 259 |
+ daemonErr = errors.New(strings.TrimSpace(errorResponse.Message)) |
|
| 260 |
+ } |
|
| 238 | 261 |
} else {
|
| 262 |
+ // Fall back to returning the response as-is for API versions < 1.24 |
|
| 263 |
+ // that didn't support JSON error responses, and for situations |
|
| 264 |
+ // where a plain text error is returned. This branch may also catch |
|
| 265 |
+ // situations where a proxy is involved, returning a HTML response. |
|
| 239 | 266 |
daemonErr = errors.New(strings.TrimSpace(string(body))) |
| 240 | 267 |
} |
| 241 | 268 |
return errors.Wrap(daemonErr, "Error response from daemon") |
| ... | ... |
@@ -154,11 +154,10 @@ func TestResponseErrors(t *testing.T) {
|
| 154 | 154 |
}, |
| 155 | 155 |
{
|
| 156 | 156 |
// Server response that's valid JSON, but not the expected (types.ErrorResponse) scheme |
| 157 |
- // TODO(thaJeztah): consider returning (partial) raw response for these |
|
| 158 | 157 |
doc: "incorrect JSON scheme", |
| 159 | 158 |
contentType: "application/json", |
| 160 | 159 |
response: `{"error":"Some error occurred"}`,
|
| 161 |
- expected: `Error response from daemon: `, |
|
| 160 |
+ expected: `Error response from daemon: API returned a 400 (Bad Request) but provided no error-message`, |
|
| 162 | 161 |
}, |
| 163 | 162 |
{
|
| 164 | 163 |
// TODO(thaJeztah): improve handling of such errors; we can return the generic "502 Bad Gateway" instead |