[lldb-dap] Improve error reporting for dap command arguments. #135684
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…arse correctly. Previously the error only contained the failed to parse JSON message, which has no additional context. This improves the error messages and improves the consistency of handling properties in protocol structures. Updating the fields to use 'ObjectMapper.map' instead of 'ObjectMapper.mapOptional' caught that adapterID was misspelled as well.
|
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesPreviously the error only contained the failed to parse JSON message, which has no additional context. This improves the error messages and improves the consistency of handling properties in protocol structures. Updating the fields to use 'ObjectMapper.map' instead of 'ObjectMapper.mapOptional' caught that adapterID was misspelled as well. For example, previously: Worked without an error but now it reports: Full diff: https://github.com/llvm/llvm-project/pull/135684.diff 5 Files Affected:
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index 50795f8252de3..488628b224f53 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -124,6 +124,7 @@ class RequestHandler : public BaseRequestHandler {
if (request.arguments && !fromJSON(request.arguments, arguments, root)) {
std::string parse_failure;
llvm::raw_string_ostream OS(parse_failure);
+ OS << "invalid arguments for request '" << request.command << "': ";
root.printErrorContext(request.arguments, OS);
protocol::ErrorMessage error_message;
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
index af63cc803e545..bfd68448fb483 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp
@@ -178,7 +178,7 @@ bool fromJSON(json::Value const &Params, Response &R, json::Path P) {
return false;
}
- return O.map("success", R.success) && O.mapOptional("message", R.message) &&
+ return O.map("success", R.success) && O.map("message", R.message) &&
mapRaw(Params, "body", R.body, P);
}
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index 7163399899f7e..3523f8ac87ec9 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -20,16 +20,16 @@ namespace lldb_dap::protocol {
bool fromJSON(const llvm::json::Value &Params, CancelArguments &CA,
llvm::json::Path P) {
llvm::json::ObjectMapper O(Params, P);
- return O && O.mapOptional("requestId", CA.requestId) &&
- O.mapOptional("progressId", CA.progressId);
+ return O && O.map("requestId", CA.requestId) &&
+ O.map("progressId", CA.progressId);
}
bool fromJSON(const json::Value &Params, DisconnectArguments &DA,
json::Path P) {
json::ObjectMapper O(Params, P);
- return O && O.mapOptional("restart", DA.restart) &&
- O.mapOptional("terminateDebuggee", DA.terminateDebuggee) &&
- O.mapOptional("suspendDebuggee", DA.suspendDebuggee);
+ return O && O.map("restart", DA.restart) &&
+ O.map("terminateDebuggee", DA.terminateDebuggee) &&
+ O.map("suspendDebuggee", DA.suspendDebuggee);
}
bool fromJSON(const llvm::json::Value &Params, PathFormat &PF,
@@ -75,23 +75,33 @@ bool fromJSON(const llvm::json::Value &Params, InitializeRequestArguments &IRA,
const json::Object *O = Params.getAsObject();
- for (auto &kv : ClientFeatureByKey)
- if (std::optional<bool> v = O->getBoolean(kv.first()); v && *v)
+ for (auto &kv : ClientFeatureByKey) {
+ const json::Value *value_ref = O->get(kv.first());
+ if (!value_ref)
+ continue;
+
+ const std::optional<bool> value = value_ref->getAsBoolean();
+ if (!value) {
+ P.field(kv.first()).report("expected bool");
+ return false;
+ }
+
+ if (*value)
IRA.supportedFeatures.insert(kv.second);
+ }
- return OM.mapOptional("adatperID", IRA.adatperID) &&
- OM.mapOptional("clientID", IRA.clientID) &&
- OM.mapOptional("clientName", IRA.clientName) &&
- OM.mapOptional("locale", IRA.locale) &&
- OM.mapOptional("linesStartAt1", IRA.linesStartAt1) &&
- OM.mapOptional("columnsStartAt1", IRA.columnsStartAt1) &&
- OM.mapOptional("pathFormat", IRA.pathFormat) &&
- OM.mapOptional("$__lldb_sourceInitFile", IRA.lldbExtSourceInitFile);
+ return OM.map("adapterID", IRA.adapterID) &&
+ OM.map("clientID", IRA.clientID) &&
+ OM.map("clientName", IRA.clientName) && OM.map("locale", IRA.locale) &&
+ OM.map("linesStartAt1", IRA.linesStartAt1) &&
+ OM.map("columnsStartAt1", IRA.columnsStartAt1) &&
+ OM.map("pathFormat", IRA.pathFormat) &&
+ OM.map("$__lldb_sourceInitFile", IRA.lldbExtSourceInitFile);
}
bool fromJSON(const json::Value &Params, SourceArguments &SA, json::Path P) {
json::ObjectMapper O(Params, P);
- return O && O.mapOptional("source", SA.source) &&
+ return O && O.map("source", SA.source) &&
O.map("sourceReference", SA.sourceReference);
}
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index 22d400fd494a5..6623dfa0db05c 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -100,7 +100,7 @@ enum PathFormat : unsigned { ePatFormatPath, ePathFormatURI };
/// Arguments for `initialize` request.
struct InitializeRequestArguments {
/// The ID of the debug adapter.
- std::string adatperID;
+ std::string adapterID;
/// The ID of the client using this adapter.
std::optional<std::string> clientID;
@@ -113,7 +113,7 @@ struct InitializeRequestArguments {
/// Determines in what format paths are specified. The default is `path`,
/// which is the native format.
- std::optional<PathFormat> pathFormat = ePatFormatPath;
+ PathFormat pathFormat = ePatFormatPath;
/// If true all line numbers are 1-based (default).
std::optional<bool> linesStartAt1;
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
index f4f0bf8dcea84..4d1e90215bbb4 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
@@ -38,9 +38,9 @@ bool fromJSON(const json::Value &Params, PresentationHint &PH, json::Path P) {
bool fromJSON(const json::Value &Params, Source &S, json::Path P) {
json::ObjectMapper O(Params, P);
- return O && O.mapOptional("name", S.name) && O.mapOptional("path", S.path) &&
- O.mapOptional("presentationHint", S.presentationHint) &&
- O.mapOptional("sourceReference", S.sourceReference);
+ return O && O.map("name", S.name) && O.map("path", S.path) &&
+ O.map("presentationHint", S.presentationHint) &&
+ O.map("sourceReference", S.sourceReference);
}
json::Value toJSON(const ExceptionBreakpointsFilter &EBF) {
|
JDevlieghere
approved these changes
Apr 15, 2025
ashgti
changed the title
ashgti
changed the title
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
…35684) Previously the error only contained the failed to parse JSON message, which has no additional context. This improves the error messages and improves the consistency of handling properties in protocol structures. Updating the fields to use 'ObjectMapper.map' instead of 'ObjectMapper.mapOptional' caught that adapterID was misspelled as well. For example, previously: ``` $ echo 'Content-Length: 81\r\n\r\n{"type":"request","command":"initialize","seq":1,"arguments":{"adapterID":12345}} | lldb-dap ``` Worked without an error but now it reports: ``` invalid arguments for request 'initialize': expected string at arguments.adapterID { "adapterID": /* error: expected string */ 12345 } ```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously the error only contained the failed to parse JSON message, which has no additional context.
This improves the error messages and improves the consistency of handling properties in protocol structures. Updating the fields to use 'ObjectMapper.map' instead of 'ObjectMapper.mapOptional' caught that adapterID was misspelled as well.
For example, previously:
Worked without an error but now it reports: