jdholtz / auto-southwest-check-in

A Python script that automatically checks in to your Southwest flight 24 hours beforehand.
GNU General Public License v3.0
460 stars 89 forks source link

Occasionally getting NTPExceptions - uncaught #256

Closed aaron-pham closed 6 months ago

aaron-pham commented 6 months ago

Version

7.4

Browser Version

124.0.6367.118

Description

I'm not quite sure why, but locally on my machine I occasionally get a NTPException. I'm not sure what exactly happens (or why), but I notice that during the NTP call we're only catching a socket exception for falling back to system time. So I think we need to catch a more general exception.

To Reproduce

  1. Run script with fare check on

Expected Behavior

If an exception occurs, fall back to system time.

Relevant logs and program output

2024-05-02 10:58:23 DEBUG Process-1[fare_checker:30]: Checking current price for flight
2024-05-02 10:58:23 DEBUG Process-1[fare_checker:88]: Retrieving reservation information
Process Process-1:2:
Traceback (most recent call last):
  File "C:\Users\Aaron\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\ntplib.py", line 318, in request
    response_packet, src_addr = s.recvfrom(256)
TimeoutError: timed out

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.3056.0_x64__qbz5n2kfra8p0\lib\multiprocessing\process.py", line 314, in _bootstrap
    self.run()
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.3056.0_x64__qbz5n2kfra8p0\lib\multiprocessing\process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "C:\seg\auto-southwest-check-in\lib\checkin_handler.py", line 78, in _set_check_in
    self._wait_for_check_in(checkin_time)
  File "C:\seg\auto-southwest-check-in\lib\checkin_handler.py", line 85, in _wait_for_check_in
    current_time = get_current_time()
  File "C:\seg\auto-southwest-check-in\lib\utils.py", line 75, in get_current_time
    response = c.request(NTP_SERVER, version=3)
  File "C:\Users\Aaron\AppData\Local\Packages\PythonSoftwareFoundation.Python.3.10_qbz5n2kfra8p0\LocalCache\local-packages\Python310\site-packages\ntplib.py", line 323, in request
    raise NTPException("No response received from %s." % host)
ntplib.NTPException: No response received from us.pool.ntp.org.
2024-05-02 10:58:25 DEBUG Process-1[utils:42]: Successfully made request after 1 attempts
2024-05-02 10:58:25 DEBUG Process-1[fare_checker:103]: Retrieving search information for the current flight

Additional context

No response

jdholtz commented 6 months ago

Thanks for filing this issue. I fixed this in the latest commit of the develop branch.

While falling over to local time is what we want in these scenarios, I still want to see if we can make the NTP requests more reliable (another reason why I don't want to catch a very general exception, as I want to see what exceptions actually could come from this request so it can be looked into and made more reliable, meaning reverting to local time is used as little as possible).

This is where the exception is being raised, which is from a socket timeout. Before merging the latest commit that catches this error, would you be able to set a longer timeout and see if you still get the exception a lot? If you aren't, I can adjust the timeout in the script to make NTP requests more reliable. The change to be done is below:

diff --git a/lib/utils.py b/lib/utils.py
index 77cdb61..9a17a5c 100644
--- a/lib/utils.py
+++ b/lib/utils.py
@@ -72,8 +72,8 @@ def get_current_time() -> datetime:
     c = ntplib.NTPClient()

     try:
-        response = c.request(NTP_SERVER, version=3)
+        response = c.request(NTP_SERVER, version=3, timeout=10)
     except socket.gaierror:
         logger.debug("Error requesting time from NTP server. Using local time")
         return datetime.utcnow()

10 seconds seems a bit long, but if the requests do succeed after a longer time, then I see that being a good thing especially because this function is never called when timing is absolutely critical.

aaron-pham commented 6 months ago

Yeah, let me change the timeout and see how often it happens (if at all).

I am surprised that the default (5 second) timeout is still not long enough sometimes. Makes sense about not catching a more general exception.

jdholtz commented 6 months ago

Hey @aaron-pham, have you still been reaching the NTP exceptions with a longer timeout?

aaron-pham commented 6 months ago

I have not seen the timeouts ever since changing it to 10 seconds. Think we can close this issue now!

jdholtz commented 6 months ago

Awesome, I will make the change in the script too. Thanks for following up @aaron-pham!

aaron-pham commented 6 months ago

Lol of course after I thought it's working good & complete, I have some issues haha.

I'm heading to work so didn't have a chance to debug it at all, but here's the stack trace:

2024-05-15 09:45:27 DEBUG Process-1[utils:78]: Error requesting time from NTP server. Using local time
Process Process-1:
2024-05-15 09:45:29 DEBUG Process-5[reservation_monitor:64]: Lock acquired
2024-05-15 09:45:29 DEBUG Process-5[checkin_scheduler:51]: Refreshing headers for current session
2024-05-15 09:45:29 DEBUG Process-5[webdriver:133]: Starting webdriver for current session (this may take a few minutes)
Traceback (most recent call last):
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.3056.0_x64__qbz5n2kfra8p0\lib\multiprocessing\process.py", line 314, in _bootstrap
    self.run()
  File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.10_3.10.3056.0_x64__qbz5n2kfra8p0\lib\multiprocessing\process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "C:\seg\auto-southwest-check-in\lib\reservation_monitor.py", line 44, in monitor
    self._monitor()
  File "C:\seg\auto-southwest-check-in\lib\reservation_monitor.py", line 174, in _monitor
    self._schedule_reservations(reservations)
  File "C:\seg\auto-southwest-check-in\lib\reservation_monitor.py", line 88, in _schedule_reservations
    self.checkin_scheduler.process_reservations(confirmation_numbers)
  File "C:\seg\auto-southwest-check-in\lib\checkin_scheduler.py", line 42, in process_reservations
    flights.extend(self._get_flights(confirmation_number))
  File "C:\seg\auto-southwest-check-in\lib\checkin_scheduler.py", line 65, in _get_flights
    if flight.departure_time > current_utc_time:
TypeError: can't compare offset-naive and offset-aware datetimes

Let me know if you want me to open a new issue. Also, I probably need to investigate why NTP is timing out so much on my PC. It seems abnormal.

jdholtz commented 6 months ago

Thanks @aaron-pham. When I replaced datetime.utcnow with datetime.now(timezone.utc), I didn't know that the latter had tzinfo which is why this exception was being thrown. I just made the fix and improved a test so this issue will be caught in the future for this scenario.