Skip to content
This repository has been archived by the owner on Aug 19, 2019. It is now read-only.

Handle error response codes to HTTP requests. #61

Merged
merged 3 commits into from
Feb 26, 2018
Merged

Handle error response codes to HTTP requests. #61

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2785388
Handle error response codes to HTTP requests.
igorpeshansky Feb 24, 2018
32e3db1
Handle HTTP response codes in the OAuth code.
igorpeshansky Feb 26, 2018
b64acd4
Handle more codes; error tweaks.
igorpeshansky Feb 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions src/api_server.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ class MetadataReporter {

// Send the given set of metadata.
void SendMetadata(
std::map<MonitoredResource, MetadataAgent::Metadata>&& metadata);
std::map<MonitoredResource, MetadataAgent::Metadata>&& metadata)
throw (boost::system::system_error);

const MetadataAgent& agent_;
Environment environment_;
Expand Down Expand Up @@ -171,9 +172,13 @@ void MetadataReporter::ReportMetadata() {
if (agent_.config_.VerboseLogging()) {
LOG(INFO) << "Sending metadata request to server";
}
SendMetadata(agent_.GetMetadataMap());
if (agent_.config_.VerboseLogging()) {
LOG(INFO) << "Metadata request sent successfully";
try {
SendMetadata(agent_.GetMetadataMap());
if (agent_.config_.VerboseLogging()) {
LOG(INFO) << "Metadata request sent successfully";
}
} catch (const boost::system::system_error& e) {
LOG(ERROR) << "Unsuccessful: " << e.what();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Unsuccessful/Metadata request unsuccessful/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

}
std::this_thread::sleep_for(period_);
}
Expand All @@ -185,7 +190,8 @@ namespace {
void SendMetadataRequest(std::vector<json::value>&& entries,
const std::string& endpoint,
const std::string& auth_header,
bool verbose_logging) {
bool verbose_logging)
throw (boost::system::system_error) {
json::value update_metadata_request = json::object({
{"entries", json::array(std::move(entries))},
});
Expand All @@ -204,6 +210,13 @@ void SendMetadataRequest(std::vector<json::value>&& entries,
request << boost::network::header("Authorization", auth_header);
request << boost::network::body(request_body);
http::client::response response = client.post(request);
if (status(response) != 200) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would accept anything in the 2xx range, technically a POST can result in a 201 Created. Applies to other posts as well. I'm sure it's not our contract with the resource metadata api yet, but just something to be mindful of.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

throw boost::system::system_error(
boost::system::errc::make_error_code(boost::system::errc::not_connected),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe not_connected is the best exception to throw here, but if it's just a matter of getting "something" to throw, it's fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that code is never examined.

format::Substitute("Server responded with '{{message}}' ({{code}})",
{{"message", status_message(response)},
{"code", format::str(status(response))}}));
}
if (verbose_logging) {
LOG(INFO) << "Server responded with " << body(response);
}
Expand All @@ -213,7 +226,8 @@ void SendMetadataRequest(std::vector<json::value>&& entries,
}

void MetadataReporter::SendMetadata(
std::map<MonitoredResource, MetadataAgent::Metadata>&& metadata) {
std::map<MonitoredResource, MetadataAgent::Metadata>&& metadata)
throw (boost::system::system_error) {
if (metadata.empty()) {
if (agent_.config_.VerboseLogging()) {
LOG(INFO) << "No data to send";
Expand Down
8 changes: 8 additions & 0 deletions src/docker.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <boost/network/protocol/http/client.hpp>
#include <chrono>

#include "format.h"
#include "instance.h"
#include "json.h"
#include "logging.h"
Expand Down Expand Up @@ -169,6 +170,13 @@ json::value DockerReader::QueryDocker(const std::string& path) const
}
try {
http::local_client::response response = client.get(request);
if (status(response) != 200) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as a note, I'm fine with just a 200 here as you have it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed for consistency.

throw boost::system::system_error(
boost::system::errc::make_error_code(boost::system::errc::not_connected),
format::Substitute("Server responded with '{{message}}' ({{code}})",
{{"message", status_message(response)},
{"code", format::str(status(response))}}));
}
#ifdef VERBOSE
LOG(DEBUG) << "QueryDocker: Response: " << body(response);
#endif
Expand Down
11 changes: 10 additions & 1 deletion src/environment.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <boost/network/protocol/http/client.hpp>
#include <fstream>

#include "format.h"
#include "logging.h"

namespace http = boost::network::http;
Expand Down Expand Up @@ -85,7 +86,15 @@ std::string Environment::GetMetadataString(const std::string& path) const {
request << boost::network::header("Metadata-Flavor", "Google");
try {
http::client::response response = client.get(request);
return body(response);
if (status(response) == 200) {
return body(response);
} else {
throw boost::system::system_error(
boost::system::errc::make_error_code(boost::system::errc::not_connected),
format::Substitute("Server responded with '{{message}}' ({{code}})",
{{"message", status_message(response)},
{"code", format::str(status(response))}}));
}
} catch (const boost::system::system_error& e) {
LOG(ERROR) << "Exception: " << e.what()
<< ": '" << kGceMetadataServerAddress << path << "'";
Expand Down
13 changes: 13 additions & 0 deletions src/format.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,19 @@

namespace format {

namespace {
template<class T>
std::string AsString(T v) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see this being used anywhere.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 32-34 below.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, my eyes certainly failed me 🤣

std::ostringstream out;
out << v;
return out.str();
}
}

std::string str(int i) { return AsString(i); }
std::string str(double d) { return AsString(d); }
std::string str(bool b) { return AsString(b); }

std::string Substitute(const std::string& format,
const std::map<std::string, std::string>&& params)
throw(Exception) {
Expand Down
4 changes: 4 additions & 0 deletions src/format.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ class Exception {
std::string explanation_;
};

std::string str(int);
std::string str(double);
std::string str(bool);

// Format string substitution.
// Placeholder format is '{{param}}'.
// All instances of each placeholder will be substituted by the value of the
Expand Down
8 changes: 8 additions & 0 deletions src/kubernetes.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include <fstream>
#include <tuple>

#include "format.h"
#include "http_common.h"
#include "instance.h"
#include "json.h"
Expand Down Expand Up @@ -549,6 +550,13 @@ json::value KubernetesReader::QueryMaster(const std::string& path) const
}
try {
http::client::response response = client.get(request);
if (status(response) != 200) {
throw boost::system::system_error(
boost::system::errc::make_error_code(boost::system::errc::not_connected),
format::Substitute("Server responded with '{{message}}' ({{code}})",
{{"message", status_message(response)},
{"code", format::str(status(response))}}));
}
#ifdef VERBOSE
LOG(DEBUG) << "QueryMaster: Response: " << body(response);
#endif
Expand Down
11 changes: 11 additions & 0 deletions src/oauth2.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <openssl/pkcs12.h>

#include "base64.h"
#include "format.h"
#include "http_common.h"
#include "json.h"
#include "logging.h"
Expand Down Expand Up @@ -236,6 +237,13 @@ json::value OAuth2::ComputeTokenFromCredentials() const {
<< " body: " << request.body();
}
http::client::response response = client.post(request);
if (status(response) != 200) {
throw boost::system::system_error(
boost::system::errc::make_error_code(boost::system::errc::not_connected),
format::Substitute("Server responded with '{{message}}' ({{code}})",
{{"message", status_message(response)},
{"code", format::str(status(response))}}));
}
if (environment_.config().VerboseLogging()) {
LOG(INFO) << "Token response: " << body(response);
}
Expand All @@ -248,6 +256,9 @@ json::value OAuth2::ComputeTokenFromCredentials() const {
} catch (const json::Exception& e) {
LOG(ERROR) << e.what();
return nullptr;
} catch (const boost::system::system_error& e) {
LOG(ERROR) << "HTTP error: " << e.what();
return nullptr;
}
}

Expand Down