urequests socket doesn't close if OSError

[yelsherbini]

The below snippet does a get or post every n amoutn of seconds using the urequests library. Normally I would catch any exception that occurs and I log it. However, if an OSError occurs when reading the response the socket doesn't close.

from urequests import get
import time

while True:
    try:
        resp = get("https://server.com")
        print(resp.json())
    except OSError:
        print(e)
    time.sleep(10)

The culprit is the content method in the response class:

    @property
    def content(self):
        if self._cached is None:
            self._cached = self.raw.read() # If this fails the socket never closes
            self.raw.close()
            self.raw = None
        return self._cached

My way of countering this is to close the socket myself in the except part of my snippet but I thought this should be something addressed in the library itself since the socket is expected to close when reading the response to begin with.

[Ahmedkalnaggar]

@yahia13 Could you help me in overcoming this Error, as I found this Error too when trying to Catch the OSError on my WiPy Board, but if fails to send the next Requests too which forces me to reset the board. How could I close the socket myself too ?

[yelsherbini]

@Ahmedkalnaggar Hi Ahmed, here is a snippet of how I catch the error.

from urequests import get
import gc

def retrieve_url(url):
    gc.collect() 
    resp = None
    try:
        resp = get(url)
        value = resp.json()
    except Exception as e: # Here it catches any error.
        if isinstance(e, OSError) and resp: # If the error is an OSError the socket has to be closed.
            resp.close()
        value = {"error": e}
    gc.collect()
    return value

The reason I call gc.collect() twice is because if you make several requests in a loop the garbage collector doesn't free any memory in between requests and eventually runs out of memory. Hope it will be of help

[pfalcon]

@dpgeorge : What's your stance on this? My usual reply would be "use exception handling properly", but given that requests' API is "broken" by design (by slurping an entire response into memory), and we have much better alternative in the form of urllib.urequests (which I'll fix re: associated issue), we can as well add more "breakage" to urequests to automatically close socket on error.

[dpgeorge]

but given that requests' API is "broken" by design (by slurping an entire response into memory)

I don't understand what you mean: requests allows to lazily read in data only when needed. You pass stream=True to the HTTP function (eg get). requests even provides the ability to iterate through data via Response.iter_content.

we can as well add more "breakage" to urequests to automatically close socket on error.

It looks like requests has the same behaviour in that it doesn't try to close the raw stream if an error is encountered while reading: http://docs.python-requests.org/en/master/_modules/requests/models/#Response.iter_content

Standard requests uses non-streaming mode by default, which is arguably simpler and doesn't lead to problems like those reported here. Users using the advanced mode of stream=True would then be expected to catch OSErrors and close the response explicitly. urequests is in a kind of middle ground here: stream=True is the default yet it still tries to be user-friendly. So closing the socket on error seems like the right thing to do, trying to emulate stream=False mode (although the fact that r.content can raise an OSError is already a big difference to stream=False mode).

[pfalcon]

I don't understand what you mean

It's another way to say what you said: "Standard requests uses non-streaming mode by default". Which means that "nobody" knows that it can work differently.

Response.iter_content.

urequests doesn't have that, given that "nobody" uses it, and there's urllib.urequest which is streaming by default.

So closing the socket on error seems like the right thing to do

Ok, thanks, unreported here issues fixed in b715ee0cb80721db1449bda7d19c8e278fc3577d and 586ae64cb0f2afa7ef26aec360cb8b09948fd8f5, and reported here in 2e834672aa97856398835199ff786ad94ae247b4.