adafruit / Adafruit_CircuitPython_PyPortal

CircuitPython driver for Adafruit PyPortal.
MIT License
45 stars 56 forks source link

Improve error handling for HTTP response errors and exception handling for JSON parse parsing in fetch() #37

Closed kevinjwalters closed 3 years ago

kevinjwalters commented 5 years ago

fetch makes an HTTP request but does not currently check the HTTP status code. This could be a 5xx error and there's no specific handling for this.

Fifth post on Adafruit Forums: Adafruit CircuitPython and MicroPython: NASA image partially blank on PyPortal has an example of the current error handling too. There is no specific check for an appropriate Content-Type for json, the code optimstically tries to parse the response body and at the moment the exception handling does not cover the KeyError case:

Retrieving data...Reply is OK!
{'code': 500, 'msg': 'Internal Service Error', 'service_version': 'v1'}
Traceback (most recent call last):
File "code.py", line 38, in <module>
File "code.py", line 35, in <module>
File "adafruit_pyportal.py", line 696, in fetch
File "adafruit_pyportal.py", line 693, in fetch
File "adafruit_pyportal.py", line 517, in _json_traverse
KeyError: title

Possible that this line was intended to be only printed for debug too? A useful enhancement would be to print the HTTP status code and if present the response Content-Length:

print("Reply is OK!")

4xx errors are also worth considering particularly because broken servers may 404 requests and buggy servers occasionally respond with a 400, etc. 418 handling could also be considered.

Handling of failures of the adafruit.io services like image converter could also be handled better. I believe that's the cause of KeyError: content-length as discussed in Adafruit Forums: Adafruit CircuitPython and MicroPython: KeyError: content-length.

kevinjwalters commented 4 years ago

Printing Content-Length would help to unravel the mystery of ValueError: syntax error in JSON which is appearing for a user sporadically and the JSON does look truncated on the PyPortal screen, see Adafruit Forums: pyPortal ESP32 issues

kevinjwalters commented 4 years ago

The first post in Adafruit Forums: wget didn't write a complete file looks like a problem getting data from the Adafruit.io service. I think adding priting of Date HTTP response header in addition to status and Content-Length on an error would be useful. The PyPortal has no local clock and getting an accurate time from the server would help analyse any Adafruit.IO issues. Perhaps that also reports some sort of ID value in response headers and that could be logged by the PyPortal framework for an error to aid investigation?

A different issue is whether adafruit.io failures should be (gently) retried over some (short?) time period. I don't know what the failure modes are here.