fix a potential json parsing error

[ziliangpeng]

As discussed here, there's a chance (which I have experienced) that the response of the oauth request is not in json format, resulting in json parsing error.

This PR fixes it by capturing the exception and convert to a RefreshError.

[ziliangpeng]

@georgysavva @busunkim96

[georgysavva]

Why not to just move json.loads() after the http_client.OK check. As it was before my change?

[ziliangpeng]

@georgysavva The decoded json needs to be used in the error handling as well as being a return value.

I believe keeping it here is the most appropriate place to decode it. Where else exactly do you see is a better place?

[alvyjudy]

Hi, please correct me if I'm wrong but it appears that this problem (response not in json format) kind of occur by chance? (I get this impression after reading the discussion you mentioned)

So when it arises wouldn't it be a better idea to re-fetch, rather than giving the error back to the client? I would imagine the error message to be quite obscure.

Also, if it is indeed the API that wouldn't return a json response, would it be better to place your proposed functionality inside _handle_error_message? I feel that would make the code look cleaner.

[mik-laj]

Suggested change

	except (KeyError, ValueError):
	except json.DecodeError:

WDYT?

[tseaver]

That would be a Python3-only change, i think.

[tseaver]

Rather than doing remapping of the error twice (once above and once below the check of response.status, it would be better to move the parsing inside the block for an OK status. E.g.:

        if response.status == http_client.OK:
            return json.loads(response_body)

        if response.content_type != "application/json":
            error_details = response_body
            raise exceptions.RefreshError(error_details, response_body)

        response_data = json.decode(response_body)
        error_desc = response_data.get("error_description") or ""
        error_code = response_data.get("error") or ""
        if (
            any(e == "internal_failure" for e in (error_code, error_desc))
            and retry < 1
        ):
            retry += 1
            continue
        _handle_error_response(response_body)

[parthea]

Hi @ziliangpeng , I'm going to close this PR due to inactivity but please feel free to re-open it.