Skip to content
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

Is the timeout and TimeoutError intended to be different? #197

Closed
Stwerp opened this issue Mar 20, 2024 · 2 comments
Closed

Is the timeout and TimeoutError intended to be different? #197

Stwerp opened this issue Mar 20, 2024 · 2 comments

Comments

@Stwerp
Copy link

Stwerp commented Mar 20, 2024

Some students are having issues with their systems running for lengths longer than an hour or so. We've been able to catch some errors, and it seems that a solution like: #170 (comment) is the best idea so far and wrapping everything related to the actual sending of the GET/POST call in a try block

try:
    [...]
except (ValueError, RuntimeError, ConnectionError, OSError, TimeoutError) as e:
   [... handle reset and retry]

However, we've also seen students have a timeout error coming from

This error is different than a TimeoutError that, for example, might come from

raise TimeoutError("Failed to establish connection")

Is this an intentional difference? It seems to be, given the class definition on these lines

class timeout(TimeoutError):
"""TimeoutError class. An instance of this error will be raised by recv_into() if
the timeout has elapsed and we haven't received any data yet."""
def __init__(self, msg):
super().__init__(msg)

I'm just not understanding why. If this is the case, then we should be catching a TimeoutError as well as a timeout error, so our except block would have this:

try:
    [...]
except (ValueError, RuntimeError, ConnectionError, OSError, TimeoutError, timeout) as e:
   [... handle reset and retry]

That seems a little strange to me, but I just wanted to check (and also be able to tell students why they need to catch both those errors).

@justmobilize
Copy link
Collaborator

@FoamyGuy was there a reason in this commit you didn't just raise a TimeoutError?

@justmobilize
Copy link
Collaborator

@Stwerp timeout has been removed. It was originally in there to match older versions of Python

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

No branches or pull requests

2 participants