Minor annoyance: Control-C on ue4 run raises an exception

[TBBle]

I've been "just living with" this for ages, and thought I should bug-report it.

> ue4 run "Highrise" -server -nosteam
# <stuff happens, then I hit Control-C>
[2021.02.05-09.52.09:829][630]LogCore: Engine exit requested (reason: ConsoleCtrl RequestExit)
[2021.02.05-09.52.09:831][630]LogCore: Warning: *** INTERRUPTED *** : SHUTTING DOWN
[2021.02.05-09.52.09:831][630]LogCore: Warning: *** INTERRUPTED *** : CTRL-C TO FORCE QUIT
[2021.02.05-09.52.09:831][630]LogCore: Engine exit requested (reason: EngineExit() was called; note: exit was already requested)
[2021.02.05-09.52.09:834][630]LogInit: Display: PreExit Game.
[2021.02.05-09.52.09:847][630]LogWorld: BeginTearingDown for /Game/Maps/Highrise
# <nice clean shutdown from UE4>
[2021.02.05-09.52.11:635][630]LogModuleManager: Shutting down and abandoning module RSA (3)
[2021.02.05-09.52.11:647][630]LogExit: Exiting.
Traceback (most recent call last):
  File "c:\program files\python39\lib\runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "c:\program files\python39\lib\runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "C:\Users\paulh\.local\bin\ue4.exe\__main__.py", line 7, in <module>
  File "c:\users\paulh\.local\pipx\venvs\ue4cli\lib\site-packages\ue4cli\cli.py", line 222, in main
    SUPPORTED_COMMANDS[command]['action'](manager, args)
  File "c:\users\paulh\.local\pipx\venvs\ue4cli\lib\site-packages\ue4cli\cli.py", line 60, in <lambda>
    'action': lambda m, args: m.runEditor(
  File "c:\users\paulh\.local\pipx\venvs\ue4cli\lib\site-packages\ue4cli\UnrealManagerBase.py", line 365, in runEditor
    Utility.run([self.getEditorBinary(True), projectFile, '-stdout', '-FullStdOutLogOutput'] + extraFlags, raiseOnError=True)
  File "c:\users\paulh\.local\pipx\venvs\ue4cli\lib\site-packages\ue4cli\Utility.py", line 143, in run
    returncode = subprocess.call(command, cwd=cwd, shell=shell)
  File "c:\program files\python39\lib\subprocess.py", line 351, in call
    return p.wait(timeout=timeout)
  File "c:\program files\python39\lib\subprocess.py", line 1185, in wait
    return self._wait(timeout=timeout)
  File "c:\program files\python39\lib\subprocess.py", line 1466, in _wait
    result = _winapi.WaitForSingleObject(self._handle,
KeyboardInterrupt

I think it'd be nice if that if a KeyboardInterrupt is raised from subprocess.call, the called subprocess was assumed to have handled it, and it was treated as a clean exit.

It'd be nicer if the return code from the subprocess.call was returned like any other exit.

And poking around, python/cpython#5026 and the relevant wait function, it looks like since Python 3.7 there's a _sigint_wait_secs value which defaults to 250ms for the first control-c, and that's probably too short for us here, which is why we don't get the "nicer" version already.

So a possible improvement would be to use the Popen object directly (replace the call to subprocess.call with its implementation, like Utility.capture) and then either change _sigint_wait_secs before waiting to a higher first-time value, or wrap the call to wait with another KeyboardInterrupt catch that perhaps doesn't put a timeout on the first control-C (assuming UE4's handling it) and has a second control-c catch that gives a timeout for the UE4 "force-quit", and a third control-C would immediately kill, if that second timeout is too long for the user in the heat of the moment.

If I have some spare time, I could take a shot at implementing it (probably the latter approach, doing more wait calls in Utility.py, not changing _sigint_wait_secs) unless you have any concerns, or want to offer particular advice on this?

Should this only apply to runEditor? I haven't checked if UAT has a similar "one to exit cleanly, two to hard-kill" behaviour, or if that's Editor-only.

[sleeptightAnsiC]

This is a general problem with Utility class which does a bunch of I/O operations but does not catch any exceptions related to them. These for sure shouldn't be treated as success, including KeyboardInterrupt.

Interrupting a process is considered as fatal error, imagine the following case: ue4 gen && ue4 build && ue4 run Interrupting gen or build would mean that the chain is broken. If ue4cli was returning 0 in this case, that would be very unexpected.

Though, I agree these cases should be catch, since current behavior is too confusing.

Probably the easiest way to fix it is creating custom Exception for Utility class, then wrapping every I/O call with try/except block - if it fails, it should raise said custom Exception, which then can be catch inside of cli::main. This is how UnrealManagerException is being used right now to handle similar problems.

[sleeptightAnsiC]

Proposed fix #65

With said change you should get something like this:

$ ue4 run
# logging from Engine...
^C
(ue4cli) Error: (KeyboardInterrupt)
# return code 1