miguelgrinberg/microdot

Support for timeouts?

jwmonaco opened this issue · 4 comments

I'm using microdot on pico w and found that the server would seemingly stop responding after making a second connection.

The problem is the browser in the first connection is making an extra connection to the server yet providing no data on that connection. Since I don't have threads that second connection would sit in a readline() call in Request.create() forever and no new connections could be made.

The super simple fix I did was just put a timeout on the socket:

   diff --git a/src/microdot.py b/src/microdot.py
   index a40ea15..dcdc4d0 100644
   --- a/src/microdot.py
   +++ b/src/microdot.py
   @@ -1090,6 +1090,7 @@ class Microdot():
         else:
             stream = sock
 
+        stream.settimeout(1.0)
         req = None
         res = None
         try:

This meets my needs, but the ask is why isn't there a timeout in the readline to prevent this sort of thing . . . or maybe there is a timeout and it is super large by default so maybe it makes sense to add timeout as parameter to run?

There was a somewhat related report in #89. I think this happens because your client is keeping the socket connected in just it needs to send another request. You can try sending a Connection: close header in your response, which may make your client drop the socket right after it gets the response.

That report does sound very similar.

Unfortunately, I tried the Connection: close header but there was no change in behavior. Your diagnosis about the client keeping an open socket for another request seems spot on.

My client is chrome browser. I went searching and found this nugget on stack overflow . Far from definitive, but this sounds like exactly what I'm seeing. The suggested remedies are not readily available in this environment.

This is clearly not a microdot issue per se, but timeout does seem like it might be a good feature.

Beyond this tiny hiccup, this package has been great. Thanks for putting it out there!

Yes, I think adding a timeout might be a good idea, I'll look into that. Using the asyncio version should also address this problem indirectly, by providing support for concurrent requests.

@jwmonaco I have added a timeout based on your suggestion. Thanks!