Skip to content

Catch recv-into exception to prevent Out of sockets #72

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 8, 2021
Merged

Catch recv-into exception to prevent Out of sockets #72

merged 2 commits into from
Feb 8, 2021

Conversation

anecdata
Copy link
Member

@anecdata anecdata commented Feb 6, 2021

An ETIMEDOUT exception occurring on a socket.recv_into call in a Session request would leave the socket open, and after two occurrences, would get RuntimeError: Out of sockets and require a microcontroller reset.

Traceback (most recent call last):
  File "code.py", line 675, in http_get
  File "/lib/adafruit_requests.py", line 595, in get
  File "/lib/adafruit_requests.py", line 572, in request
OSError: [Errno 116] ETIMEDOUT
# ...
Traceback (most recent call last):
  File "code.py", line 675, in http_get
  File "/lib/adafruit_requests.py", line 595, in get
  File "/lib/adafruit_requests.py", line 572, in request
OSError: [Errno 116] ETIMEDOUT
# ...
Traceback (most recent call last):  
  File "code.py", line 675, in http_get
  File "adafruit_requests.py", line 595, in get
  File "adafruit_requests.py", line 559, in request
  File "adafruit_requests.py", line 436, in _get_socket
  File "adafruit_requests.py", line 433, in _get_socket
RuntimeError: Out of sockets

After this change, if the failure persists, it will raise an OutOfRetries: exception, just as in the case of persistent _send_request failures.

fix socket leak
@anecdata anecdata requested a review from brentru February 8, 2021 17:49
@brentru
Copy link
Member

brentru commented Feb 8, 2021

@anecdata I have a question..since this eventually raises OutOfRetries if we cant obtain a socket, do we want to pass some "out of sockets" RuntimeError text to OutOfRetries? Currently it's just raising which isn't descriptive about why it failed.

@anecdata
Copy link
Member Author

anecdata commented Feb 8, 2021

It seems to be transient timeouts that recover naturally, but more info is better. I know nothing about custom exceptions, but I'm sure I can find an example somewhere. New commit forthcoming.

@anecdata
Copy link
Member Author

anecdata commented Feb 8, 2021

Managed to catch it in testing:

Traceback (most recent call last):
  File "code.py", line 663, in http_get
  File "/lib/adafruit_requests.py", line 598, in get
  File "/lib/adafruit_requests.py", line 583, in request
OutOfRetries: Repeated socket failures

Copy link
Member

@brentru brentru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good!

@anecdata anecdata merged commit 622856f into adafruit:master Feb 8, 2021
@anecdata anecdata deleted the recv_into branch February 8, 2021 20:03
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Feb 9, 2021
aggieNick02 added a commit to PCPartPicker/Adafruit_CircuitPython_Requests that referenced this pull request Sep 27, 2024
…exiting the method without the created socket being owned by a response or closed. This would then cause future calls to get a socket with the same parameters to fail because one already existed and hadn't been closed. The PR adafruit#72 fixed this issue for one case where an exception could be raised. This generalizes that in an attempt to fix it in other cases that could still be hit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants