search

adamrehn / ue4cli

Command-line interface for Unreal Engine 4
https://docs.adamrehn.com/ue4cli/
MIT License
249 stars 45 forks source link

`ue4cli` does not recover if the `config.json` file is not a valid JSON #53

Closed StephenSorriaux closed 3 months ago

StephenSorriaux commented 1 year ago

Hello,

Thanks for the project. Very useful and we are using it on a daily basis for our CI pipeline to build our project or even rebuild UE using our custom sources.

Today we got a weird crash using the ue4 setroot {path_to_engine} command on Windows:

Traceback (most recent call last):
  File "<Python Path>\lib\runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "<Python Path>\lib\runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "<redacted>\.env\Scripts\ue4.exe\__main__.py", line 7, in <module>
  File "<redacted>\.env\lib\site-packages\ue4cli\cli.py", line 222, in main
    SUPPORTED_COMMANDS[command]['action'](manager, args)
  File "<redacted>\.env\lib\site-packages\ue4cli\cli.py", line 18, in <lambda>
    'action': lambda m, args: m.setEngineRootOverride(args[0]),
  File "<redacted>\.env\lib\site-packages\ue4cli\UnrealManagerBase.py", line 40, in setEngineRootOverride
    ConfigurationManager.setConfigKey('rootDirOverride', rootDir)
  File "<redacted>\.env\lib\site-packages\ue4cli\ConfigurationManager.py", line 39, in setConfigKey
    return JsonDataManager(configFile).setKey(key, value)
  File "<redacted>\.env\lib\site-packages\ue4cli\JsonDataManager.py", line 38, in setKey
    data = self.getDictionary()
  File "<redacted>\.env\lib\site-packages\ue4cli\JsonDataManager.py", line 30, in getDictionary
    return json.loads(Utility.readFile(self.jsonFile))
  File "<Python Path>\lib\json\__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "<Python Path>\lib\json\decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "<Python Path>\lib\json\decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

This was due because of the config.json that was only containing spaces (ie. not a valid JSON), probably because the host was not shutdown cleanly. I fixed it by manually editing the config.json file to make it a valid JSON.

I think it would be great if that kind of (weird, I concur, but possible) case would be handled gracefully by ue4cli, maybe by catching the exception and "resetting" the config file (or simply giving a clear error message). But that is only my thinking, I let you decide if that would make sens or not and please feel free to close this issue if you don't see it as such.

sleeptightAnsiC commented 3 months ago

If I remember right, config.json always contains only one key for rootDirOverride, so this specific case is very easy to fix by always recreating the file while using ue4 setroot. This would become a problem though when trying to save more than one key in said file.

Perhaps .json files should not be used at all for configs, and each key should be split to a different text file so it can be easily recreated. For now, ue4cli uses only one config key so this won't be too complicated.

However, if config file is corrupted, ue4cli should still handle this case and yell about it, we cannot just implicitly recreate it.

Current implementation for reference: https://github.com/adamrehn/ue4cli/blob/39d6f6ab2aab1484d104850f2de8f8f8f3cf45cf/ue4cli/UnrealManagerBase.py#L33-L41 https://github.com/adamrehn/ue4cli/blob/39d6f6ab2aab1484d104850f2de8f8f8f3cf45cf/ue4cli/ConfigurationManager.py#L33-L39 https://github.com/adamrehn/ue4cli/blob/39d6f6ab2aab1484d104850f2de8f8f8f3cf45cf/ue4cli/JsonDataManager.py#L34-L40

adamrehn commented 3 months ago

I agree that automatically clearing the JSON file is undesirable behaviour (particularly in the event that we want to introduce additional configuration keys in a future version of ue4cli), but a more descriptive error message certainly sounds like a good idea!

I've implemented this in commit fed71c1, so you'll now see output like this if the JSON file is malformed:

$ ue4cli root
Error: malformed JSON configuration file "/home/adam/.config/ue4cli/config.json" (Expecting value: line 1 column 4 (char 3))