Skip to content

Commit 138d4ca

Browse files
committed
Adjusting Transport::Read to return an optional<string> and adding logging directly to the Transport class.
1 parent c340c38 commit 138d4ca

File tree

5 files changed

+132
-120
lines changed

5 files changed

+132
-120
lines changed

lldb/tools/lldb-dap/DAP.cpp

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,12 @@ const char DEV_NULL[] = "/dev/null";
6464

6565
namespace lldb_dap {
6666

67-
DAP::DAP(llvm::StringRef client_name, llvm::StringRef path, std::ofstream *log,
68-
lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
69-
std::vector<std::string> pre_init_commands)
70-
: client_name(client_name), debug_adapter_path(path), log(log),
71-
transport(client_name, std::move(input), std::move(output)),
72-
broadcaster("lldb-dap"), exception_breakpoints(),
67+
DAP::DAP(llvm::StringRef path, std::ofstream *log,
68+
const ReplMode default_repl_mode,
69+
const std::vector<std::string> &pre_init_commands,
70+
llvm::StringRef client_name, Transport &transport)
71+
: debug_adapter_path(path), log(log), client_name(client_name),
72+
transport(transport), broadcaster("lldb-dap"), exception_breakpoints(),
7373
pre_init_commands(std::move(pre_init_commands)),
7474
focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false),
7575
enable_auto_variable_summaries(false),
@@ -79,7 +79,7 @@ DAP::DAP(llvm::StringRef client_name, llvm::StringRef path, std::ofstream *log,
7979
configuration_done_sent(false), waiting_for_run_in_terminal(false),
8080
progress_event_reporter(
8181
[&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }),
82-
reverse_request_seq(0), repl_mode(repl_mode) {}
82+
reverse_request_seq(0), repl_mode(default_repl_mode) {}
8383

8484
DAP::~DAP() = default;
8585

@@ -227,22 +227,17 @@ void DAP::StopEventHandlers() {
227227
void DAP::SendJSON(const llvm::json::Value &json) {
228228
// FIXME: Instead of parsing the output message from JSON, pass the `Message`
229229
// as parameter to `SendJSON`.
230-
protocol::Message M;
230+
protocol::Message message;
231231
llvm::json::Path::Root root;
232-
if (!protocol::fromJSON(json, M, root)) {
233-
if (log) {
234-
std::string error;
235-
llvm::raw_string_ostream OS(error);
236-
root.printErrorContext(json, OS);
237-
*log << "encoding failure: " << error << "\n";
238-
}
232+
if (!fromJSON(json, message, root)) {
233+
DAP_LOG_ERROR(log, root.getError(), "({1}) encoding failed: {0}",
234+
client_name);
239235
return;
240236
}
241-
auto status = transport.Write(log, M);
242-
if (status.Fail() && log)
243-
*log << llvm::formatv("failed to send {0}: {1}\n", llvm::json::Value(M),
244-
status.AsCString())
245-
.str();
237+
auto err = transport.Write(message);
238+
if (err) {
239+
DAP_LOG_ERROR(log, std::move(err), "({1}) write failed: {0}", client_name);
240+
}
246241
}
247242

248243
// "OutputEvent": {
@@ -775,13 +770,9 @@ llvm::Error DAP::Loop() {
775770
StopEventHandlers();
776771
});
777772
while (!disconnecting) {
778-
auto next = transport.Read(log);
779-
if (auto Err = next.takeError()) {
780-
// On EOF, simply break out of the loop.
781-
std::error_code ec = llvm::errorToErrorCode(std::move(Err));
782-
if (ec == Transport::kEOF)
783-
break;
784-
return llvm::errorCodeToError(ec);
773+
std::optional<protocol::Message> next = transport.Read();
774+
if (!next) {
775+
break;
785776
}
786777

787778
if (!HandleObject(*next)) {

lldb/tools/lldb-dap/DAP.h

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -125,30 +125,30 @@ struct Variables {
125125

126126
struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface {
127127
DAP &dap;
128-
explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {};
128+
explicit StartDebuggingRequestHandler(DAP &d) : dap(d){};
129129
bool DoExecute(lldb::SBDebugger debugger, char **command,
130130
lldb::SBCommandReturnObject &result) override;
131131
};
132132

133133
struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface {
134134
DAP &dap;
135-
explicit ReplModeRequestHandler(DAP &d) : dap(d) {};
135+
explicit ReplModeRequestHandler(DAP &d) : dap(d){};
136136
bool DoExecute(lldb::SBDebugger debugger, char **command,
137137
lldb::SBCommandReturnObject &result) override;
138138
};
139139

140140
struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
141141
DAP &dap;
142-
explicit SendEventRequestHandler(DAP &d) : dap(d) {};
142+
explicit SendEventRequestHandler(DAP &d) : dap(d){};
143143
bool DoExecute(lldb::SBDebugger debugger, char **command,
144144
lldb::SBCommandReturnObject &result) override;
145145
};
146146

147147
struct DAP {
148-
llvm::StringRef client_name;
149148
llvm::StringRef debug_adapter_path;
150149
std::ofstream *log;
151-
Transport transport;
150+
llvm::StringRef client_name;
151+
Transport &transport;
152152
lldb::SBFile in;
153153
OutputRedirector out;
154154
OutputRedirector err;
@@ -209,12 +209,33 @@ struct DAP {
209209
// will contain that expression.
210210
std::string last_nonempty_var_expression;
211211

212-
DAP(llvm::StringRef client_name, llvm::StringRef path, std::ofstream *log,
213-
lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode,
214-
std::vector<std::string> pre_init_commands);
212+
/// Creates a new DAP sessions.
213+
///
214+
/// \param[in] path
215+
/// Path to the lldb-dap binary.
216+
/// \param[in] log
217+
/// Log file stream, if configured.
218+
/// \param[in] default_repl_mode
219+
/// Default repl mode behavior, as configured by the binary.
220+
/// \param[in] pre_init_commands
221+
/// LLDB commands to execute as soon as the debugger instance is allocaed.
222+
/// \param[in] client_name
223+
/// Debug session client name, for example 'stdin/stdout' or 'client_1'.
224+
/// \param[in] transport
225+
/// Transport for this debug session.
226+
DAP(llvm::StringRef path, std::ofstream *log,
227+
const ReplMode default_repl_mode,
228+
const std::vector<std::string> &pre_init_commands,
229+
llvm::StringRef client_name, Transport &transport);
230+
215231
~DAP();
232+
233+
/// DAP is not copyable.
234+
/// @{
216235
DAP(const DAP &rhs) = delete;
217236
void operator=(const DAP &rhs) = delete;
237+
/// @}
238+
218239
ExceptionBreakpoint *GetExceptionBreakpoint(const std::string &filter);
219240
ExceptionBreakpoint *GetExceptionBreakpoint(const lldb::break_id_t bp_id);
220241

lldb/tools/lldb-dap/Transport.cpp

Lines changed: 68 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,16 @@
77
//===----------------------------------------------------------------------===//
88

99
#include "Transport.h"
10+
#include "DAPLog.h"
1011
#include "Protocol.h"
11-
#include "c++/v1/__system_error/error_code.h"
1212
#include "lldb/Utility/IOObject.h"
1313
#include "lldb/Utility/Status.h"
1414
#include "lldb/lldb-forward.h"
1515
#include "llvm/ADT/StringExtras.h"
16+
#include "llvm/ADT/StringRef.h"
1617
#include "llvm/Support/Error.h"
1718
#include "llvm/Support/raw_ostream.h"
1819
#include <string>
19-
#include <system_error>
2020
#include <utility>
2121

2222
using namespace llvm;
@@ -28,18 +28,11 @@ using namespace lldb_dap::protocol;
2828
static Expected<std::string> ReadFull(IOObjectSP &descriptor, size_t length) {
2929
if (!descriptor || !descriptor->IsValid())
3030
return createStringError("transport input is closed");
31-
3231
std::string data;
3332
data.resize(length);
34-
3533
auto status = descriptor->Read(data.data(), length);
3634
if (status.Fail())
3735
return status.takeError();
38-
39-
// If we got back zero then we have reached EOF.
40-
if (length == 0)
41-
return createStringError(Transport::kEOF, "end-of-file");
42-
4336
return data.substr(0, length);
4437
}
4538

@@ -48,99 +41,101 @@ static Expected<std::string> ReadUntil(IOObjectSP &descriptor,
4841
std::string buffer;
4942
buffer.reserve(delimiter.size() + 1);
5043
while (!llvm::StringRef(buffer).ends_with(delimiter)) {
51-
auto next = ReadFull(descriptor, 1);
44+
Expected<std::string> next =
45+
ReadFull(descriptor, buffer.empty() ? delimiter.size() : 1);
5246
if (auto Err = next.takeError())
5347
return std::move(Err);
48+
// '' is returned on EOF.
49+
if (next->empty())
50+
return buffer;
5451
buffer += *next;
5552
}
5653
return buffer.substr(0, buffer.size() - delimiter.size());
5754
}
5855

59-
static Error ReadExpected(IOObjectSP &descriptor, StringRef want) {
60-
auto got = ReadFull(descriptor, want.size());
61-
if (auto Err = got.takeError())
62-
return Err;
63-
if (*got != want) {
64-
return createStringError("want %s, got %s", want.str().c_str(),
65-
got->c_str());
66-
}
67-
return Error::success();
68-
}
56+
/// DAP message format
57+
/// ```
58+
/// Content-Length: (?<length>\d+)\r\n\r\n(?<content>.{\k<length>})
59+
/// ```
60+
static const StringLiteral kHeaderContentLength = "Content-Length: ";
61+
static const StringLiteral kHeaderSeparator = "\r\n\r\n";
6962

7063
namespace lldb_dap {
7164

72-
const std::error_code Transport::kEOF =
73-
std::error_code(0x1001, std::generic_category());
74-
75-
Transport::Transport(StringRef client_name, IOObjectSP input, IOObjectSP output)
76-
: m_client_name(client_name), m_input(std::move(input)),
65+
Transport::Transport(StringRef client_name, std::ofstream *log,
66+
IOObjectSP input, IOObjectSP output)
67+
: m_client_name(client_name), m_log(log), m_input(std::move(input)),
7768
m_output(std::move(output)) {}
7869

79-
Expected<protocol::Message> Transport::Read(std::ofstream *log) {
80-
// If we don't find the expected header we have reached EOF.
81-
if (auto Err = ReadExpected(m_input, "Content-Length: "))
82-
return std::move(Err);
70+
std::optional<Message> Transport::Read() {
71+
Expected<std::string> message_header =
72+
ReadFull(m_input, kHeaderContentLength.size());
73+
if (!message_header) {
74+
DAP_LOG_ERROR(m_log, message_header.takeError(), "({1}) read failed: {0}",
75+
m_client_name);
76+
return std::nullopt;
77+
}
8378

84-
auto rawLength = ReadUntil(m_input, "\r\n\r\n");
85-
if (auto Err = rawLength.takeError())
86-
return std::move(Err);
79+
// '' returned on EOF.
80+
if (message_header->empty())
81+
return std::nullopt;
82+
if (*message_header != kHeaderContentLength) {
83+
DAP_LOG(m_log, "({0}) read failed: expected '{1}' and got '{2}'",
84+
m_client_name, kHeaderContentLength, *message_header);
85+
return std::nullopt;
86+
}
87+
88+
Expected<std::string> raw_length = ReadUntil(m_input, kHeaderSeparator);
89+
if (!raw_length) {
90+
DAP_LOG_ERROR(m_log, raw_length.takeError(), "({1}) read failed: {0}",
91+
m_client_name);
92+
return std::nullopt;
93+
}
8794

8895
size_t length;
89-
if (!to_integer(*rawLength, length))
90-
return createStringError("invalid content length %s", rawLength->c_str());
91-
92-
auto rawJSON = ReadFull(m_input, length);
93-
if (auto Err = rawJSON.takeError())
94-
return std::move(Err);
95-
if (rawJSON->length() != length)
96-
return createStringError(
97-
"malformed request, expected %ld bytes, got %ld bytes", length,
98-
rawJSON->length());
99-
100-
if (log) {
101-
auto now = std::chrono::duration<double>(
102-
std::chrono::system_clock::now().time_since_epoch());
103-
*log << formatv("{0:f9} <-- ({1}) {2}\n", now.count(), m_client_name,
104-
*rawJSON)
105-
.str();
96+
if (!to_integer(*raw_length, length)) {
97+
DAP_LOG(m_log, "({0}) read failed: invalid content length {1}",
98+
m_client_name, *raw_length);
99+
return std::nullopt;
106100
}
107101

108-
auto JSON = json::parse(*rawJSON);
109-
if (auto Err = JSON.takeError()) {
110-
return createStringError("malformed JSON %s\n%s", rawJSON->c_str(),
111-
llvm::toString(std::move(Err)).c_str());
102+
Expected<std::string> raw_json = ReadFull(m_input, length);
103+
if (!raw_json) {
104+
DAP_LOG_ERROR(m_log, raw_json.takeError(), "({1}) read failed: {0}",
105+
m_client_name);
106+
return std::nullopt;
107+
}
108+
if (raw_json->length() != length) {
109+
DAP_LOG(m_log, "({0}) read failed: expected {1} bytes and got {2} bytes",
110+
m_client_name, length, raw_json->length());
111+
return std::nullopt;
112112
}
113113

114-
protocol::Message M;
115-
llvm::json::Path::Root Root;
116-
if (!fromJSON(*JSON, M, Root)) {
117-
std::string error;
118-
raw_string_ostream OS(error);
119-
Root.printErrorContext(*JSON, OS);
120-
return createStringError("malformed request: %s", error.c_str());
114+
DAP_LOG(m_log, "<-- ({0}) {1}", m_client_name, *raw_json);
115+
116+
llvm::Expected<Message> message = json::parse<Message>(*raw_json);
117+
if (!message) {
118+
DAP_LOG_ERROR(m_log, message.takeError(), "({1}) read failed: {0}",
119+
m_client_name);
120+
return std::nullopt;
121121
}
122-
return std::move(M);
122+
123+
return std::move(*message);
123124
}
124125

125-
lldb_private::Status Transport::Write(std::ofstream *log,
126-
const protocol::Message &M) {
126+
Error Transport::Write(const Message &message) {
127127
if (!m_output || !m_output->IsValid())
128-
return Status("transport output is closed");
128+
return createStringError("transport output is closed");
129129

130-
std::string JSON = formatv("{0}", toJSON(M)).str();
130+
std::string json = formatv("{0}", toJSON(message)).str();
131131

132-
if (log) {
133-
auto now = std::chrono::duration<double>(
134-
std::chrono::system_clock::now().time_since_epoch());
135-
*log << formatv("{0:f9} --> ({1}) {2}\n", now.count(), m_client_name, JSON)
136-
.str();
137-
}
132+
DAP_LOG(m_log, "--> ({0}) {1}", m_client_name, json);
138133

139134
std::string Output;
140135
raw_string_ostream OS(Output);
141-
OS << "Content-Length: " << JSON.length() << "\r\n\r\n" << JSON;
136+
OS << kHeaderContentLength << json.length() << kHeaderSeparator << json;
142137
size_t num_bytes = Output.size();
143-
return m_output->Write(Output.data(), num_bytes);
138+
return m_output->Write(Output.data(), num_bytes).takeError();
144139
}
145140

146141
} // end namespace lldb_dap

lldb/tools/lldb-dap/Transport.h

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,36 +15,37 @@
1515
#define LLDB_TOOLS_LLDB_DAP_TRANSPORT_H
1616

1717
#include "Protocol.h"
18-
#include "lldb/Utility/Status.h"
1918
#include "lldb/lldb-forward.h"
2019
#include "llvm/ADT/StringRef.h"
2120
#include "llvm/Support/Error.h"
2221
#include <fstream>
22+
#include <optional>
2323

2424
namespace lldb_dap {
2525

2626
/// A transport class that performs the Debug Adapter Protocol communication
2727
/// with the client.
2828
class Transport {
2929
public:
30-
Transport(llvm::StringRef client_name, lldb::IOObjectSP input,
31-
lldb::IOObjectSP output);
30+
Transport(llvm::StringRef client_name, std::ofstream *log,
31+
lldb::IOObjectSP input, lldb::IOObjectSP output);
3232
~Transport() = default;
3333

34+
/// Transport is not copyable.
35+
/// @{
3436
Transport(const Transport &rhs) = delete;
3537
void operator=(const Transport &rhs) = delete;
36-
37-
/// Error code returned this if EOF is encountered.
38-
static const std::error_code kEOF;
38+
/// @}
3939

4040
/// Writes a Debug Adater Protocol message to the output stream.
41-
lldb_private::Status Write(std::ofstream *log, const protocol::Message &M);
41+
llvm::Error Write(const protocol::Message &M);
4242

4343
/// Reads the next Debug Adater Protocol message from the input stream.
44-
llvm::Expected<protocol::Message> Read(std::ofstream *log);
44+
std::optional<protocol::Message> Read();
4545

4646
private:
4747
llvm::StringRef m_client_name;
48+
std::ofstream *m_log;
4849
lldb::IOObjectSP m_input;
4950
lldb::IOObjectSP m_output;
5051
};

0 commit comments

Comments
 (0)