adafruit / Adafruit_CircuitPython_ESP32SPI

ESP32 as wifi with SPI interface
MIT License
101 stars 74 forks source link

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

Closed Stwerp closed 3 months ago

Stwerp commented 6 months ago

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: https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/issues/170#issuecomment-1734285273 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 https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/blob/956d6a08d287c6aef2556fc2d175df921390b747/adafruit_esp32spi/adafruit_esp32spi_socket.py#L154

This error is different than a TimeoutError that, for example, might come from https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/blob/956d6a08d287c6aef2556fc2d175df921390b747/adafruit_esp32spi/adafruit_esp32spi.py#L831

Is this an intentional difference? It seems to be, given the class definition on these lines https://github.com/adafruit/Adafruit_CircuitPython_ESP32SPI/blob/956d6a08d287c6aef2556fc2d175df921390b747/adafruit_esp32spi/adafruit_esp32spi_socket.py#L196-L201

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 commented 6 months ago

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

justmobilize commented 3 months ago

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