adafruit / Adafruit_CircuitPython_PyPortal

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

get_local_time doesn't properly check the http response status code before attempting to parse the time #57

Closed theacodes closed 4 years ago

theacodes commented 4 years ago

https://github.com/adafruit/Adafruit_CircuitPython_PyPortal/blob/81c82035649007bbccb7d0ec75ce192f3b2ca058/adafruit_pyportal.py#L618

If the request is an error, such as 404 for User not found (example url), then this code will attempt to parse the response into a datetime anyway.

It should makes sure the status code is 200 before attempting to parse. If it's not, it should throw an exception and include the response text in the exception to help with debugging. As it stands, this will raise a very unhelpful error:

Traceback (most recent call last):
  File "code.py", line 44, in <module>
  File "adafruit_pyportal.py", line 629, in get_local_time
  File "adafruit_pyportal.py", line 625, in get_local_time
ValueError: invalid syntax for integer with base 10 

This would be an excellent first-time issue for someone.

ladyada commented 4 years ago

good idea - thanks for spotting this one @theacodes @brentru might also be a good one for you :)

brentru commented 4 years ago

@ladyada I'll leave it for a new contributor within the next 15 days as people's holiday purchases roll in. I'll patch/release by then if it doesn't get picked up.

brentru commented 4 years ago

Addressed in https://github.com/adafruit/Adafruit_CircuitPython_PyPortal/pull/63.

Merged into master and released in v3.1.8.