Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kevinjwalters opened this issue May 23, 2019 · 2 comments · Fixed by #88
Labels
enhancement New feature or request

Comments

@kevinjwalters
Copy link
Contributor

kevinjwalters commented May 23, 2019

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants