From 162f99534464d1174e4d0eaf931db0871051b8be Mon Sep 17 00:00:00 2001
From: Fangrui Song <i@maskray.me>
Date: Fri, 19 Jan 2018 00:14:47 -0800
Subject: [PATCH] Better deserialization error messages

---
 src/command_line.cc        | 19 +++++++++++++++++--
 src/language_server_api.cc | 20 +++++++++-----------
 src/language_server_api.h  |  7 ++++---
 3 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/src/command_line.cc b/src/command_line.cc
index 349eed6e..c7ba782e 100644
--- a/src/command_line.cc
+++ b/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();
diff --git a/src/language_server_api.cc b/src/language_server_api.cc
index 00c88a23..16f1b583 100644
--- a/src/language_server_api.cc
+++ b/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();
   }
 }
 
diff --git a/src/language_server_api.h b/src/language_server_api.h
index b65a4472..cadb8d24 100644
--- a/src/language_server_api.h
+++ b/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>