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!