Closed grypoB closed 1 year ago
With no content-length
or chunked encoding, a timeout would be useful to handle cases where all of the data is not yet in the buffer. Not sure if that happens in the existing structure.
@anecdata, I was wondering about that too. Looks like the timeout is already handled by the socket itself and is set at the socket creation: sock.settimeout(timeout) # socket read timeout
It is set by default to 60s, when not given as argument of the request.
On my laptop, reaching the timeout will raise a simple socket.timeout
error.
On the Pico W with the socketpool.SocketPool(wifi.radio)
, different errors could be raised (I got an OutOfRetries
error, as well as one in the socket creation (maybe the wifi.radio
not liking creating multiple sockets in a row?).
I only tried with a server which has a long initial response time. I'll try to setup a server to see how the Pico W behaves if the server hangs midstream.
@anecdata I had some fun with building an http.server which spits out its response very slowly (each line of the json response takes longer and longer to be sent): https://gist.github.com/grypoB/0c0cda901c2c2d437924ee36f7641ea9
The results are as follows (I only tried with not chunked responses) if you set a small timeout value in the GET
:
socket.timeout
error if the server took longer than the timeout value in between two lines of the responseOSError: [Errno 116] ETIMEDOUT
in the _recv_into
functionAnd if the servers just keep sending data, on the Pico W it raises a MemoryError
somewhere here
All this is very expected, so everything works just fine with regards to timeouts.
That sounds good, thank you for testing it out.
@zvxr Are you able to test this still?
@anecdata yes, I can test this change for the tonight using HUE workflow (https://github.com/adafruit/Adafruit_CircuitPython_Requests/issues/38)
@anecdata @grypoB
This might be a cross-dependency issue (running older versions of:) adafruit_bus_device adafruit_esp32spi
but initial attempt in using that branch:
Traceback (most recent call last):
File "/lib/hue.py", line 123, in set_light_state
File "/lib/adafruit_hue.py", line 168, in set_light
File "/lib/adafruit_hue.py", line 259, in _put
File "/lib/adafruit_esp32spi/adafruit_esp32spi_wifimanager.py", line 262, in put
File "/lib/adafruit_requests.py", line 826, in put
File "/lib/adafruit_requests.py", line 688, in request
File "/lib/adafruit_requests.py", line 191, in __init__
File "/lib/adafruit_requests.py", line 228, in _readto
AttributeError: 'bytearray' object has no attribute 'find'
which eventually leads to
Traceback (most recent call last):
File "/lib/hue.py", line 123, in set_light_state
File "/lib/adafruit_hue.py", line 168, in set_light
File "/lib/adafruit_hue.py", line 259, in _put
File "/lib/adafruit_esp32spi/adafruit_esp32spi_wifimanager.py", line 262, in put
File "/lib/adafruit_requests.py", line 826, in put
File "/lib/adafruit_requests.py", line 662, in request
File "/lib/adafruit_requests.py", line 540, in _get_socket
RuntimeError: Repeated socket failures
@anecdata @grypoB
I upgraded project dependencies
Adafruit_CircuitPython_BusDriver 5.2.3 Adafruit_CircuitPython_ESP32SPI 5.0.5 Adafruit_CircuitPython_Hue 1.2.3
Unfortunately using branch I am still getting the AttributeError: 'bytearray' object has no attribute 'find'
mentioned above.
@zvxr What version of CircuitPython is loaded (and what board is this)? I think bytearray
has had find
since 2020.
@anecdata @zvxr from the CircuitPython changelog, the bytearray.find() was added in the 6.0.0 release:
Add
.find
,.rfind
,.index
and.rindex
to bytearray for CPython-compatible builds
@anecdata @grypoB
I upgraded to Adafruit CircuitPython 7.3.3 -- my device is an Adafruit Metro M4 Express with samd51j19
Unfortunately it seems to hang on any request through hue. This is the traceback when I interrupt:
File "/lib/adafruit_hue.py", line 178, in get_light
File "/lib/adafruit_hue.py", line 256, in _get
File "/lib/adafruit_requests.py", line 426, in json
File "/lib/adafruit_requests.py", line 162, in readinto
File "/lib/adafruit_requests.py", line 305, in _readinto
File "/lib/adafruit_requests.py", line 222, in _recv_into
File "/lib/adafruit_esp32spi/adafruit_esp32spi_socket.py", line 179, in recv_into
File "/lib/adafruit_esp32spi/adafruit_esp32spi_socket.py", line 210, in available
File "/lib/adafruit_esp32spi/adafruit_esp32spi.py", line 776, in socket_available
File "/lib/adafruit_esp32spi/adafruit_esp32spi.py", line 332, in _send_command_get_response
File "/lib/adafruit_esp32spi/adafruit_esp32spi.py", line 313, in _wait_response_cmd
File "/lib/adafruit_esp32spi/adafruit_esp32spi.py", line 263, in _read_bytes
Verified with new version of circuitpython (and other deps I upgraded) that requests version 1.6 still works (it's just anything after)
@zvxr Thank you for the traceback. The default timeout is 60s. Can you try with a shorter timeout? To see if the GET
eventually returns with all the data. My guess it that the socket implementation of adafruit_esp32spi_socket.py
doesn't detect that the server closed the connection and it is waiting for the timeout to return.
I had a look at the adafruit_esp32spi_socket.py
, and why it used to work with adafruit_requests.py
version 1.6.0.
With version 1.6, it called socket.recv()
, and since then it calls socket.recv_into()
.
In adafruit_esp32spi_socket.py
those functions behaves slightly differently:
.recv
, when passed 0 bytes to read (like it did in version 1.6.0 when there was no Content-Length
), reads until there are no bytes available in the socket, without checking the timeout. See the code source here..recv_into
, since version 1.7.0 was never called (no Content-Length
so it wasn't trying to read any data), but with my PR, it is: I try to read a full chunk, but the .recv_into
implementation of the esp32spi_socket
will only return on timeout, if the buffer is full or it received at least one byte (source here).
@anecdata Hopefully reducing the timeout mitigates the issues for @zvxr, but properly fixing this would require that adafruit_esp32spi_socket.py
detects when the server closed the connection.
@grypoB I would really like to fix ESP32SPI socket and socketpool.socket have the same behavior as CPython socket.socket. The .recv(0)
behavior of reading until nothing is left is non-standard, right? I tried to start doing this in https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/pull/167 but was stymied because it broke some existing things. A sweeping change like this requires changing many things at once, but I think it is worth it.
Here is a draft to read full responses that do not provide a Content-Length in the header and are not chunked.
I was porting some working code from MicroPython which used urequests to CircuitPython on a Pico W, and I was sad to see it break because of issue #38. After some time sifting through the code of both libraries, it seems that the urequest library simply does a
usocket.socket().read()
at line 20, whereas this library only reads a specific number of bytes from the response.So here is my hack that fixes my specific use case while not breaking the unit tests.
If you think it is a good way of fixing issue #38, I'd be happy to add some unit test for it.