adafruit / Adafruit_CircuitPython_Requests

Requests-like interface for web interfacing
MIT License
51 stars 37 forks source link

Add Simpletest for Ethernet Library #22

Closed brentru closed 4 years ago

brentru commented 4 years ago

Adding a simpletest for the ethernet library. This will be used in a future Learning System Guide.

Requires: https://github.com/adafruit/Adafruit_CircuitPython_Wiznet5k/pull/7

Tested on: Adafruit CircuitPython 5.0.0 on 2020-03-02; Adafruit Metro M4 Express with samd51j19

Test output:

code.py output:
Fetching text from http://wifitest.adafruit.com/testwifi/index.html
----------------------------------------
Text Response:  This is a test of Adafruit WiFi!
If you can read this, its working :)

----------------------------------------
Fetching JSON data from http://httpbin.org/get
----------------------------------------
JSON Response:  {'url': 'http://httpbin.org/get', 'headers': {'User-Agent': 'Adafruit CircuitPython', 'Host': 'httpbin.org', 'X-Amzn-Trace-Id': 'Root=1-5e627b53-b98194cf883dda881877943c'}, 'args': {}, 'origin': '73.38.253.75'}
----------------------------------------
POSTing data to http://httpbin.org/post: 31F
----------------------------------------
Data received from server: 31F
----------------------------------------
POSTing data to http://httpbin.org/post: {'Date': 'July 25, 2019'}
----------------------------------------
JSON Data received from server: {'Date': 'July 25, 2019'}
----------------------------------------
FoamyGuy commented 4 years ago

I did eventually get successful tests from both of these on PyGamer with Ethernet Featherwing. If it's decided that no retry logic or comment is needed, then these look good to me.

brentru commented 4 years ago

@FoamyGuy I'll add a retry loop for network handling to these two examples prior to merging. WiFiManager performs this for ESP32SPI.

brentru commented 4 years ago

@FoamyGuy Ok, was unable to fully replicate your AssertionError so I manually specified an incorrect DNS server address and was able to test from there.

The following commit adds retry logic with three attempts (user can increase from code) to each http request.. I'll merge if it looks good to you.

Example output

Fetching text from http://wifitest.adafruit.com/testwifi/index.html
Request failed, retrying...
 Failed to resolve hostname!
Request failed, retrying...
 Failed to resolve hostname!
Request failed, retrying...
 Failed to resolve hostname!
Traceback (most recent call last):
  File "code.py", line 46, in <module>
  File "code.py", line 45, in <module>
AssertionError: Failed to resolve hostname, please check your router's DNS configuration.
FoamyGuy commented 4 years ago

There may be a logical error within the retry attempting actually. I got this output attempting the simpletest:

code.py output:
* Received DHCP Message is not OFFER
Fetching text from http://wifitest.adafruit.com/testwifi/index.html
----------------------------------------
Text Response:  This is a test of Adafruit WiFi!
If you can read this, its working :)

----------------------------------------
Fetching JSON data from http://httpbin.org/get
----------------------------------------
JSON Response:  {'url': 'http://httpbin.org/get', 'headers': {'User-Agent': 'Adafruit CircuitPython', 'Host': 'httpbin.org', 'X-Amzn-Trace-Id': 'Root=1-5e68123e-3f3097d7ca47f0958e51b602'}, 'args': {}, 'origin': '136.34.156.146'}
----------------------------------------
POSTing data to http://httpbin.org/post: 31F
----------------------------------------
Data received from server: 31F
----------------------------------------
POSTing data to http://httpbin.org/post: {'Date': 'July 25, 2019'}
Request failed, retrying...
 Failed to resolve hostname!
Traceback (most recent call last):
  File "code.py", line 97, in <module>
  File "code.py", line 95, in <module>
AssertionError: Failed to resolve hostname,                                   please check your router's DNS configuration.

I think the issue is that attempts is getting set to 0 at the end of the try blocks. So after one of requests succeeds attempts get set down to 0 and then the next (first) failed request makes failure_count go higher than the 0 that is in attempts which results in re-raising the exception instead of trying again.

If I'm understanding the code correctly I think it should be failure_count getting reset back to 0 at the end of the try blocks rather than attempts

FoamyGuy commented 4 years ago

The advanced example does contain the same attempts = 0 statement at the end of the try block but it's not currently susceptible to the same problem because there is only a single request being made. Here is output from the updated advanced example showing it successfully retrying:

code.py output:
* Received DHCP Message is not OFFER
Fetching JSON data from http://httpbin.org/get...
Request failed, retrying...
 Failed to resolve hostname!
------------------------------------------------------------
Response's Custom User-Agent Header: Adafruit CircuitPython,blinka/1.0.0
------------------------------------------------------------
Response HTTP Status Code:  200
------------------------------------------------------------
Raw Response:  b'{\n  "args": {}, \n  "headers": {\n    "Host": "httpbin.org", \n    "User-Agent": "Adafruit CircuitPython,blinka/1.0.0", \n    "X-Amzn-Trace-Id": "Root=1-5e681557-d500f46a7997a575145767a9"\n  }, \n  "origin": "136.34.156.146", \n  "url": "http://httpbin.org/get"\n}\n'

If my understanding is correct we should probably fix this in the advanced example as well just in case in the future more requests are added to it.

brentru commented 4 years ago

@FoamyGuy b531942 addresses the attempts issue on both the advanced and simpletest examples.