Do not silence subprocess's exit code and avoid polluting `stdout` #31

loathingKernel · 2024-02-19T13:59:56Z

This PR aims to address the following issues.

Capture and return the exit code from the subprocess to the caller in case ULWGL itself was executed successfully. The caller might use the exit code to identify issues in the execution of the subprocess or ULWGL itself. Note: the ruff check was explicitly disabled because the exception at line is used for logging, similar to the patterns ruff ignores by default
ULWGL should use stderr only for its own logging to avoid polluting stdout where the caller might expect the result of the argument command. Examples of such internal commands are winepath.exe and reg.exe where the output can be used by the caller.

@R1kaB3rN Since you are actively working on the python code and they might cause conflicts, feel free to either cherry-pick or merge by hand any of these changes if you agree with them. I also updated a few long strings to not exceed the 100th column to make the code slightly easier to parse.

ulwgl_run.py

Co-authored-by: Stelios Tsampas <[email protected]> - Do not print to stdout when finding or downloading Proton as it is not intended to be processed directly as input for other applications. - Related to #31 (comment)

R1kaB3rN · 2024-02-19T19:54:15Z

ulwgl_run.py

+        print(f"{e}", file=sys.stderr)
+        sys.exit(1)
+    else:
+        sys.exit(ret)


Please correct me if I am wrong, but print_tb just formats the traceback nicely compared to simply raising the exception, correct?

Yes and no, print_tb is the way to print the traceback from the exception object, but ideally here, if logging is implemented, the print_tb and print calls should be replaced with logging.excpetion. This was an intermediate solution without using logging

The point here is to not simply raise the exception and exit due to it, but handle it, print the information and exit with an exit code of our choice.

ulwgl_run.py

R1kaB3rN · 2024-02-19T20:08:59Z

ulwgl_run.py

+        ret = main()
+    except Exception as e:  # noqa: BLE001
+        traceback.print_tb(e.__traceback__, file=sys.stderr)
+        print(f"{e}", file=sys.stderr)


Because of the above line, we shouldn't have to print twice, right?

In case of print_tb it was required to get the message. It was printing only the traceback. In case print_exception behaves differently, I will remove it.

No, I believe this is unnecessary and I've confirmed this when using print_exception on a syntax error

* Return the exitcode of the subprocess from `main` in case the caller wants to do something this it. * Catch any exceptions during execution and return an error exitcode.

* Avoid polluting `stdout` since the caller might expect the output of the command there.

loathingKernel force-pushed the stderr branch from 5772887 to 3d8942a Compare February 19, 2024 14:07