-
Notifications
You must be signed in to change notification settings - Fork 11
Set Content-Length when writing responses to close client connections. #168
Conversation
src/api_server.cc
Outdated
|
||
std::string response = "unhealthy components:\n"; | ||
for (const auto& component : unhealthy_components) { | ||
response.append(component + "\n"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use std::ostringstrean
instead of std::string::append
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, but I'll link to this for context:
https://stackoverflow.com/questions/905479/stdstring-length-and-size-member-functions
src/api_server.cc
Outdated
conn->set_headers(std::map<std::string, std::string>({ | ||
{"Content-Type", "application/json"}, | ||
{"Content-Length", std::to_string(response.length())}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we use std::string::size
everywhere else...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
src/api_server.cc
Outdated
} else { | ||
LOG(WARNING) << "/healthz returning 500; unhealthy components: " | ||
<< boost::algorithm::join(unhealthy_components, ", "); | ||
conn->set_status(HttpServer::connection::internal_server_error); | ||
|
||
std::ostringstream response_stream; | ||
response_stream << "unhealthy components:\n"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Optional] You can combine these:
std::ostringstream response_stream("unhealthy components:\n");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.