search

burnash / gspread

Google Sheets Python API
https://docs.gspread.org
MIT License
7.12k stars 948 forks source link

better handler API error parsing. #1510

Closed lavigne958 closed 2 months ago

lavigne958 commented 2 months ago

Possibly we encountered an error while parsing the API error JSON, like connection closes early etc and the received JSON is invalid.

Better handle such scenario and build a valid error Dict to keep the exception stack flow running and raise this message to the application.

extract the returned text so if we any bit of valid information at least we can pass that to the application.

This could be helpfull to if we add calls to the API made for web browser that returns HTML error message, it will fail to decode the recieved error and raise a proper error.

closes #1504

lavigne958 commented 2 months ago

I could possibly manually test it, but that's quiet hard to do. otherwise, using automatic test no :disappointed: that would require us to close the connection perfectly in the middle of the transfer of the response from the server, way too hard to be automatic :laughing:

we do not regress in anything so far.

alifeee commented 2 months ago

we could make a test that uses a HTTP client and mock the response?

lavigne958 commented 2 months ago

we could make a test that uses a HTTP client and mock the response?

Possibly yes 🤔 We could use a custom http client for a single request that returns an invalid JSON.

I'm worried how this is gonna play with the cassette recorder too 🤔 it's worth the try.

dhuang commented 2 months ago

appreciate the quick fix here, we started seeing these as well. any chance we'll get a release with this change?

lavigne958 commented 1 month ago

It's done :heavy_check_mark: in #1512 I managed to create a fake HTTP client that can be tested and raises the appropriate exception (the regular APIError exception).

lavigne958 commented 1 month ago

appreciate the quick fix here, we started seeing these as well. any chance we'll get a release with this change?

so far we can't tell, we have quite a few new commits but nothing minor/major except this one. we need to discuss what is waiting to be reviewed or merged and check our priorities.

natebessa commented 1 month ago

Helpful change, thanks for fixing. We need this fix because suddenly we've been getting a lot of 502 errors from the Google Sheets API. The response on the 502 does not contain a json object but instead the HTML/CSS of an error page, which triggers a JSONDecodeError. We implemented gspreads BackOffHTTPClient to retry on those 502s but the JSONDecodeError breaks that.

Will be on the look out for a release. Will be massively helpful. Thanks.

alifeee commented 1 month ago

I have turned the release into an issue... we will do it soon :)

https://github.com/burnash/gspread/issues/1516

Kirkman commented 1 month ago

Helpful change, thanks for fixing. We need this fix because suddenly we've been getting a lot of 502 errors from the Google Sheets API. The response on the 502 does not contain a json object but instead the HTML/CSS of an error page, which triggers a JSONDecodeError. We implemented gspreads BackOffHTTPClient to retry on those 502s but the JSONDecodeError breaks that.

Glad to know I'm not the only one seeing this the last few weeks.

natebessa commented 1 month ago

@Kirkman absolutely. It's almost 1 in 10 calls for me at this rate. Not something I'd expect from Google, even if it's a free service. I can't find much chatter about it online, and no acknowledgement from Google on any social media. Meanwhile my Google Cloud dashboard is massively undercounting the API error rate I'm seeing.

But I've since forked the repo, added the status_code fix I mention above, implemented the BackOffHttpClient, and have been swimming problem-free since.

eitchtee commented 1 month ago

The intermittent 502 errors seems to be a bug on Google's end, related issue: https://issuetracker.google.com/issues/369670473

alifeee commented 1 month ago

hi all

a new release 6.1.3 has been released (on github) (on PyPi) which contains a fix for this issue

we now catch the non-JSON error and turn it into a GSpreadException which should be easier to catch and find out what went wrong with the request

see more context here: https://github.com/burnash/gspread/pull/1510