Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.x] Improve error message when OS.execute() fails on Windows #97171

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Sep 19, 2024

This takes the error message from 4.x and backports it to 3.x.

PS: I noticed we don't have any such message when OS.execute() fails on Linux (and possibly macOS). This also applies to 4.x.

Testing project: test_os_execute_3.x.zip

Preview

Before

ERROR: Condition "ret == 0" is true. Returned: ERR_CANT_FORK
   at: execute (platform/windows/os_windows.cpp:2981)
ERROR: Condition "ret == 0" is true. Returned: ERR_CANT_FORK
   at: execute (platform/windows/os_windows.cpp:2981)

After

ERROR: Could not create child process: non-existent-command
   at: (platform/windows/os_windows.cpp:2981)
ERROR: Could not create child process: non-existent-command with arguments
   at: (platform/windows/os_windows.cpp:2981)

This takes the error message from 4.x and backports it to 3.x.
@Calinou Calinou added this to the 3.7 milestone Sep 19, 2024
@Calinou Calinou requested a review from a team as a code owner September 19, 2024 00:12
@Calinou Calinou changed the title Improve error message when OS.execute() fails on Windows [3.x] Improve error message when OS.execute() fails on Windows Sep 19, 2024
@akien-mga akien-mga requested a review from lawnjelly September 19, 2024 09:37
@lawnjelly lawnjelly merged commit 553030b into godotengine:3.x Sep 20, 2024
14 checks passed
@lawnjelly
Copy link
Member

Thanks!

@Calinou Calinou deleted the os-execute-windows-improve-cant-fork-error-message branch September 23, 2024 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants