Fix process exit code error handling

src/process/ProcessManagerImpl.cpp

@@ -57,6 +57,38 @@ enum ProcessLifecycle
 namespace stellar
 {
 
+// Custom error category for process exit codes
+class process_exit_category_impl : public std::error_category
+{
+  public:
+    virtual const char* name() const noexcept override
+    {
+        return "process_exit";
+    }
+
+    virtual std::string message(int value) const override
+    {
+        if (value == 0)
+        {
+            return "Process exited successfully";
+        }
+        else if (value == -1)
+        {
+            return "Process terminated by signal";
+        }
+        else
+        {
+            return fmt::format("Process exited with code {}", value);
+        }
+    }
+};
+
+const std::error_category& process_exit_category()
+{
+    static process_exit_category_impl instance;
+    return instance;
+}
+
 static const asio::error_code ABORT_ERROR_CODE(asio::error::operation_aborted,
                                                asio::system_category());
 
@@ -66,7 +98,7 @@ mapExitStatusToErrorCode(std::string const& cmdLine, int pid, int status)
 // On windows, an exit status is just an exit status. On unix it's
 // got some flags incorporated into it.
 #ifdef _WIN32
-    return asio::error_code(status, asio::system_category());
+    return asio::error_code(status, process_exit_category());
 #else
     if (WIFEXITED(status))
     {
@@ -98,23 +130,14 @@ mapExitStatusToErrorCode(std::string const& cmdLine, int pid, int status)
             CLOG_WARNING(Process, "");
         }
 #endif
-        // FIXME: this doesn't _quite_ do the right thing; it conveys
-        // the exit status back to the caller but it puts it in "system
-        // category" which on POSIX means if you call .message() on it
-        // you'll get perror(value()), which is not correct. Errno has
-        // nothing to do with process exit values. We could make a new
-        // error_category to tighten this up, but it's a bunch of work
-        // just to convey the meaningless string "exited" to the user.
-        return asio::error_code(WEXITSTATUS(status), asio::system_category());
+        // Use our custom process_exit_category for proper error messages
+        return asio::error_code(WEXITSTATUS(status), process_exit_category());
     }
     else
     {
-        // FIXME: for now we also collapse all non-WIFEXITED exits on
-        // posix into a single "exit 1" error_code. This is enough
-        // for most callers; we can enrich it if anyone really wants
-        // to differentiate various signals that might have killed
-        // the child.
-        return asio::error_code(1, asio::system_category());
+        // Process was terminated by a signal rather than normal exit
+        // Use -1 to indicate signal termination in our custom category
+        return asio::error_code(-1, process_exit_category());
     }
 #endif
 }

src/process/test/ProcessTests.cpp

@@ -75,6 +75,80 @@ TEST_CASE("subprocess fails", "[process]")
     REQUIRE(failed);
 }
 
+TEST_CASE("subprocess error messages", "[process]")
+{
+    VirtualClock clock;
+    Config const& cfg = getTestConfig();
+    Application::pointer app = createTestApplication(clock, cfg);
+
+    SECTION("successful exit")
+    {
+        bool exited = false;
+        asio::error_code capturedEc;
+        auto evt = app->getProcessManager().runProcess("true", "").lock();
+        REQUIRE(evt);
+        evt->async_wait([&](asio::error_code ec) {
+            capturedEc = ec;
+            exited = true;
+        });
+
+        while (!exited && !clock.getIOContext().stopped())
+        {
+            clock.crank(true);
+        }
+
+        // Process should exit with code 0
+        REQUIRE(capturedEc.value() == 0);
+        REQUIRE(capturedEc.message() == "Process exited successfully");
+        REQUIRE(std::string(capturedEc.category().name()) == "process_exit");
+    }
+
+    SECTION("failed exit")
+    {
+        bool exited = false;
+        asio::error_code capturedEc;
+        auto evt = app->getProcessManager().runProcess("false", "").lock();
+        REQUIRE(evt);
+        evt->async_wait([&](asio::error_code ec) {
+            capturedEc = ec;
+            exited = true;
+        });
+
+        while (!exited && !clock.getIOContext().stopped())
+        {
+            clock.crank(true);
+        }
+
+        // Process should exit with non-zero code
+        REQUIRE(capturedEc.value() != 0);
+        REQUIRE(capturedEc.message().find("Process exited with code") != std::string::npos);
+        REQUIRE(std::string(capturedEc.category().name()) == "process_exit");
+    }
+
+    SECTION("custom exit code")
+    {
+        bool exited = false;
+        asio::error_code capturedEc;
+        // Use sh -c "exit 42" to exit with specific code
+        auto evt = app->getProcessManager().runProcess("sh -c 'exit 42'", "").lock();
+        REQUIRE(evt);
+        evt->async_wait([&](asio::error_code ec) {
+            capturedEc = ec;
+            exited = true;
+        });
+
+        while (!exited && !clock.getIOContext().stopped())
+        {
+            clock.crank(true);
+        }
+
+        // Process should exit with code 42
+        REQUIRE(capturedEc.value() == 42);
+        REQUIRE(capturedEc.message() == "Process exited with code 42");
+        REQUIRE(std::string(capturedEc.category().name()) == "process_exit");
+    }
+}
+
 TEST_CASE("subprocess redirect to new file", "[process]")
 {
     VirtualClock clock;