Skip to content

Commit 14cb656

Browse files
authored
[lldb-dap] Improve error reporting for dap command arguments. (#135684)
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 } ```
1 parent 823adc7 commit 14cb656

File tree

5 files changed

+37
-25
lines changed

5 files changed

+37
-25
lines changed

lldb/tools/lldb-dap/Handler/RequestHandler.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,13 @@ class RequestHandler : public BaseRequestHandler {
120120
}
121121

122122
Args arguments;
123-
llvm::json::Path::Root root;
124-
if (request.arguments && !fromJSON(request.arguments, arguments, root)) {
123+
llvm::json::Path::Root root("arguments");
124+
if (request.arguments && !fromJSON(*request.arguments, arguments, root)) {
125125
std::string parse_failure;
126126
llvm::raw_string_ostream OS(parse_failure);
127-
root.printErrorContext(request.arguments, OS);
127+
OS << "invalid arguments for request '" << request.command
128+
<< "': " << llvm::toString(root.getError()) << "\n";
129+
root.printErrorContext(*request.arguments, OS);
128130

129131
protocol::ErrorMessage error_message;
130132
error_message.format = parse_failure;

lldb/tools/lldb-dap/Protocol/ProtocolBase.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ bool fromJSON(json::Value const &Params, Response &R, json::Path P) {
178178
return false;
179179
}
180180

181-
return O.map("success", R.success) && O.mapOptional("message", R.message) &&
181+
return O.map("success", R.success) && O.map("message", R.message) &&
182182
mapRaw(Params, "body", R.body, P);
183183
}
184184

lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,16 @@ namespace lldb_dap::protocol {
2020
bool fromJSON(const llvm::json::Value &Params, CancelArguments &CA,
2121
llvm::json::Path P) {
2222
llvm::json::ObjectMapper O(Params, P);
23-
return O && O.mapOptional("requestId", CA.requestId) &&
24-
O.mapOptional("progressId", CA.progressId);
23+
return O && O.map("requestId", CA.requestId) &&
24+
O.map("progressId", CA.progressId);
2525
}
2626

2727
bool fromJSON(const json::Value &Params, DisconnectArguments &DA,
2828
json::Path P) {
2929
json::ObjectMapper O(Params, P);
30-
return O && O.mapOptional("restart", DA.restart) &&
31-
O.mapOptional("terminateDebuggee", DA.terminateDebuggee) &&
32-
O.mapOptional("suspendDebuggee", DA.suspendDebuggee);
30+
return O && O.map("restart", DA.restart) &&
31+
O.map("terminateDebuggee", DA.terminateDebuggee) &&
32+
O.map("suspendDebuggee", DA.suspendDebuggee);
3333
}
3434

3535
bool fromJSON(const llvm::json::Value &Params, PathFormat &PF,
@@ -75,23 +75,33 @@ bool fromJSON(const llvm::json::Value &Params, InitializeRequestArguments &IRA,
7575

7676
const json::Object *O = Params.getAsObject();
7777

78-
for (auto &kv : ClientFeatureByKey)
79-
if (std::optional<bool> v = O->getBoolean(kv.first()); v && *v)
78+
for (auto &kv : ClientFeatureByKey) {
79+
const json::Value *value_ref = O->get(kv.first());
80+
if (!value_ref)
81+
continue;
82+
83+
const std::optional<bool> value = value_ref->getAsBoolean();
84+
if (!value) {
85+
P.field(kv.first()).report("expected bool");
86+
return false;
87+
}
88+
89+
if (*value)
8090
IRA.supportedFeatures.insert(kv.second);
91+
}
8192

82-
return OM.mapOptional("adatperID", IRA.adatperID) &&
83-
OM.mapOptional("clientID", IRA.clientID) &&
84-
OM.mapOptional("clientName", IRA.clientName) &&
85-
OM.mapOptional("locale", IRA.locale) &&
86-
OM.mapOptional("linesStartAt1", IRA.linesStartAt1) &&
87-
OM.mapOptional("columnsStartAt1", IRA.columnsStartAt1) &&
88-
OM.mapOptional("pathFormat", IRA.pathFormat) &&
89-
OM.mapOptional("$__lldb_sourceInitFile", IRA.lldbExtSourceInitFile);
93+
return OM.map("adapterID", IRA.adapterID) &&
94+
OM.map("clientID", IRA.clientID) &&
95+
OM.map("clientName", IRA.clientName) && OM.map("locale", IRA.locale) &&
96+
OM.map("linesStartAt1", IRA.linesStartAt1) &&
97+
OM.map("columnsStartAt1", IRA.columnsStartAt1) &&
98+
OM.map("pathFormat", IRA.pathFormat) &&
99+
OM.map("$__lldb_sourceInitFile", IRA.lldbExtSourceInitFile);
90100
}
91101

92102
bool fromJSON(const json::Value &Params, SourceArguments &SA, json::Path P) {
93103
json::ObjectMapper O(Params, P);
94-
return O && O.mapOptional("source", SA.source) &&
104+
return O && O.map("source", SA.source) &&
95105
O.map("sourceReference", SA.sourceReference);
96106
}
97107

lldb/tools/lldb-dap/Protocol/ProtocolRequests.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ enum PathFormat : unsigned { ePatFormatPath, ePathFormatURI };
100100
/// Arguments for `initialize` request.
101101
struct InitializeRequestArguments {
102102
/// The ID of the debug adapter.
103-
std::string adatperID;
103+
std::string adapterID;
104104

105105
/// The ID of the client using this adapter.
106106
std::optional<std::string> clientID;
@@ -113,7 +113,7 @@ struct InitializeRequestArguments {
113113

114114
/// Determines in what format paths are specified. The default is `path`,
115115
/// which is the native format.
116-
std::optional<PathFormat> pathFormat = ePatFormatPath;
116+
PathFormat pathFormat = ePatFormatPath;
117117

118118
/// If true all line numbers are 1-based (default).
119119
std::optional<bool> linesStartAt1;

lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ bool fromJSON(const json::Value &Params, PresentationHint &PH, json::Path P) {
3838

3939
bool fromJSON(const json::Value &Params, Source &S, json::Path P) {
4040
json::ObjectMapper O(Params, P);
41-
return O && O.mapOptional("name", S.name) && O.mapOptional("path", S.path) &&
42-
O.mapOptional("presentationHint", S.presentationHint) &&
43-
O.mapOptional("sourceReference", S.sourceReference);
41+
return O && O.map("name", S.name) && O.map("path", S.path) &&
42+
O.map("presentationHint", S.presentationHint) &&
43+
O.map("sourceReference", S.sourceReference);
4444
}
4545

4646
json::Value toJSON(const ExceptionBreakpointsFilter &EBF) {

0 commit comments

Comments
 (0)