adafruit / Adafruit_CircuitPython_Requests

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

Add HTTP1.1 support and socket reuse #31

Closed tannewt closed 4 years ago

tannewt commented 4 years ago

Reusing sockets makes sharing memory easier and keeping the connection open will make them faster.

This PR also moves to a model of passing the socket and ssl implementation into Session instead of into the module with a function.

json parsing will now happen character-by-character on CircuitPython and lower the peak memory used and the largest allocation size.

It includes legacy compatibility so it shouldn't break anything. Corresponding libraries will benefit more from this when they implement recv_into so they don't allocate a new buffer every read.

This is draft because I need to:

FoamyGuy commented 4 years ago

Does this need a new version of adafruit_esp32spi? I tried this on a PyPortal with a basic GET request example but I am consistently getting this exception:

Fetching text from http://wifitest.adafruit.com/testwifi/index.html
Traceback (most recent call last):
  File "code.py", line 40, in <module>
  File "/lib/adafruit_requests.py", line 566, in get
  File "/lib/adafruit_requests.py", line 454, in request
  File "/lib/adafruit_requests.py", line 403, in _get_socket
  File "/lib/adafruit_requests.py", line 399, in _get_socket
  File "adafruit_esp32spi/adafruit_esp32spi_socket.py", line 75, in connect
  File "adafruit_esp32spi/adafruit_esp32spi.py", line 750, in socket_connect
  File "adafruit_esp32spi/adafruit_esp32spi.py", line 649, in socket_open
  File "adafruit_esp32spi/adafruit_esp32spi.py", line 323, in _send_command_get_response
  File "adafruit_esp32spi/adafruit_esp32spi.py", line 306, in _wait_response_cmd
  File "adafruit_esp32spi/adafruit_esp32spi.py", line 293, in _wait_response_cmd
  File "adafruit_esp32spi/adafruit_esp32spi.py", line 275, in _check_data
RuntimeError: Expected 01 but got 00
tannewt commented 4 years ago

@FoamyGuy No, it's not fully backwards compatible atm. I'm hoping to get to that today. Specifically, the current iteration doesn't use an IP address for HTTP connections and doesn't set TLS_MODE during .connect. Both are non-standard behavior I missed by testing with CPython's socket.

anecdata commented 4 years ago

With this version of Requests and: Adafruit CircuitPython 6.0.0-alpha.3-142-g683462c1b on 2020-09-10; Saola 1 w/Wrover with ESP32S2 (S3 .bin corresponding to "Merge pull request #3326 from tannewt/native_wifi") test code.py download from Discord yesterday

I consistently get:

  File "code.py", line 34, in <module>
  File "/lib/adafruit_requests.py", line 505, in get
  File "/lib/adafruit_requests.py", line 492, in request
  File "/lib/adafruit_requests.py", line 102, in __init__
RuntimeError: Unable to read HTTP response.

The older version (Discord version) of Requests generally works, some intermittent errors: OSError: Failed SSL handshake in _get_socket,
espidf.MemoryError: in _get_socket, TypeError: unsupported types for __sub__: 'NoneType', 'int' in _readinto (attributed to dev version of chunking code OSError: [Errno 9] EBADF in _readto)

tannewt commented 4 years ago

Ok, @brentru and @anecdata this should be good to go.

anecdata commented 4 years ago

RuntimeError: Unable to read HTTP response. fixed by https://github.com/adafruit/circuitpython/pull/3397 6.0.0-alpha.3-182-g0c7212250 on 2020-09-11 ✅

anecdata commented 4 years ago

Some edge cases. (I'm tracking down why I still get RuntimeError: Unable to read HTTP response. under certain circumstances; the typical text/json websites we usually test with seem fine.)

Here's one: Zero-length response bodies should be valid, but try this:

for _ in range(0, 5):
    response = requests.get("https://httpbin.org/status/200")
    # print(response.status_code)
    # print(response.reason)
    # print(response.headers)
    # print(response.text)

    print()

yields some odd behavior, sometimes gets RuntimeError: Unable to read HTTP response. It waits for about a minute. There is a {'Content-Length': '0'} header.

tannewt commented 4 years ago

@anecdata Can you reproduce that with CPython sockets? Sounds like it may be a timeout thing.

anecdata commented 4 years ago
import requests

for _ in range(0, 5):
    response = requests.get("https://httpbin.org/status/200")
    print(response.status_code)
    print(response.reason)
    print(response.headers)
    print(response.text)

That URL returns right away consistently in browser and python3 on macOS, with header:

{'Date': 'Mon, 14 Sep 2020 20:24:48 GMT', 'Content-Type': 'text/html; charset=utf-8', 'Content-Length': '0', 'Connection': 'keep-alive', 'Server': 'gunicorn/19.9.0', 'Access-Control-Allow-Origin': '*', 'Access-Control-Allow-Credentials': 'true'}

If there's another test procedure I can try, please point me in the right direction.

It doesn't always exception on CicruitPython, but typically at least once in 5 tries. It may be two separate issues, but with a content-length header, client should be able to return right away.

anecdata commented 4 years ago

This may be a clue: if I manually shorten the requests timeout in CircuitPython: requests.get("https://httpbin.org/status/200", timeout=5), it comes back in 5 seconds rather than a minute, and I don't get the "Unable to read HTTP response" exception so far.

anecdata commented 4 years ago

BTW, noticed this the other day and it seems consistent... if a scan isn't done first, wifi.radio.connect() does not return (I did a keyboard interrupt after a few minutes, and the trace indicated the wifi.radio.connect() lines).

oops, I guess that doesn't belong in this PR

tannewt commented 4 years ago

@anecdata Ok, sounds like these are ESP32-S2 wifi related issues, not with this library itself.

anecdata commented 4 years ago

Should I file... 2 issues on circuitpython? • 'Content-Length': '0' • scan before connect

tannewt commented 4 years ago

@anecdata Yes please!