Better deserialization error messages

src/command_line.cc

@@ -238,15 +238,30 @@ void LaunchStdinLoop(Config* config,
   WorkThread::StartThread("stdin", [request_times]() {
     auto* queue = QueueManager::instance();
     while (true) {
-      std::unique_ptr<BaseIpcMessage> message =
+      std::variant<std::string, std::unique_ptr<BaseIpcMessage>> err_or_message =
           MessageRegistry::instance()->ReadMessageFromStdin(
               g_log_stdin_stdout_to_stderr);
 
       // Message parsing can fail if we don't recognize the method.
-      if (!message)
+      auto* err = std::get_if<std::string>(&err_or_message);
+      if (err) {
+        // FIXME LanguageClient-neovim will error without the check, probably
+        // because we do not support didChangeConfiguration and do not fill in
+        // the |id| field.
+        if (err->find("workspace/didChangeConfiguration") ==
+            std::string::npos) {
+          Out_Error out;
+          // TODO We cannot fill in out.id because RequestMessage.id is not a base
+          // field.
+          out.error.code = lsErrorCodes::InvalidParams;
+          out.error.message = std::move(*err);
+          queue->WriteStdout(IpcId::Unknown, out);
+        }
         continue;
+      }
 
       // Cache |method_id| so we can access it after moving |message|.
+      auto& message = std::get<std::unique_ptr<BaseIpcMessage>>(err_or_message);
       IpcId method_id = message->method_id;
 
       (*request_times)[message->method_id] = Timer();

src/language_server_api.cc

@@ -113,8 +113,8 @@ optional<char> ReadCharFromStdinBlocking() {
   return nullopt;
 }
 
-std::unique_ptr<BaseIpcMessage> MessageRegistry::ReadMessageFromStdin(
-    bool log_stdin_to_stderr) {
+std::variant<std::string, std::unique_ptr<BaseIpcMessage>>
+MessageRegistry::ReadMessageFromStdin(bool log_stdin_to_stderr) {
   optional<std::string> content =
       ReadJsonRpcContentFrom(&ReadCharFromStdinBlocking);
   if (!content) {
@@ -138,7 +138,8 @@ std::unique_ptr<BaseIpcMessage> MessageRegistry::ReadMessageFromStdin(
   return Parse(json_reader);
 }
 
-std::unique_ptr<BaseIpcMessage> MessageRegistry::Parse(Reader& visitor) {
+std::variant<std::string, std::unique_ptr<BaseIpcMessage>>
+    MessageRegistry::Parse(Reader& visitor) {
   if (!visitor.HasMember("jsonrpc") ||
       std::string(visitor["jsonrpc"]->GetString()) != "2.0") {
     LOG_S(FATAL) << "Bad or missing jsonrpc version";
@@ -148,19 +149,16 @@ std::unique_ptr<BaseIpcMessage> MessageRegistry::Parse(Reader& visitor) {
   std::string method;
   ReflectMember(visitor, "method", method);
 
-  if (allocators.find(method) == allocators.end()) {
-    LOG_S(ERROR) << "Unable to find registered handler for method \"" << method
-                 << "\"";
-    return nullptr;
-  }
+  if (allocators.find(method) == allocators.end())
+    return std::string("Unable to find registered handler for method '") +
+           method + "'";
 
   Allocator& allocator = allocators[method];
-  // FIXME Print error message for deserialization error
   try {
     return allocator(visitor);
   } catch (std::invalid_argument& e) {
-    LOG_S(ERROR) << "Unable to deserialize request '" << method << "'";
-    return nullptr;
+    return std::string("Unable to deserialize request '") + method + "' " +
+           static_cast<JsonReader&>(visitor).GetPath() + " " + e.what();
   }
 }
 

src/language_server_api.h

@@ -45,9 +45,10 @@ struct MessageRegistry {
       std::function<std::unique_ptr<BaseIpcMessage>(Reader& visitor)>;
   std::unordered_map<std::string, Allocator> allocators;
 
-  std::unique_ptr<BaseIpcMessage> ReadMessageFromStdin(
-      bool log_stdin_to_stderr);
-  std::unique_ptr<BaseIpcMessage> Parse(Reader& visitor);
+  std::variant<std::string, std::unique_ptr<BaseIpcMessage>>
+      ReadMessageFromStdin(bool log_stdin_to_stderr);
+  std::variant<std::string, std::unique_ptr<BaseIpcMessage>> Parse(
+      Reader& visitor);
 };
 
 template <typename T>