FriendsOfGalaxy / galaxy-integration-origin

origin integration for galaxy
77 stars 14 forks source link

Crash if ProgramData is undefined in Environmental variables. #23

Closed NathanaelA closed 3 years ago

NathanaelA commented 4 years ago

Describe the bug If ProgramData is not defined in the environment the plugin crashes here: https://github.com/FriendsOfGalaxy/galaxy-integration-origin/blob/0aa5832b96de9d8b41ef35e64065ad41d3d6386a/src/local_games.py#L247

To Reproduce Set ProgramData= Start GOG and see if disconnect immediately and be in a crashed state crashed

Expected behavior It assumes that ProgramData will always be defined, I expect it not to crash if somehow this isn't set...

Fix Replace: local_content_path = os.path.join(os.environ["ProgramData"], "Origin", "LocalContent") with local_content_path = os.path.join(os.environ.get("ProgramData","C:\ProgramData"), "Origin", "LocalContent") As it will allow the normal value to be used if ProgramData is not set...

Relevant Log

Traceback (most recent call last): File "C:\users\nathanael\Local Settings\Application Data\GOG.com\Galaxy\plugins\installed\origin_7f53219b-4e2b-4591-9f4f-dfc5f4ba9eb0\galaxy\api\plugin.py", line 1140, in create_and_run_plugin asyncio.run(coroutine()) File "D:\obj\Windows-Release\37win32_Release\msi_python\zip_win32\runners.py", line 43, in run File "D:\obj\Windows-Release\37win32_Release\msi_python\zip_win32\base_events.py", line 584, in run_until_complete File "C:\users\nathanael\Local Settings\Application Data\GOG.com\Galaxy\plugins\installed\origin_7f53219b-4e2b-4591-9f4f-dfc5f4ba9eb0\galaxy\api\plugin.py", line 1130, in coroutine async with plugin_class(reader, writer, token) as plugin: File "C:\users\nathanael\Local Settings\Application Data\GOG.com\Galaxy\plugins\installed\origin_7f53219b-4e2b-4591-9f4f-dfc5f4ba9eb0\plugin.py", line 64, i n init self._local_games = LocalGames(get_local_content_path()) File "C:\users\nathanael\Local Settings\Application Data\GOG.com\Galaxy\plugins\installed\origin_7f53219b-4e2b-4591-9f4f-dfc5f4ba9eb0\local_games.py", line 247, in get_local_content_path local_content_path = os.path.join(os.environ["ProgramData"], "Origin", "LocalContent") File "D:\obj\Windows-Release\37win32_Release\msi_python\zip_win32\os.py", line 678, in getitem KeyError: 'ProgramData'

FriendsOfGalaxy commented 3 years ago

Hi @NathanaelA thanks for report.

Is there any valid scenario when %Programdata% is unset? What was your case?

Defaulting to C:\ProgramData looks reasonable.. however system drive letter could be not "C" as well. Maybe %SystemDrive%\ProgramData? But on the other hand if you break one of your environment vars then, SystemDrive can we also broken. I think that is the user responsibility - if you modify your system "read-only" vars, then you can break your system basic functionality.

NathanaelA commented 3 years ago

Is there any valid scenario when %Programdata% is unset? What was your case?

Wine had it unset, not sure why because normally Wine is pretty good about things like that... In addition I just went back and checked several of my Virtual Machines, and Win XP also does not have it set. (Appears that Microsoft added this in Vista)

Defaulting to C:\ProgramData looks reasonable.. however system drive letter could be not "C" as well

I've used windows since Windows 2.0 --- Virtually all machines have ProgramData set to C:\ProgramData; only people who move it have it in a different location. That is why that is a good default value for in the weird case it is no longer set.

Checking multiple of my VM's, with different Windows versions; they all show this... image

if you modify your system "read-only" vars

Agreed, but in this case it actually wasn't set, and probably is a issue with Wine ultimately, but no reason you can't bulletproof your code either. :grinning:

FriendsOfGalaxy commented 3 years ago

ah, Wine. Sure, no reason to be stubborn here :) Would like like to submit PR?