drive: FilesGetCall.Download did not unmarshal the body in the error response

[aofei]

Just now I happened to notice that the FilesGetCall.Download threw me this error:

googleapi: got HTTP response code 403 with body: {"error":{"errors":[{"domain":"usageLimits","reason":"userRateLimitExceeded","message":"User Rate Limit Exceeded. Rate of requests for user exceed configured project quota. You may consider re-evaluating expected per-user traffic to the API and adjust project quota limits accordingly. You may monitor aggregate quota usage and adjust limits in the API Console: https://console.developers.google.com/apis/api/drive.googleapis.com/quotas?project=xxxxxxxxxxxxxx","extendedHelp":"https://console.developers.google.com/apis/api/drive.googleapis.com/quotas?project=xxxxxxxxxxxxxx"}],"code":403,"message":"User Rate Limit Exceeded. Rate of requests for user exceed configured project quota. You may consider re-evaluating expected per-user traffic to the API and adjust project quota limits accordingly. You may monitor aggregate quota usage and adjust limits in the API Console: https://console.developers.google.com/apis/api/drive.googleapis.com/quotas?project=xxxxxxxxxxxxxx"}}

The returned error is still *googleapi.Error, which is fine. But in its fields, only Code and Body have values. This caused me to be unable to handle errors through the Errors field as usual.

So, does this work as designed?

[tbpg]

Thanks for the issue. I'm not sure why that's the case for this call. Here is what appears to be the relevant function:

google-api-go-client/googleapi/googleapi.go

Lines 165 to 178 in 84df58a

	// CheckMediaResponse returns an error (of type *Error) if the response
	// status code is not 2xx. Unlike CheckResponse it does not assume the
	// body is a JSON error document.
	// It is the caller's responsibility to close res.Body.
	func CheckMediaResponse(res *http.Response) error {
	if res.StatusCode >= 200 && res.StatusCode <= 299 {
	return nil
	}
	slurp, _ := ioutil.ReadAll(io.LimitReader(res.Body, 1<<20))
	return &Error{
	Code: res.StatusCode,
	Body: string(slurp),
	}
	}

Note that it doesn't try to parse the body as JSON, per the function comment. It seems that all "media" requests use CheckMediaResponse, which makes sense based on naming! See https://github.com/googleapis/google-api-go-client/search?q=checkmediaresponse.

Error parsing came up for #473. So, let's wait for @codyoss. Notably, https://google.aip.dev/193 doesn't call out media requests as different.

CheckMediaResponse was added in 0a735f7 6 years ago, so it's possible that it's now safe to assume JSON.

[codyoss]

Thanks for the issue @aofei. @tbpg I assume this is a special case for storage, at least I know that api does not conform to the aip and has xml endpoints... We could keep using this method for storage and move all other apis to use CheckResponse. That way we can give the opportunity to parse out extra info for apis that conform but not slow down storage errors that we know will not parse into JSON.

[tbpg]

We could keep using this method for storage and move all other apis to use CheckResponse.

Sounds good to me.