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

Fix make_request Retry Error and Prevent 403/429 in Docker #344

Open
wants to merge 23 commits into
base: develop
Choose a base branch
from

Conversation

dmytrokoren
Copy link
Contributor

Seems to resolve the make_request retry error in Docker when fetching matching flights and running fare_checker.

@dmytrokoren
Copy link
Contributor Author

dmytrokoren commented Feb 20, 2025

docker pull dmytrokoren/auto-southwest-check-in:develop


*Docker (Synology):* CleanShot 2025-02-19 at 22 03 16@2x

@jdholtz
Copy link
Owner

jdholtz commented Feb 20, 2025

In this case, do we still need the webdriver logic to start the virtual display? I don’t think we should need it as the browser isn’t headed anymore.

@dmytrokoren
Copy link
Contributor Author

In this case, do we still need the webdriver logic to start the virtual display? I don’t think we should need it as the browser isn’t headed anymore.

While headless mode avoids the overhead of rendering a GUI, the virtual display adds another layer of compatibility to ensure the environment is closer to a typical user's machine.

@dmytrokoren
Copy link
Contributor Author

Let me do more testing—let's park this pull request for now.

@dmytrokoren dmytrokoren changed the title Seems to resolve the make_request retry error in Docker Fix make_request Retry Error and Prevent 403/429 in Docker Mar 4, 2025
@dmytrokoren
Copy link
Contributor Author

@jdholtz please review and let me know thanks.

@jdholtz
Copy link
Owner

jdholtz commented Mar 7, 2025

@dmytrokoren I just added some more linting rules + changes to the repository to improve the code quality. Do you mind resolving the conflicts? Then I'll take a look. And as always, thanks for the hard work you do to help prevent 403/429 errors!

jdholtz and others added 3 commits March 7, 2025 13:07
other

This is an attempt at fixing jdholtz#325 and improving the speed of check-ins.
During check-in, what usually happens is that a bad request (400) will
go through on the first attempt. This will then take ~3 seconds to
finish before the next request is attempted.

This change will allow multiple threads to check in at the same time,
each starting one second after the other. This will reduce the time
between check-in requests and hopefully reduce the total time it takes
to check in.
@jdholtz
Copy link
Owner

jdholtz commented Mar 8, 2025

Hey @dmytrokoren, the multiple-checkin-threads is not ready to be merged into the develop branch as it hasn't been tested or even known to fix the issue it is intended for. Could you remove it from this PR? Maybe it was accidentally added, but want to make sure.

@dmytrokoren
Copy link
Contributor Author

Hey @dmytrokoren, the multiple-checkin-threads is not ready to be merged into the develop branch as it hasn't been tested or even known to fix the issue it is intended for. Could you remove it from this PR? Maybe it was accidentally added, but want to make sure.

Hey @jdholtz, I merged it to test, but I’ll roll it back. I want to try adding a 2-second delay before making the POST request for check-in — I’ll push a new commit for that.

…e-checkin-threads' into webdriver-enhacement"

This reverts commit 2c698f2, reversing
changes made to d84cddf.
…ate-limiting

Implemented driver.close() to close all browser windows before calling driver.quit() to avoid issues with shutil temporary folder removal
Changed retry attemp to 2 from 1
and decreased retry wait time from 20 to 10 seconds
@dmytrokoren
Copy link
Contributor Author

@jdholtz can we review this PR?

Copy link
Owner

@jdholtz jdholtz left a comment

Choose a reason for hiding this comment

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

Looks good overall! Thanks especially for also adding tests.

I wonder if all of the bypasses are needed--filling out the check-in form, the user data dir changes, etc. Have these been verified that detection is much better with all of them, and not just one? Just curious myself, and want to make sure we add what's necessary without extra things that don't help prevent these errors.

Is the user_data_dir persistence needed? The temp dir seems to be removed every time the driver is ran, with the exception of when a driver timeout error is raised--which I think is a bug as there is no quit() call before raising the timeout error (but I will add it).

@@ -206,6 +206,8 @@ def _check_in_to_flight(self) -> JSON:
site = CHECKIN_URL + self.flight.confirmation_number

logger.debug("Making first POST request to check in")
time.sleep(2)
Copy link
Owner

Choose a reason for hiding this comment

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

Let's figure out the delay in #325 instead of adding it here as this has to do more with 400s than 403/429s.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to test how multi-threading performs in real-time. Based on my calculations, Southwest’s API starts accepting check-ins approximately two seconds after the official check-in time.

try:
self._monitor()
except KeyboardInterrupt:
# Add a small delay so the MainThread's message prints first
time.sleep(0.05)
# Lock so all processes are stopped sequentially
with self.lock:
webdriver.reset_temp_dir()
Copy link
Owner

Choose a reason for hiding this comment

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

With an instance of the webdriver class being solely created to reset the temp directory, could we make reset_temp_dir a static or class method so we don't need to create a new instance that isn't otherwise used? Not very familiar with the classmethod syntax, so let me know if this isn't feasible.

driver.get(CHECKIN_URL)
driver.click_if_visible(".button-popup.confirm-button")
driver.click_if_visible("#onetrust-accept-btn-handler")

Copy link
Owner

Choose a reason for hiding this comment

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

Can we move the after_page_load.png screenshot to after these clicks, and then put a screenshot, after_form_submission.png where the page load screenshot is now? That'll help with specifically debugging this part of the webdriver, if needed.

@@ -284,7 +328,9 @@ def _set_account_name(self, account_monitor: AccountMonitor, response: JSON) ->
) # Don't log as it contains sensitive information

def _quit_driver(self, driver: Driver) -> None:
driver.close()
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not sure if we need this. .close() only appears once in the SeleniumBase examples, and that is only after opening a new window to bypass Cloudflare's bot detection, so I think .quit() is sufficient.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that if I don’t call .close() before .quit(), I get an error in the console when deleting the temporary directory.

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