Skip to content

Commit

Permalink
[JSON] Add error reporting to fromJSON and ObjectMapper
Browse files Browse the repository at this point in the history
Translating between JSON objects and C++ strutctures is common.
From experience in clangd, fromJSON/ObjectMapper work well and save a lot of
code, but aren't adopted elsewhere at least partly due to total lack of error
reporting beyond "ok"/"bad".

The recently-added error model should be rich enough for most applications.
It requires tracking the path within the root object and reporting local
errors at appropriate places.
To do this, we exploit the fact that the call graph of recursive
parse functions mirror the structure of the JSON itself.
The current path is represented as a linked list of segments, each of which is
on the stack as a parameter. Concretely, fromJSON now looks like:
  bool fromJSON(const Value&, T&, Path);

Beyond the signature change, this is reasonably unobtrusive: building
the path segments is mostly handled by ObjectMapper and the vector<T> fromJSON.
However the root caller of fromJSON must now create a Root object to
store the errors, which is a little clunky.

I've added high-level parse<T>(StringRef) -> Expected<T>, but it's not
general enough to be the primary interface I think (at least, not usable in
clangd).

All existing users (mostly just clangd) are updated in this patch,
making this change backwards-compatible is a bit hairy.

Differential Revision: https://reviews.llvm.org/D88103
  • Loading branch information
sam-mccall committed Sep 23, 2020
1 parent 881aba7 commit fa69b60
Show file tree
Hide file tree
Showing 11 changed files with 440 additions and 255 deletions.
22 changes: 8 additions & 14 deletions clang-tools-extra/clangd/ClangdLSPServer.cpp
Expand Up @@ -247,14 +247,10 @@ class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
void (ClangdLSPServer::*Handler)(const Param &, Callback<Result>)) {
Calls[Method] = [Method, Handler, this](llvm::json::Value RawParams,
ReplyOnce Reply) {
Param P;
if (fromJSON(RawParams, P)) {
(Server.*Handler)(P, std::move(Reply));
} else {
elog("Failed to decode {0} request.", Method);
Reply(llvm::make_error<LSPError>("failed to decode request",
ErrorCode::InvalidRequest));
}
auto P = parse<Param>(RawParams, Method, "request");
if (!P)
return Reply(P.takeError());
(Server.*Handler)(*P, std::move(Reply));
};
}

Expand Down Expand Up @@ -292,14 +288,12 @@ class ClangdLSPServer::MessageHandler : public Transport::MessageHandler {
void (ClangdLSPServer::*Handler)(const Param &)) {
Notifications[Method] = [Method, Handler,
this](llvm::json::Value RawParams) {
Param P;
if (!fromJSON(RawParams, P)) {
elog("Failed to decode {0} request.", Method);
return;
}
llvm::Expected<Param> P = parse<Param>(RawParams, Method, "request");
if (!P)
return llvm::consumeError(P.takeError());
trace::Span Tracer(Method, LSPLatency);
SPAN_ATTACH(Tracer, "Params", RawParams);
(Server.*Handler)(P);
(Server.*Handler)(*P);
};
}

Expand Down
37 changes: 26 additions & 11 deletions clang-tools-extra/clangd/ClangdLSPServer.h
Expand Up @@ -180,22 +180,37 @@ class ClangdLSPServer : private ClangdServer::Callbacks {
std::unique_ptr<MessageHandler> MsgHandler;
std::mutex TranspWriter;

template <typename T>
static Expected<T> parse(const llvm::json::Value &Raw,
llvm::StringRef PayloadName,
llvm::StringRef PayloadKind) {
T Result;
llvm::json::Path::Root Root;
if (!fromJSON(Raw, Result, Root)) {
elog("Failed to decode {0} {1}", PayloadName, PayloadKind);
// Dump the relevant parts of the broken message.
std::string Context;
llvm::raw_string_ostream OS(Context);
Root.printErrorContext(Raw, OS);
vlog("{0}", OS.str());
// Report the error (e.g. to the client).
return llvm::make_error<LSPError>(
llvm::formatv("failed to decode {0} {1}", PayloadName, PayloadKind),
ErrorCode::InvalidParams);
}
return std::move(Result);
}

template <typename Response>
void call(StringRef Method, llvm::json::Value Params, Callback<Response> CB) {
// Wrap the callback with LSP conversion and error-handling.
auto HandleReply =
[CB = std::move(CB), Ctx = Context::current().clone()](
[CB = std::move(CB), Ctx = Context::current().clone(),
Method = Method.str()](
llvm::Expected<llvm::json::Value> RawResponse) mutable {
Response Rsp;
if (!RawResponse) {
CB(RawResponse.takeError());
} else if (fromJSON(*RawResponse, Rsp)) {
CB(std::move(Rsp));
} else {
elog("Failed to decode {0} response", *RawResponse);
CB(llvm::make_error<LSPError>("failed to decode response",
ErrorCode::InvalidParams));
}
if (!RawResponse)
return CB(RawResponse.takeError());
CB(parse<Response>(*RawResponse, Method, "response"));
};
callRaw(Method, std::move(Params), std::move(HandleReply));
}
Expand Down

0 comments on commit fa69b60

Please sign in to comment.