grrttedwards / wow-addon-updater

Python utility for updating World of Warcraft addons
GNU General Public License v3.0
71 stars 14 forks source link

Multithreading causing failed updates #72

Closed dagviken closed 4 years ago

dagviken commented 5 years ago

It seems that the clever multi-threading addon updating is not working as expected, possibly because of too many requests being sent to the same site at once. When updating 10 addons or less, it is not much of a problem. But I have 60+ addons (too much, I know), and it fails every time. You get messages like this: Failed to find addon version number for game version: GameVersion.retail, https://www.wowinterface.com/downloads/info8533-Altoholic.html

When you run it again, it may be OK.

Or you get messages like this:

Exception in thread Thread-17: Traceback (most recent call last): File "C:\home\game\WowAddonUpdater\wow-addon-updater-retail\updater\site\curse.py", line 60, in _convert_old_curse_urls page.raise_for_status() File "C:\Users\davi.virtualenvs\wow-addon-updater-retail-4k4Ric-f\lib\site-packages\requests\models.py", line 940, in raise_for_status raise HTTPError(http_error_msg, response=self) requests.exceptions.HTTPError: 403 Client Error: Forbidden for url: https://wow.curseforge.com/projects/deadly-boss-mods-dbm-legion

The above exception was the direct cause of the following exception:

Traceback (most recent call last): File "c:\program files\python37\Lib\threading.py", line 926, in _bootstrap_inner self.run() File "c:\program files\python37\Lib\threading.py", line 870, in run self._target(*self._args, **self._kwargs) File "C:\home\game\WowAddonUpdater\wow-addon-updater-retail\updater\manager\addon_manager.py", line 73, in update_addon site = site_handler.get_handler(addon_url, self.game_version) File "C:\home\game\WowAddonUpdater\wow-addon-updater-retail\updater\site\site_handler.py", line 16, in get_handler return Curse(url, game_version) File "C:\home\game\WowAddonUpdater\wow-addon-updater-retail\updater\site\curse.py", line 15, in init url = Curse._convert_old_curse_urls(url) File "C:\home\game\WowAddonUpdater\wow-addon-updater-retail\updater\site\curse.py", line 63, in _convert_old_curse_urls raise SiteError(f"Failed to find the current page for old URL: {url}") from e updater.site.abstract_site.SiteError: Failed to find the current page for old URL: https://wow.curseforge.com/projects/deadly-boss-mods-dbm-legion

My solution has been to disable the multithreading:

Original code in addonmanager.py:

def update_all(self):
    threads = []

    with open(self.addon_list_file, 'r') as fin:
        addon_entries = fin.read().splitlines()

    # filter any blank lines or lines commented with an octothorp (#)
    addon_entries = [entry for entry in addon_entries if entry and not entry.startswith('#')]

    for addon_entry in addon_entries:
        thread = threading.Thread(target=self.update_addon, args=(addon_entry,))
        threads.append(thread)
    for thread in threads:
        thread.start()
    for thread in threads:
        thread.join()

    self.set_installed_versions()
    self.display_results()

Now just comment out the thread join loop, making all threads run in sequence:

   for addon_entry in addon_entries:
        thread = threading.Thread(target=self.update_addon, args=(addon_entry,))
        threads.append(thread)
    for thread in threads:
        thread.start()
    #for thread in threads:
        thread.join()

The code now runs without any problems, albeit slower.

/br Dag

dagviken commented 5 years ago

I also have to say that CurseForge is no longer working with automated downloads, but this issue also applies to wowinterface.

grrttedwards commented 5 years ago

Hi @dagviken

Now just comment out the thread join loop, making all threads run in sequence:

So you're seeing issues with just too many connections at once? That's surprising, but we can do better by just using the hidden multiprocessing ThreadPool

Try this patch https://github.com/grrttedwards/wow-addon-updater/commit/2db9900034ab0fe5f3b9560d99729e1e832240de and let me know if it's any better? I have never seen the concurrency be an issue, but never tested it with more than ~30 addons personally.

I also have to say that CurseForge is no longer working with automated downloads, but this issue also applies to wowinterface.


~Curse and WoWAce's CDN Cloudflare has put in some new scraping prevention, and those are broken right now. There are two other issues open for that (#68, #69). I don't think that can be fixed on our side without resorting to some really unsavory methods.~

Edit: #73 has been raised, which will address the Curse issues.

grrttedwards commented 4 years ago

:) #103