CPP-799 Handle HTTP client request timeouts (#276)

* Expand HTTP client error handling
* Handle and verify`cancel()`
* Pinned to version=1
* Corrected default metadata service port
* Cleaned up deeply nested error handling (in favor of early return)

Author: mpenick

cpp-driver/gtests/src/unit/http_server.cpp

@@ -27,7 +27,7 @@ using datastax::internal::core::Task;
 String response(int status, const String& body = "", const String& content_type = "") {
   OStringStream ss;
   ss << "HTTP/1.0 " << status << " " << http_status_str(static_cast<http_status>(status)) << "\r\n";
-  if (status == 200 && !body.empty()) {
+  if (!body.empty()) {
     ss << "Content-Type: ";
     if (content_type.empty()) {
       ss << "text/plain";
@@ -63,13 +63,14 @@ bool Server::use_ssl(const String& key, const String& cert, const String& passwo
 }
 
 Server::ClientConnection::ClientConnection(internal::ServerConnection* server_connection,
-                                           const String& path, const String& content_type,
-                                           const String& response_body, bool enable_valid_response)
+                                           Server* server)
     : internal::ClientConnection(server_connection)
-    , path_(path)
-    , content_type_(content_type)
-    , response_body_(response_body)
-    , enable_valid_response_(enable_valid_response) {
+    , path_(server->path())
+    , content_type_(server->content_type())
+    , response_body_(server->response_body())
+    , response_status_code_(server->response_status_code())
+    , enable_valid_response_(server->enable_valid_response())
+    , close_connnection_after_request_(server->close_connnection_after_request()) {
   http_parser_init(&parser_, HTTP_REQUEST);
   http_parser_settings_init(&parser_settings_);
 
@@ -95,17 +96,26 @@ int Server::ClientConnection::on_url(http_parser* parser, const char* buf, size_
 
 void Server::ClientConnection::handle_url(const char* buf, size_t len) {
   String path(buf, len);
-  if (path == path_) {
+  if (path.substr(0, path.find("?")) == path_) { // Compare without query parameters
     if (enable_valid_response_) {
       if (response_body_.empty()) {
-        write(response(200, request_)); // Echo response
+        write(response(response_status_code_, request_)); // Echo response
       } else {
-        write(response(200, response_body_, content_type_));
+        write(response(response_status_code_, response_body_, content_type_));
       }
     } else {
       write("Invalid HTTP server response");
     }
   } else {
     write(response(404));
   }
+  // From the HTTP/1.0 protocol specification:
+  //
+  // > When an Entity-Body is included with a message, the length of that body may be determined in
+  // > one of two ways. If a Content-Length header field is present, its value in bytes represents
+  // > the length of the Entity-Body. Otherwise, the body length is determined by the closing of the
+  // > connection by the server.
+  if (close_connnection_after_request_) {
+    close();
+  }
 }

cpp-driver/gtests/src/unit/http_server.hpp

@@ -33,12 +33,13 @@ namespace mockssandra { namespace http {
  * server.
  */
 class Server {
-public:
 public:
   Server()
       : path_("/")
       , content_type_("text/plain")
+      , response_status_code_(200)
       , enable_valid_response_(true)
+      , close_connnection_after_request_(true)
       , event_loop_group_(1, "HTTP Server")
       , factory_(this)
       , server_connection_(new internal::ServerConnection(
@@ -47,12 +48,18 @@ class Server {
   const String& path() const { return path_; }
   const String& content_type() const { return content_type_; }
   const String& response_body() const { return response_body_; }
+  int response_status_code() const { return response_status_code_; }
   bool enable_valid_response() { return enable_valid_response_; }
+  bool close_connnection_after_request() { return close_connnection_after_request_; }
 
   void set_path(const String& path) { path_ = path; }
   void set_content_type(const String& content_type) { content_type_ = content_type; }
   void set_response_body(const String& response_body) { response_body_ = response_body; }
+  void set_response_status_code(int status_code) { response_status_code_ = status_code; }
   void enable_valid_response(bool enable) { enable_valid_response_ = enable; }
+  void set_close_connnection_after_request(bool enable) {
+    close_connnection_after_request_ = enable;
+  }
 
   bool use_ssl(const String& key, const String& cert, const String& password = "",
                const String& client_cert = "");
@@ -63,9 +70,7 @@ class Server {
 private:
   class ClientConnection : public internal::ClientConnection {
   public:
-    ClientConnection(internal::ServerConnection* server_connection, const String& endpoint,
-                     const String& content_type, const String& response_body,
-                     bool enable_valid_response);
+    ClientConnection(internal::ServerConnection* server_connection, Server* server);
 
     virtual void on_read(const char* data, size_t len);
 
@@ -77,7 +82,9 @@ class Server {
     String path_;
     String content_type_;
     String response_body_;
+    int response_status_code_;
     bool enable_valid_response_;
+    bool close_connnection_after_request_;
     String request_;
     http_parser parser_;
     http_parser_settings parser_settings_;
@@ -90,8 +97,7 @@ class Server {
 
     virtual internal::ClientConnection*
     create(internal::ServerConnection* server_connection) const {
-      return new ClientConnection(server_connection, server_->path(), server_->content_type(),
-                                  server_->response_body(), server_->enable_valid_response());
+      return new ClientConnection(server_connection, server_);
     }
 
   private:
@@ -102,7 +108,9 @@ class Server {
   String path_;
   String content_type_;
   String response_body_;
+  int response_status_code_;
   bool enable_valid_response_;
+  bool close_connnection_after_request_;
   SimpleEventLoopGroup event_loop_group_;
   ClientConnectionFactory factory_;
   internal::ServerConnection::Ptr server_connection_;

cpp-driver/gtests/src/unit/http_test.hpp

@@ -39,8 +39,14 @@ class HttpTest : public LoopTest {
     server_.set_response_body(response_body);
   }
 
+  void set_response_status_code(int status_code) { server_.set_response_status_code(status_code); }
+
   void enable_valid_response(bool enable) { server_.enable_valid_response(enable); }
 
+  void set_close_connnection_after_request(bool enable) {
+    server_.set_close_connnection_after_request(enable);
+  }
+
   void start_http_server() { server_.listen(); }
   void stop_http_server() { server_.close(); }
 

cpp-driver/gtests/src/unit/tests/test_cloud_secure_connect_config.cpp

@@ -52,6 +52,7 @@
 #define SNI_HOST_ID_1 "276b1694-64c4-4ba8-afb4-e33915a02f1e"
 #define SNI_HOST_ID_2 "8c29f723-5c1c-4ffd-a4ef-8c683a7fc02b"
 #define SNI_HOST_ID_3 "fb91d3ff-47cb-447d-b31d-c5721ca8d7ab"
+#define METADATA_SERVICE_PORT 30443
 
 using datastax::String;
 using datastax::internal::core::AddressVec;
@@ -404,12 +405,17 @@ class CloudMetadataServerTest : public CloudSecureConnectionConfigTest {
 
     const AddressVec& contact_points = resolver->resolved_contact_points();
     ASSERT_EQ(3u, contact_points.size());
-    EXPECT_EQ(Address(SNI_HOST, CASS_DEFAULT_PORT, SNI_HOST_ID_1), contact_points[0]);
-    EXPECT_EQ(Address(SNI_HOST, CASS_DEFAULT_PORT, SNI_HOST_ID_2), contact_points[1]);
-    EXPECT_EQ(Address(SNI_HOST, CASS_DEFAULT_PORT, SNI_HOST_ID_3), contact_points[2]);
+    EXPECT_EQ(Address(SNI_HOST, METADATA_SERVICE_PORT, SNI_HOST_ID_1), contact_points[0]);
+    EXPECT_EQ(Address(SNI_HOST, METADATA_SERVICE_PORT, SNI_HOST_ID_2), contact_points[1]);
+    EXPECT_EQ(Address(SNI_HOST, METADATA_SERVICE_PORT, SNI_HOST_ID_3), contact_points[2]);
   }
 
   static void on_resolve_failed(ClusterMetadataResolver* resolver, bool* flag) {
+    *flag = true;
+    EXPECT_EQ(0u, resolver->resolved_contact_points().size());
+  }
+
+  static void on_resolve_local_dc_failed(ClusterMetadataResolver* resolver, bool* flag) {
     *flag = true;
     EXPECT_EQ("", resolver->local_dc());
     EXPECT_EQ(0u, resolver->resolved_contact_points().size());
@@ -525,7 +531,8 @@ TEST_F(CloudMetadataServerTest, ResolveV1MissingLocalDcSsl) {
 
   bool is_resolved = false;
   AddressVec contact_points;
-  resolver()->resolve(loop(), contact_points, bind_callback(on_resolve_failed, &is_resolved));
+  resolver()->resolve(loop(), contact_points,
+                      bind_callback(on_resolve_local_dc_failed, &is_resolved));
   uv_run(loop(), UV_RUN_DEFAULT);
   EXPECT_TRUE(is_resolved);
 
@@ -555,4 +562,67 @@ TEST_F(CloudMetadataServerTest, ResolveV1MissingSniProxyAddressSsl) {
 
   stop_http_server();
 }
+
+TEST_F(CloudMetadataServerTest, ResolveInvalidJsonResponse) {
+  add_logging_critera("Unable to configure driver from metadata server: Metadata JSON is invalid");
+
+  set_path("/metadata");
+  set_response_body("[]");
+  set_content_type("application/json");
+  HttpTest::start_http_server();
+
+  bool is_resolve_failed = false;
+  AddressVec contact_points;
+  resolver()->resolve(loop(), contact_points, bind_callback(on_resolve_failed, &is_resolve_failed));
+  uv_run(loop(), UV_RUN_DEFAULT);
+  EXPECT_TRUE(is_resolve_failed);
+  EXPECT_EQ(logging_criteria_count(), 1);
+
+  stop_http_server();
+}
+
+TEST_F(CloudMetadataServerTest, ResolveErrorResponse) {
+  add_logging_critera("Unable to configure driver from metadata server: Returned error response "
+                      "code 400: 'Invalid version'");
+
+  const char* response_body = "{"
+                              "\"code\": 400,"
+                              "\"message\": \"Invalid version\""
+                              "}";
+
+  set_path("/metadata");
+  set_response_body(response_body);
+  set_response_status_code(400);
+  set_content_type("application/json");
+  HttpTest::start_http_server();
+
+  bool is_resolve_failed = false;
+  AddressVec contact_points;
+  resolver()->resolve(loop(), contact_points, bind_callback(on_resolve_failed, &is_resolve_failed));
+  uv_run(loop(), UV_RUN_DEFAULT);
+  EXPECT_TRUE(is_resolve_failed);
+  EXPECT_EQ(logging_criteria_count(), 1);
+
+  stop_http_server();
+}
+
+TEST_F(CloudMetadataServerTest, ResolveInvalidJsonErrorResponse) {
+  add_logging_critera("Unable to configure driver from metadata server: Returned error response "
+                      "code 400: '[]'");
+
+  set_path("/metadata");
+  set_response_body("[]");
+  set_response_status_code(400);
+  set_content_type("application/json");
+  HttpTest::start_http_server();
+
+  bool is_resolve_failed = false;
+  AddressVec contact_points;
+  resolver()->resolve(loop(), contact_points, bind_callback(on_resolve_failed, &is_resolve_failed));
+  uv_run(loop(), UV_RUN_DEFAULT);
+  EXPECT_TRUE(is_resolve_failed);
+  EXPECT_EQ(logging_criteria_count(), 1);
+
+  stop_http_server();
+}
 #endif