LibChess+ChessEngine: Don't crash on error when reading UCI commands

ChessEngine and the chess GUI will no longer crash on error while
reading UCI commands. ChessEngine will print a message to the standard
output, while the GUI will ignore unknown commands. Both will print
the error to the debug log if the UCI_DEBUG flag is enabled.

Trailing and preceding whitespace is now stripped from commands before
they are parsed. Commands which are just whitespace no longer produce
errors.

Userland/Libraries/LibChess/UCIEndpoint.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2020, the SerenityOS developers.
+ * Copyright (c) 2023, Tim Ledbetter <timledbetter@gmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -80,49 +81,60 @@ void Endpoint::set_in_notifier()
             return;
         }
 
-        while (m_in->can_read_line())
+        while (m_in->can_read_line()) {
-            Core::EventLoop::current().post_event(*this, read_command());
+            auto line = m_in->read_line(4096).trim_whitespace();
+            if (line.is_empty())
+                continue;
+
+            auto maybe_command = read_command(line);
+            if (maybe_command.is_error()) {
+                dbgln_if(UCI_DEBUG, "{} Error while parsing UCI command: {}, error: {}", class_name(), maybe_command.error(), line);
+                if (on_command_read_error)
+                    on_command_read_error(move(line), maybe_command.release_error());
+
+                continue;
+            }
+
+            Core::EventLoop::current().post_event(*this, maybe_command.release_value());
+        }
     };
 }
 
-NonnullOwnPtr<Command> Endpoint::read_command()
+ErrorOr<NonnullOwnPtr<Command>> Endpoint::read_command(StringView line) const
 {
-    DeprecatedString line(ReadonlyBytes(m_in->read_line(4096).bytes()), Chomp);
-
     dbgln_if(UCI_DEBUG, "{} Received UCI Command: {}", class_name(), line);
 
     if (line == "uci") {
-        return UCICommand::from_string(line).release_value_but_fixme_should_propagate_errors();
+        return UCICommand::from_string(line);
     } else if (line.starts_with("debug"sv)) {
-        return DebugCommand::from_string(line).release_value_but_fixme_should_propagate_errors();
+        return DebugCommand::from_string(line);
     } else if (line.starts_with("isready"sv)) {
-        return IsReadyCommand::from_string(line).release_value_but_fixme_should_propagate_errors();
+        return IsReadyCommand::from_string(line);
     } else if (line.starts_with("setoption"sv)) {
-        return SetOptionCommand::from_string(line).release_value_but_fixme_should_propagate_errors();
+        return SetOptionCommand::from_string(line);
     } else if (line.starts_with("position"sv)) {
-        return PositionCommand::from_string(line).release_value_but_fixme_should_propagate_errors();
+        return PositionCommand::from_string(line);
     } else if (line.starts_with("go"sv)) {
-        return GoCommand::from_string(line).release_value_but_fixme_should_propagate_errors();
+        return GoCommand::from_string(line);
     } else if (line.starts_with("stop"sv)) {
-        return StopCommand::from_string(line).release_value_but_fixme_should_propagate_errors();
+        return StopCommand::from_string(line);
     } else if (line.starts_with("id"sv)) {
-        return IdCommand::from_string(line).release_value_but_fixme_should_propagate_errors();
+        return IdCommand::from_string(line);
     } else if (line.starts_with("uciok"sv)) {
-        return UCIOkCommand::from_string(line).release_value_but_fixme_should_propagate_errors();
+        return UCIOkCommand::from_string(line);
     } else if (line.starts_with("readyok"sv)) {
-        return ReadyOkCommand::from_string(line).release_value_but_fixme_should_propagate_errors();
+        return ReadyOkCommand::from_string(line);
     } else if (line.starts_with("bestmove"sv)) {
-        return BestMoveCommand::from_string(line).release_value_but_fixme_should_propagate_errors();
+        return BestMoveCommand::from_string(line);
     } else if (line.starts_with("info"sv)) {
-        return InfoCommand::from_string(line).release_value_but_fixme_should_propagate_errors();
+        return InfoCommand::from_string(line);
     } else if (line.starts_with("quit"sv)) {
-        return QuitCommand::from_string(line).release_value_but_fixme_should_propagate_errors();
+        return QuitCommand::from_string(line);
     } else if (line.starts_with("ucinewgame"sv)) {
-        return UCINewGameCommand::from_string(line).release_value_but_fixme_should_propagate_errors();
+        return UCINewGameCommand::from_string(line);
     }
 
-    dbgln("command line: {}", line);
+    return Error::from_string_literal("Unknown command");
-    VERIFY_NOT_REACHED();
 }
 
 };

Userland/Libraries/LibChess/UCIEndpoint.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2020-2022, the SerenityOS developers.
+ * Copyright (c) 2023, Tim Ledbetter <timledbetter@gmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -18,6 +19,8 @@ class Endpoint : public Core::Object {
 public:
     virtual ~Endpoint() override = default;
 
+    Function<void(DeprecatedString, Error)> on_command_read_error;
+
     virtual void handle_uci() { }
     virtual void handle_debug(DebugCommand const&) { }
     virtual void handle_isready() { }
@@ -58,7 +61,7 @@ private:
         UnexpectedEof
     };
     void set_in_notifier();
-    NonnullOwnPtr<Command> read_command();
+    ErrorOr<NonnullOwnPtr<Command>> read_command(StringView line) const;
 
     RefPtr<Core::IODevice> m_in;
     RefPtr<Core::IODevice> m_out;

Userland/Services/ChessEngine/ChessEngine.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2020, the SerenityOS developers.
+ * Copyright (c) 2023, Tim Ledbetter <timledbetter@gmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -25,10 +26,12 @@ public:
     Function<void(int)> on_quit;
 
 private:
-    ChessEngine() = default;
     ChessEngine(NonnullRefPtr<Core::IODevice> in, NonnullRefPtr<Core::IODevice> out)
         : Endpoint(in, out)
     {
+        on_command_read_error = [](auto command, auto error) {
+            outln("{}: '{}'", error, command);
+        };
     }
 
     Chess::Board m_board;