adafruit / Adafruit_CircuitPython_Requests

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

request.get isn't working in 1.7.x #41

Closed makermelissa closed 3 years ago

makermelissa commented 3 years ago

I'm not sure of the exact issue, but in the PyPortal library, it works fine in 1.6.0, but in 1.7.x it fails. The line in the PyPortal library that it seems to fail on is: https://github.com/adafruit/Adafruit_CircuitPython_PyPortal/blob/master/adafruit_pyportal.py#L731 which is r = requests.get(url, stream=True).

The URL is an image https://io.adafruit.com/api/v2/MakerMelissa/integrations/image-formatter?x-aio-key=[redacted]&width=320&height=240&output=BMP16&url=https://apod.nasa.gov/apod/image/2010/VLA_Moonset_NIK_0991_1024.jpg.

More possibly related errors. I'm not sure where they're all generated:

Some error occurred, retrying! - wget didn't write a complete file (part of PyPortal)
Free mem:  130112
Retrieving data...Some error occurred, retrying! - Failed to send 3 bytes (sent 0)
Free mem:  130160
Retrieving data...Some error occurred, retrying! - ESP32 not responding
Free mem:  130160

Honestly not sure what's going on here, but I thought I'd post the issue.

anecdata commented 3 years ago

Can you print the server headers? It's probably related to a known issue.

makermelissa commented 3 years ago

No, at least not in the PyPortal library. It fails before the response is returned.

makermelissa commented 3 years ago

I'm going to try within parse_headers.

makermelissa commented 3 years ago

It doesn't appear there are any headers for the image for some reason.

makermelissa commented 3 years ago

Sorry, I thought you meant response headers. I don't have access to the server headers.

makermelissa commented 3 years ago

Oh, I realized I can get these with the 1.6.0 version. Here you go:

{'access-control-max-age': '1728000', 'etag': 'W/"5830bfd3b724f974bfcb1925d81bd1a5"', 'access-control-expose-headers': 'X-Pagination-Limit, X-Pagination-Start, X-Pagination-End, X-Pagination-Count, X-Pagination-Total', 'referrer-policy': 'strict-origin-when-cross-origin', 'content-transfer-encoding': 'binary', 'x-aio-proxy': '1', 'content-length': '153738', 'strict-transport-security': 'max-age=31536000', 'content-type': 'image/bmp', 'x-download-options': 'noopen', 'access-control-request-method': '*', 'connection': 'close', 'access-control-allow-methods': 'POST, PUT, DELETE, GET, OPTIONS, PATCH', 'access-control-allow-headers': 'DNT, Origin, X-Requested-With, X-AIO-Key, Content-Type, Accept, Authorization', 'content-disposition': 'attachment', 'cache-control': 'private', 'date': 'Fri, 09 Oct 2020 19:59:57 GMT', 'server': 'nginx', 'x-content-type-options': 'nosniff', 'x-xss-protection': '1; mode=block', 'access-control-allow-origin': '*', 'x-frame-options': 'SAMEORIGIN', 'x-request-id': '6d466f47-24ec-4a38-aece-5ae1c015fddb', 'x-aio-worker': 'io-rails-2', 'x-runtime': '0.476318', 'x-permitted-cross-domain-policies': 'none'}
anecdata commented 3 years ago

haha, I was just about to suggest that. Are there trace lines pointing to where in Requests it's failing?

anecdata commented 3 years ago

There is a content-length header, possibly a timeout is prematurely closing the socket. May help to just use requests to try to get the response, to see what's happening. Is that image accessible to other AIO users?

makermelissa commented 3 years ago

I'm still a perplexed about exactly where it's failing and have been trying to trace this down. It seems like maybe like it's a parsing issue.

makermelissa commented 3 years ago

Probably it's just using the image converter, but I didn't want to post my AIO key for everyone to see.

makermelissa commented 3 years ago

Ok, I'm suspecting this may be related to the fact that there are 2 calls going on. It does a call to get JSON and that's successful. Then it does a call to download the image and perhaps something isn't resetting that should be between the calls.

tannewt commented 3 years ago

@makermelissa 1.7.0 tries to reuse sockets to servers. I see that there is a connection: close header that the first request may have also had that we ignored. I suspect we need to actually close the socket when we see that in the response.

(I leave sockets open because TCP has a start up cost I was hoping to avoid.)

makermelissa commented 3 years ago

Ok cool. I had to move off of this for the time being. Hopefully it's something simple.

gmeader commented 3 years ago

Multiple Adafruit Learn Guides examples do not work accessing the Internet on 6.0.0-rc.0 on my PyPortal Titano with the latest libraries (6.x-mpy-20201019) ESP firmware: bytearray(b'1.6.1\x00') (So maybe it's not only 1.7.x?) Note: Libraries examples: pyportal_simpletest.py and esp32spi_simpletest.py work fine.

These do not work:

PyPortal_CMA_ART_FRAME PyPortal_NASA PyPortal_NewNewNew PyPortal_CuteFuzz PyPortal_Tides Those are all I bothered to try. The errors are similar to what's being reported in this issue.

Suggest that tests that are run as part of CI include one of these more complex uses of the pyportal library, downloading multiple large files.

tannewt commented 3 years ago

@gmeader I believe it's requests 1.7.x which is a different from the 1.6.1 version of NINA.

We don't have a way currently to test on device so automating these tests is a bit tough. I did add CPython compatibility in 1.7.0 and unittests as well. So, as we fix these issues, we can add tests to make sure we don't regress again.

ladyada commented 3 years ago

i think this is resolved - if folks in the thread can try the latest release here (its not in bundle yet) please let us know any broken URLs :)

gmeader commented 3 years ago

Next week I will test: PyPortal_CMA_ART_FRAME PyPortal_NASA PyPortal_NewNewNew PyPortal_CuteFuzz (these are the Guides where large-ish images are downloaded) on a Titano with a new library bundle release (presumably sometime next week.) and report the results here.

makermelissa commented 3 years ago

This is still an issue though it is much less frequent. I have been trying out various things, but it still fails at least a quarter of the time. I have noticed when it does fail, it needs to reconnect to the Access Point, so that may be related.

ladyada commented 3 years ago

@gmeader did you have a chance to do more tests?

makermelissa commented 3 years ago

I suspect this issue is fixed at this point. @gmeader were you able to test?

askpatrickw commented 3 years ago

Hey @gmeader were you able to test again? I'm thinking we can close this.

gmeader commented 3 years ago

I tested PyPortal_CMA_ART_FRAME and PyPortal_NewNewNew with the latest CircuitPython and libraries

They both work, but occasionally get an error on CMA_ART_FRAME I'll report that on a new issue if I can determine if it is reproducible. So you can close this one

askpatrickw commented 3 years ago

Thanks @gmeader !