[Lldb-commits] [lldb] [lldb] Reply to malformed requests and use JSON-RPC error codes (PR #212678)
via lldb-commits
lldb-commits at lists.llvm.org
Tue Jul 28 21:32:49 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
<details>
<summary>Changes</summary>
The transport dropped a message that failed to parse: it logged the error and returned, so a peer waiting on that request hung forever and every message already buffered behind it was discarded. Keep going after a failed parse, and add ReplyWithParseError so a message that is valid JSON but not a valid request is answered against its own id. Unparseable JSON still gets no reply, since it carries no id and JSON-RPC forbids inventing one.
Method-not-found was raised with createStringError, which converts through inconvertibleErrorCode() to -32603, so clients could not tell a misspelled method from a server fault. Use the MethodNotFound error, which already carries -32601, and give InvalidParams the -32602 code it was missing.
---
Full diff: https://github.com/llvm/llvm-project/pull/212678.diff
5 Files Affected:
- (modified) lldb/include/lldb/Host/JSONTransport.h (+41-4)
- (modified) lldb/include/lldb/Protocol/MCP/Transport.h (+3)
- (modified) lldb/source/Host/common/JSONTransport.cpp (+13-1)
- (modified) lldb/source/Protocol/MCP/Transport.cpp (+29)
- (modified) lldb/unittests/Host/JSONTransportTest.cpp (+21)
``````````diff
diff --git a/lldb/include/lldb/Host/JSONTransport.h b/lldb/include/lldb/Host/JSONTransport.h
index d3c6e8e101ad5..a6170303e13f0 100644
--- a/lldb/include/lldb/Host/JSONTransport.h
+++ b/lldb/include/lldb/Host/JSONTransport.h
@@ -68,6 +68,8 @@ class InvalidParams : public llvm::ErrorInfo<InvalidParams> {
public:
static char ID;
+ static constexpr int kErrorCode = -32602;
+
explicit InvalidParams(std::string method, std::string context)
: m_method(std::move(method)), m_context(std::move(context)) {}
@@ -83,6 +85,25 @@ class InvalidParams : public llvm::ErrorInfo<InvalidParams> {
std::string m_context;
};
+/// An error to indicate that an incoming message could not be parsed as a
+/// valid protocol message.
+class InvalidMessage : public llvm::ErrorInfo<InvalidMessage> {
+public:
+ static char ID;
+
+ static constexpr int kErrorCode = -32700;
+
+ explicit InvalidMessage(std::string raw_message, std::string reason)
+ : m_raw_message(std::move(raw_message)), m_reason(std::move(reason)) {}
+
+ void log(llvm::raw_ostream &OS) const override;
+ std::error_code convertToErrorCode() const override;
+
+private:
+ std::string m_raw_message;
+ std::string m_reason;
+};
+
/// An error to indicate that no handler was registered for a given method.
class MethodNotFound : public llvm::ErrorInfo<MethodNotFound> {
public:
@@ -139,6 +160,14 @@ class JSONTransport {
/// Sends a response to a specific request.
virtual llvm::Error Send(const Resp &) = 0;
+ /// Sends an error response for a message that failed to parse, described by
+ /// \p reason. Sends nothing if no request id can be recovered from it, since
+ /// there is then no request to respond to.
+ virtual llvm::Error ReplyWithParseError(llvm::StringRef raw_message,
+ llvm::StringRef reason) {
+ return llvm::Error::success();
+ }
+
/// Implemented to handle incoming messages. (See `RegisterMessageHandler()`
/// below).
class MessageHandler {
@@ -245,8 +274,15 @@ template <typename Proto> class IOTransport : public JSONTransport<Proto> {
llvm::Expected<Message> message =
llvm::json::parse<Message>(raw_message);
if (!message) {
- handler.OnError(message.takeError());
- return;
+ // Messages are independent, so one that fails to parse must not
+ // discard those already buffered behind it.
+ std::string reason = llvm::toString(message.takeError());
+ if (llvm::Error error =
+ this->ReplyWithParseError(raw_message, reason))
+ handler.OnError(std::move(error));
+ handler.OnError(
+ llvm::make_error<InvalidMessage>(raw_message, std::move(reason)));
+ continue;
}
std::visit([&handler](auto &&msg) { handler.Received(msg); }, *message);
@@ -546,7 +582,7 @@ class Binder : public JSONTransport<Proto>::MessageHandler {
auto it = m_event_handlers.find(Proto::KeyFor(evt));
if (it == m_event_handlers.end()) {
OnError(llvm::createStringError(
- llvm::formatv("no handled for event {0}", toJSON(evt))));
+ llvm::formatv("no handler for event {0}", toJSON(evt))));
return;
}
it->second(evt);
@@ -558,7 +594,8 @@ class Binder : public JSONTransport<Proto>::MessageHandler {
std::scoped_lock<std::recursive_mutex> guard(m_mutex);
auto it = m_request_handlers.find(Proto::KeyFor(req));
if (it == m_request_handlers.end()) {
- reply(Proto::Make(req, llvm::createStringError("method not found")));
+ reply(Proto::Make(req,
+ llvm::make_error<MethodNotFound>(Proto::KeyFor(req))));
return;
}
diff --git a/lldb/include/lldb/Protocol/MCP/Transport.h b/lldb/include/lldb/Protocol/MCP/Transport.h
index ceadf1dbd82b8..4e55b7939cfe6 100644
--- a/lldb/include/lldb/Protocol/MCP/Transport.h
+++ b/lldb/include/lldb/Protocol/MCP/Transport.h
@@ -95,6 +95,9 @@ class Transport final
void Log(llvm::StringRef message) override;
+ llvm::Error ReplyWithParseError(llvm::StringRef raw_message,
+ llvm::StringRef reason) override;
+
private:
LogCallback m_log_callback;
};
diff --git a/lldb/source/Host/common/JSONTransport.cpp b/lldb/source/Host/common/JSONTransport.cpp
index 22de7fa8cbead..17af066dfa80d 100644
--- a/lldb/source/Host/common/JSONTransport.cpp
+++ b/lldb/source/Host/common/JSONTransport.cpp
@@ -37,7 +37,19 @@ void InvalidParams::log(raw_ostream &OS) const {
<< "'";
}
std::error_code InvalidParams::convertToErrorCode() const {
- return std::make_error_code(std::errc::invalid_argument);
+ // JSON-RPC Invalid params
+ return std::error_code(InvalidParams::kErrorCode, std::generic_category());
+}
+
+char InvalidMessage::ID;
+
+void InvalidMessage::log(raw_ostream &OS) const {
+ OS << "invalid message '" << m_raw_message << "': '" << m_reason << "'";
+}
+
+std::error_code InvalidMessage::convertToErrorCode() const {
+ // JSON-RPC Parse error
+ return std::error_code(InvalidMessage::kErrorCode, std::generic_category());
}
char MethodNotFound::ID;
diff --git a/lldb/source/Protocol/MCP/Transport.cpp b/lldb/source/Protocol/MCP/Transport.cpp
index 1dc01a9f59008..47a3ff2b6f77c 100644
--- a/lldb/source/Protocol/MCP/Transport.cpp
+++ b/lldb/source/Protocol/MCP/Transport.cpp
@@ -22,3 +22,32 @@ void Transport::Log(StringRef message) {
if (m_log_callback)
m_log_callback(message);
}
+
+llvm::Error Transport::ReplyWithParseError(StringRef raw_message,
+ StringRef reason) {
+ llvm::Expected<json::Value> value = json::parse(raw_message);
+ if (!value) {
+ // JSON-RPC forbids guessing an id, and malformed JSON carries none.
+ consumeError(value.takeError());
+ return llvm::Error::success();
+ }
+
+ const json::Object *object = value->getAsObject();
+ if (!object)
+ return llvm::Error::success();
+
+ // A message without an id is a notification, which takes no response.
+ const json::Value *raw_id = object->get("id");
+ if (!raw_id)
+ return llvm::Error::success();
+
+ Id id;
+ if (std::optional<StringRef> str = raw_id->getAsString())
+ id = str->str();
+ else if (std::optional<int64_t> num = raw_id->getAsInteger())
+ id = *num;
+ else
+ return llvm::Error::success();
+
+ return Send(Response{id, mcp::Error{eErrorCodeInvalidRequest, reason.str()}});
+}
diff --git a/lldb/unittests/Host/JSONTransportTest.cpp b/lldb/unittests/Host/JSONTransportTest.cpp
index c0adbd855cec8..d54576b9af900 100644
--- a/lldb/unittests/Host/JSONTransportTest.cpp
+++ b/lldb/unittests/Host/JSONTransportTest.cpp
@@ -546,6 +546,19 @@ TEST_F(JSONRPCTransportTest, MalformedRequests) {
ASSERT_THAT_ERROR(Run(), Succeeded());
}
+TEST_F(JSONRPCTransportTest, MalformedRequestDoesNotDropLaterMessages) {
+ InSequence seq;
+ std::string messages =
+ "notjson\n" + Encode(Message{Request{1, "foo", std::nullopt}});
+ ASSERT_THAT_EXPECTED(input.Write(messages.data(), messages.size()),
+ Succeeded());
+ EXPECT_CALL(message_handler, OnError(_)).WillOnce([](llvm::Error err) {
+ consumeError(std::move(err));
+ });
+ EXPECT_CALL(message_handler, Received(Request{1, "foo", std::nullopt}));
+ ASSERT_THAT_ERROR(Run(), Succeeded());
+}
+
TEST_F(JSONRPCTransportTest, Read) {
Write(Message{Request{1, "foo", std::nullopt}});
EXPECT_CALL(message_handler, Received(Request{1, "foo", std::nullopt}));
@@ -831,6 +844,14 @@ TEST_F(TransportBinderTest, InBoundAsyncRequestsError) {
Run();
}
+TEST_F(TransportBinderTest, InBoundRequestUnknownMethod) {
+ EXPECT_THAT_ERROR(from_remote->Send(Request{4, "nosuch", MyFnParams{1, 2}}),
+ Succeeded());
+ EXPECT_CALL(remote, Received(Response{4, MethodNotFound::kErrorCode,
+ "method not found: 'nosuch'"}));
+ Run();
+}
+
TEST_F(TransportBinderTest, FailPendingRequests) {
OutgoingRequest<MyFnResult, MyFnParams> addFn =
binder->Bind<MyFnResult, MyFnParams>("add");
``````````
</details>
https://github.com/llvm/llvm-project/pull/212678
More information about the lldb-commits
mailing list