Repository: moby/moby
File: client/request.go (Go)

Code before:

	var body []byte
	var err error
	if serverResp.body != nil {
		bodyMax := 1 * 1024 * 1024 // 1 MiB
		bodyR := &io.LimitedReader{
			R: serverResp.body,
			N: int64(bodyMax),
		}
		body, err = io.ReadAll(bodyR)
		if err != nil {
			return err
		}
		if bodyR.N == 0 {
			return fmt.Errorf("request returned %s with a message (> %d bytes) for API route and version %s, check if the server supports the requested API version", http.StatusText(serverResp.statusCode), bodyMax, serverResp.reqURL)
		}
	}
	if len(body) == 0 {
		return fmt.Errorf("request returned %s for API route and version %s, check if the server supports the requested API version", http.StatusText(serverResp.statusCode), serverResp.reqURL)
	}

	var daemonErr error
	if serverResp.header.Get("Content-Type") == "application/json" && (cli.version == "" || versions.GreaterThan(cli.version, "1.23")) {
		var errorResponse types.ErrorResponse
		if err := json.Unmarshal(body, &errorResponse); err != nil {
			return errors.Wrap(err, "Error reading JSON")
		}
		daemonErr = errors.New(strings.TrimSpace(errorResponse.Message))
	} else {
		daemonErr = errors.New(strings.TrimSpace(string(body)))
	}
	return errors.Wrap(daemonErr, "Error response from daemon")
}

func (cli *Client) addHeaders(req *http.Request, headers http.Header) *http.Request {
	// Add CLI Config's HTTP Headers BEFORE we set the Docker headers
	// then the user can't change OUR headers
	for k, v := range cli.customHTTPHeaders {
		if versions.LessThan(cli.version, "1.25") && http.CanonicalHeaderKey(k) == "User-Agent" {
			continue
		}
		req.Header.Set(k, v)
	}

	for k, v := range headers {
		req.Header[http.CanonicalHeaderKey(k)] = v
	}

	if cli.userAgent != nil {
		if *cli.userAgent == "" {
			req.Header.Del("User-Agent")

Review comment (bug):
Probably one more follow-up coming for this case, where we could end up with a proxy returning HTML with an invalid content-type header, which may result in;

```
Error reading JSON: invalid character '<' looking for beginning of value
```

Possibly we could consider;

- by default capture something similar to below (include the failure to parse, but also reply with status-code and "text")
- adding a debug log to log the response, so that a user could enable `--debug` to see the problem
- maybe same for `text/html` responses, which are unexpected, but we may not always want to dump the HTML as-is.


Code after:

	var body []byte
	var err error
	if serverResp.body != nil {
		bodyMax := 1 * 1024 * 1024 // 1 MiB
		bodyR := &io.LimitedReader{
			R: serverResp.body,
			N: int64(bodyMax),
		}
		body, err = io.ReadAll(bodyR)
		if err != nil {
			return err
		}
		if bodyR.N == 0 {
			return fmt.Errorf("request returned %s with a message (> %d bytes) for API route and version %s, check if the server supports the requested API version", http.StatusText(serverResp.statusCode), bodyMax, serverResp.reqURL)
		}
	}
	if len(body) == 0 {
		return fmt.Errorf("request returned %s for API route and version %s, check if the server supports the requested API version", http.StatusText(serverResp.statusCode), serverResp.reqURL)
	}

	var daemonErr error
	if serverResp.header.Get("Content-Type") == "application/json" && (cli.version == "" || versions.GreaterThan(cli.version, "1.23")) {
		var errorResponse types.ErrorResponse
		if err := json.Unmarshal(body, &errorResponse); err != nil {
			return errors.Wrap(err, "Error reading JSON")
		}
		if errorResponse.Message == "" {
			// Error-message is empty, which means that we successfully parsed the
			// JSON-response (no error produced), but it didn't contain an error
			// message. This could either be because the response was empty, or
			// the response was valid JSON, but not with the expected schema
			// ([types.ErrorResponse]).
			//
			// We cannot use "strict" JSON handling (json.NewDecoder with DisallowUnknownFields)
			// due to the API using an open schema (we must anticipate fields
			// being added to [types.ErrorResponse] in the future, and not
			// reject those responses.
			//
			// For these cases, we construct an error with the status-code
			// returned, but we could consider returning (a truncated version
			// of) the actual response as-is.
			//
			// TODO(thaJeztah): consider adding a log.Debug to allow clients to debug the actual response when enabling debug logging.
			daemonErr = fmt.Errorf(`API returned a %d (%s) but provided no error-message`,
				serverResp.statusCode,
				http.StatusText(serverResp.statusCode),
			)
		} else {
			daemonErr = errors.New(strings.TrimSpace(errorResponse.Message))
		}