Add support for response without Content-Length

[grypoB]

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.

[anecdata]

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.

[grypoB]

@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.

[grypoB]

@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:

• on my laptop, it raises a socket.timeout error if the server took longer than the timeout value in between two lines of the response
• on the Pico W, it raises an OSError: [Errno 116] ETIMEDOUT in the _recv_into function

And 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.

[anecdata]

That sounds good, thank you for testing it out.

[anecdata]

@zvxr Are you able to test this still?

[zvxr]

@anecdata yes, I can test this change for the tonight using HUE workflow (#38)

[zvxr]

@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

[zvxr]

@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.

[anecdata]

@zvxr What version of CircuitPython is loaded (and what board is this)? I think bytearray has had find since 2020.

[grypoB]

@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

[zvxr]

@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)

[grypoB]

@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.
• whereas .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).
  • so if the response length is aligned with the chunk length, then reading another chunk at the end will only return on a timeout
  • if the response isn't aligned, in this PR, we still try to read another chunk after that. We could check that the last chunk read was less than the full size of the buffer, and assume the server stopped answering (but for server which send data slowly, this would trigger a return to early).
  • if the socket detected the server closed the connection, all this would be handled properly though, and we wouldn't need to wait the timeout.

@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.

[dhalbert]

@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 adafruit/Adafruit_CircuitPython_ESP32SPI#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.

[anecdata]

This PR reported to help in the Hue case where there is no content-length or chunked encoding.

[FoamyGuy]

This looks good to me. Thanks for the fix @grypoB.

I don't have the Hue hardware specifically to test, but I did confirm on a Feather ESP32-S2 TFT that the existing example scripts still work as expected. I also set up a local test server with this very basic implementation I found a gist for: https://gist.github.com/shieldwed/0312c39fa486912060c68bfd314f2393 and modified it to omit the content-length header from the response and verified that this new version of the library handles those responses better than the existing released version.