HackStudio: Propagate error from TerminalWrapper

Use the ErrorOr pattern with the Core::System wrappers to propagate more
errors from the TerminalWrapper.

The run_command method, when called with WaitForExit::Yes now returns an
error on command failure.

Userland/DevTools/HackStudio/HackStudioWidget.cpp

@@ -1341,7 +1341,8 @@ NonnullRefPtr<GUI::Action> HackStudioWidget::create_stop_action()
 {
     auto action = GUI::Action::create("&Stop", Gfx::Bitmap::try_load_from_file("/res/icons/16x16/program-stop.png").release_value_but_fixme_should_propagate_errors(), [this](auto&) {
         if (!Debugger::the().session()) {
-            m_terminal_wrapper->kill_running_command();
+            if (auto result = m_terminal_wrapper->kill_running_command(); result.is_error())
+                warnln("{}", result.error());
             return;
         }
 

Userland/DevTools/HackStudio/ProjectBuilder.cpp

@@ -28,11 +28,12 @@ ErrorOr<void> ProjectBuilder::build(StringView active_file)
         return Error::from_string_literal("no active file"sv);
 
     if (active_file.ends_with(".js")) {
-        m_terminal->run_command(String::formatted("js -A {}", active_file));
+        TRY(m_terminal->run_command(String::formatted("js -A {}", active_file)));
         return {};
     }
+
     if (m_is_serenity == IsSerenityRepo::No) {
-        m_terminal->run_command("make");
+        TRY(m_terminal->run_command("make"));
         return {};
     }
 
@@ -47,12 +48,12 @@ ErrorOr<void> ProjectBuilder::run(StringView active_file)
         return Error::from_string_literal("no active file"sv);
 
     if (active_file.ends_with(".js")) {
-        m_terminal->run_command(String::formatted("js {}", active_file));
+        TRY(m_terminal->run_command(String::formatted("js {}", active_file)));
         return {};
     }
 
     if (m_is_serenity == IsSerenityRepo::No) {
-        m_terminal->run_command("make run");
+        TRY(m_terminal->run_command("make run"));
         return {};
     }
 
@@ -64,7 +65,7 @@ ErrorOr<void> ProjectBuilder::run(StringView active_file)
 ErrorOr<void> ProjectBuilder::run_serenity_component()
 {
     auto relative_path_to_dir = LexicalPath::relative_path(LexicalPath::dirname(m_serenity_component_cmake_file), m_project_root);
-    m_terminal->run_command(LexicalPath::join(relative_path_to_dir, m_serenity_component_name).string(), build_directory());
+    TRY(m_terminal->run_command(LexicalPath::join(relative_path_to_dir, m_serenity_component_name).string(), build_directory()));
     return {};
 }
 
@@ -89,10 +90,8 @@ ErrorOr<void> ProjectBuilder::update_active_file(StringView active_file)
 
 ErrorOr<void> ProjectBuilder::build_serenity_component()
 {
-    m_terminal->run_command(String::formatted("make {}", m_serenity_component_name), build_directory(), TerminalWrapper::WaitForExit::Yes);
-    if (m_terminal->child_exit_status() == 0)
-        return {};
-    return Error::from_string_literal("Make failed"sv);
+    TRY(m_terminal->run_command(String::formatted("make {}", m_serenity_component_name), build_directory(), TerminalWrapper::WaitForExit::Yes, "Make failed"sv));
+    return {};
 }
 
 ErrorOr<String> ProjectBuilder::component_name(StringView cmake_file_path)
@@ -122,14 +121,12 @@ ErrorOr<void> ProjectBuilder::initialize_build_directory()
     auto cmake_file = TRY(Core::File::open(cmake_file_path, Core::OpenMode::WriteOnly));
     cmake_file->write(generate_cmake_file_content());
 
-    m_terminal->run_command(String::formatted("cmake -S {} -DHACKSTUDIO_BUILD=ON -DHACKSTUDIO_BUILD_CMAKE_FILE={}"
-                                              " -DENABLE_UNICODE_DATABASE_DOWNLOAD=OFF",
-                                m_project_root, cmake_file_path),
-        build_directory(), TerminalWrapper::WaitForExit::Yes);
+    TRY(m_terminal->run_command(String::formatted("cmake -S {} -DHACKSTUDIO_BUILD=ON -DHACKSTUDIO_BUILD_CMAKE_FILE={}"
+                                                  " -DENABLE_UNICODE_DATABASE_DOWNLOAD=OFF",
+                                    m_project_root, cmake_file_path),
+        build_directory(), TerminalWrapper::WaitForExit::Yes, "CMake error"sv));
 
-    if (m_terminal->child_exit_status() == 0)
-        return {};
-    return Error::from_string_literal("CMake error"sv);
+    return {};
 }
 
 Optional<String> ProjectBuilder::find_cmake_file_for(StringView file_path) const

Userland/DevTools/HackStudio/TerminalWrapper.cpp

@@ -7,7 +7,7 @@
 
 #include "TerminalWrapper.h"
 #include <AK/String.h>
-#include <LibCore/ConfigFile.h>
+#include <LibCore/System.h>
 #include <LibGUI/Application.h>
 #include <LibGUI/BoxLayout.h>
 #include <LibGUI/MessageBox.h>
@@ -24,52 +24,41 @@
 
 namespace HackStudio {
 
-void TerminalWrapper::run_command(const String& command, Optional<String> working_directory, WaitForExit wait_for_exit)
+ErrorOr<void> TerminalWrapper::run_command(const String& command, Optional<String> working_directory, WaitForExit wait_for_exit, Optional<StringView> failure_message)
 {
     if (m_pid != -1) {
         GUI::MessageBox::show(window(),
             "A command is already running in this TerminalWrapper",
             "Can't run command",
             GUI::MessageBox::Type::Error);
-        return;
+        return {};
     }
 
-    auto ptm_res = setup_master_pseudoterminal();
-    if (ptm_res.is_error()) {
-        perror("setup_master_pseudoterminal");
-        return;
-    }
+    auto ptm_fd = TRY(setup_master_pseudoterminal());
 
-    int ptm_fd = ptm_res.value();
     m_child_exited = false;
     m_child_exit_status.clear();
 
-    m_pid = fork();
-    if (m_pid < 0) {
-        perror("fork");
-        return;
-    }
+    m_pid = TRY(Core::System::fork());
 
     if (m_pid > 0) {
         if (wait_for_exit == WaitForExit::Yes) {
             GUI::Application::the()->event_loop().spin_until([this]() {
                 return m_child_exited;
             });
+
+            VERIFY(m_child_exit_status.has_value());
+            if (m_child_exit_status.value() != 0)
+                return Error::from_string_literal(failure_message.value_or("Command execution failed"sv));
         }
-        return;
+
+        return {};
     }
 
-    if (working_directory.has_value()) {
-        if (chdir(working_directory->characters())) {
-            perror("chdir");
-            exit(1);
-        }
-    }
+    if (working_directory.has_value())
+        TRY(Core::System::chdir(working_directory->view()));
 
-    if (setup_slave_pseudoterminal(ptm_fd).is_error()) {
-        perror("setup_pseudoterminal");
-        exit(1);
-    }
+    TRY(setup_slave_pseudoterminal(ptm_fd));
 
     auto parts = command.split(' ');
     VERIFY(!parts.is_empty());
@@ -77,6 +66,7 @@ void TerminalWrapper::run_command(const String& command, Optional<String> workin
     for (size_t i = 0; i < parts.size(); i++) {
         args[i] = parts[i].characters();
     }
+
     auto rc = execvp(args[0], const_cast<char**>(args));
     if (rc < 0) {
         perror("execve");
@@ -87,29 +77,28 @@ void TerminalWrapper::run_command(const String& command, Optional<String> workin
 
 ErrorOr<int> TerminalWrapper::setup_master_pseudoterminal(WaitForChildOnExit wait_for_child)
 {
-    int ptm_fd = posix_openpt(O_RDWR | O_CLOEXEC);
-    if (ptm_fd < 0) {
-        perror("posix_openpt");
-        VERIFY_NOT_REACHED();
-    }
-    if (grantpt(ptm_fd) < 0) {
-        perror("grantpt");
-        VERIFY_NOT_REACHED();
-    }
-    if (unlockpt(ptm_fd) < 0) {
-        perror("unlockpt");
-        VERIFY_NOT_REACHED();
-    }
+    int ptm_fd = TRY(Core::System::posix_openpt(O_RDWR | O_CLOEXEC));
+    bool error_happened = true;
+
+    ScopeGuard close_ptm { [&]() {
+        if (error_happened) {
+            if (auto result = Core::System::close(ptm_fd); result.is_error())
+                warnln("{}", result.release_error());
+        }
+    } };
+
+    TRY(Core::System::grantpt(ptm_fd));
+    TRY(Core::System::unlockpt(ptm_fd));
 
     m_terminal_widget->set_pty_master_fd(ptm_fd);
     m_terminal_widget->on_command_exit = [this, wait_for_child] {
         if (wait_for_child == WaitForChildOnExit::Yes) {
-            int wstatus;
-            int rc = waitpid(m_pid, &wstatus, 0);
-            if (rc < 0) {
-                perror("waitpid");
+            auto result = Core::System::waitpid(m_pid, 0);
+            if (result.is_error()) {
+                warnln("{}", result.error());
                 VERIFY_NOT_REACHED();
             }
+            int wstatus = result.release_value().status;
 
             if (WIFEXITED(wstatus)) {
                 m_terminal_widget->inject_string(String::formatted("\033[{};1m(Command exited with code {})\033[0m\r\n", wstatus == 0 ? 32 : 31, WEXITSTATUS(wstatus)));
@@ -130,6 +119,8 @@ ErrorOr<int> TerminalWrapper::setup_master_pseudoterminal(WaitForChildOnExit wai
 
     terminal().scroll_to_bottom();
 
+    error_happened = false;
+
     return ptm_fd;
 }
 
@@ -137,55 +128,41 @@ ErrorOr<void> TerminalWrapper::setup_slave_pseudoterminal(int master_fd)
 {
     setsid();
 
-    const char* tty_name = ptsname(master_fd);
-    if (!tty_name)
-        return Error::from_errno(errno);
+    auto tty_name = TRY(Core::System::ptsname(master_fd));
 
     close(master_fd);
-    int pts_fd = open(tty_name, O_RDWR);
-    if (pts_fd < 0)
-        return Error::from_errno(errno);
+
+    int pts_fd = TRY(Core::System::open(tty_name, O_RDWR));
 
     tcsetpgrp(pts_fd, getpid());
 
     // NOTE: It's okay if this fails.
-    int rc = ioctl(0, TIOCNOTTY);
+    ioctl(0, TIOCNOTTY);
 
     close(0);
     close(1);
     close(2);
 
-    rc = dup2(pts_fd, 0);
-    if (rc < 0)
-        return Error::from_errno(errno);
+    TRY(Core::System::dup2(pts_fd, 0));
+    TRY(Core::System::dup2(pts_fd, 1));
+    TRY(Core::System::dup2(pts_fd, 2));
 
-    rc = dup2(pts_fd, 1);
-    if (rc < 0)
-        return Error::from_errno(errno);
+    TRY(Core::System::close(pts_fd));
 
-    rc = dup2(pts_fd, 2);
-    if (rc < 0)
-        return Error::from_errno(errno);
-
-    rc = close(pts_fd);
-    if (rc < 0)
-        return Error::from_errno(errno);
-
-    rc = ioctl(0, TIOCSCTTY);
-    if (rc < 0)
-        return Error::from_errno(errno);
+    TRY(Core::System::ioctl(0, TIOCSCTTY));
 
     setenv("TERM", "xterm", true);
 
     return {};
 }
 
-void TerminalWrapper::kill_running_command()
+ErrorOr<void> TerminalWrapper::kill_running_command()
 {
     VERIFY(m_pid != -1);
 
     // Kill our child process and its whole process group.
-    [[maybe_unused]] auto rc = killpg(m_pid, SIGTERM);
+    TRY(Core::System::killpg(m_pid, SIGTERM));
+    return {};
 }
 
 void TerminalWrapper::clear_including_history()
@@ -199,9 +176,11 @@ TerminalWrapper::TerminalWrapper(bool user_spawned)
     set_layout<GUI::VerticalBoxLayout>();
 
     m_terminal_widget = add<VT::TerminalWidget>(-1, false);
-
-    if (user_spawned)
-        run_command("Shell");
+    if (user_spawned) {
+        auto maybe_error = run_command("Shell");
+        if (maybe_error.is_error())
+            warnln("{}", maybe_error.release_error());
+    }
 }
 
 int TerminalWrapper::child_exit_status() const

Userland/DevTools/HackStudio/TerminalWrapper.h

@@ -21,8 +21,8 @@ public:
         No,
         Yes
     };
-    void run_command(const String&, Optional<String> working_directory = {}, WaitForExit = WaitForExit::No);
-    void kill_running_command();
+    ErrorOr<void> run_command(const String&, Optional<String> working_directory = {}, WaitForExit = WaitForExit::No, Optional<StringView> failure_message = {});
+    ErrorOr<void> kill_running_command();
     void clear_including_history();
 
     bool user_spawned() const { return m_user_spawned; }