Improve OS.kill()'s error handling and add a method is_process_running to check for running processes

[MikeSchulze]

Describe the project you are working on:
I'm working on a unit test plugin. It startes a test client in the background (hidden application)
The client comunicates over network connection with der server running by the plugin.
To run the test client i use the provided functionallity OS.excetute() with this arguments
[--no-window, -d, -s, res://addons/gdUnit/src/core/GdUnitClient.gd]
The client is closing/terminate by it self when all tests are done.
To manually stop a running test client i use the pid from OS.excetute() and send a kill OS.kill() to terminate the client.
So good so far ;)

Describe the problem or limitation you are having in your project:
When i stop a running test client with OS.kill() results into OK for a success terminated process.
But when the client is already terminatet i have no chance to check if the client still running or not.
e.g. when i try now OS.kill() on a already terminated process i got only an generic error Error.FAILED

Describe the feature / enhancement and how it helps to overcome the problem or limitation:
To improve the OS.kill() it should return a better error

• add to the Error enum PID_NOT_EXIST
• replace the Error enum with and Error object where contains the error number and a human readable message

Add a new function to check if a process running by given PID

• OS.is_process_running(<pid>) -> bool
  returns true when a process is running by given PID or false

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:
With this better error handling on OS.kill() we can check if the process already terminated for better control flow.
With OS.is_process() we can verify if the process running or not.

If this enhancement will not be used often, can it be worked around with a few lines of script?:
i found no way to check if an process is running by given PID

Is there a reason why this should be core and not an add-on in the asset library?:
This is a missing function on the OS class to manage spawned processes

[Calinou]

add to the Error enum PID_NOT_EXIST

We'd prefer not to add new error codes for specific use cases. If you look at the current Error enum, nearly all error codes are designed to be generic.

replace the Error enum with and Error object where contains the error number and a human readable message

Maybe, but that's a pretty significant "global" change for a specific use case (until proven otherwise). Since this would affect all errors, this should be discussed in a separate proposal anyway.

OS.is_process()

I'd name it OS.is_process_running() instead, which is much clearer.

[MikeSchulze]

Thanks for this fast answer yes is_process_running is a better name ;)
A improvement for error handling like an extra function
OS.get_current_error_message() -> String would be helpfull

[mdavisprog]

Hello,

I am also interested in having an OS.is_process_running() function as my project makes use of external processes for interfacing with Lua.

If the core developers are interested, I could provide a pull request for implementing this function.

Looking at the master branch, it seems this function would only need to be implemented on Unix/Windows platforms and can be ignored on others. This also seems pretty straight forward since OS_Windows keeps a pid to process handle mapping. However, this will be a little different for the 3.3 branch but wouldn't be too difficult. The new API functions could even be ported to this branch, though I haven't looked at what is involved in this.

[Calinou]

@mdavisprog Feel free to open a pull request for this 🙂

[mhilbrunner]

godotengine/godot#38992 is relevant to the error part