Fix an issue of receiving response; refine error handling.

example/rest/book_async_client/main.cc

@@ -19,13 +19,13 @@ class BookListClient {
  public:
   BookListClient(boost::asio::io_context& io_context,
                  const std::string& host, const std::string& port)
-      : client_(io_context, host, port) {
+      : rest_client_(io_context, host, port) {
   }
 
   void ListBooks(webcc::HttpResponseHandler handler) {
     std::cout << "ListBooks" << std::endl;
 
-    client_.Get("/books", handler);
+    rest_client_.Get("/books", handler);
   }
 
   void CreateBook(const std::string& id,
@@ -40,11 +40,11 @@ class BookListClient {
     json["title"] = title;
     json["price"] = price;
 
-    client_.Post("/books", JsonToString(json), handler);
+    rest_client_.Post("/books", JsonToString(json), handler);
   }
 
  private:
-  webcc::RestAsyncClient client_;
+  webcc::RestAsyncClient rest_client_;
 };
 
 // -----------------------------------------------------------------------------

example/rest/book_client/main.cc

@@ -15,29 +15,60 @@ std::string JsonToString(const Json::Value& json) {
 
 // -----------------------------------------------------------------------------
 
-class BookListClient {
+class BookClientBase {
+public:
+  BookClientBase(const std::string& host, const std::string& port,
+                 int timeout_seconds)
+      : rest_client_(host, port) {
+    rest_client_.set_timeout_seconds(timeout_seconds);
+  }
+
+  virtual ~BookClientBase() = default;
+
+protected:
+  void PrintSeparateLine() {
+    std::cout << "--------------------------------";
+    std::cout << "--------------------------------";
+    std::cout << std::endl;
+  }
+
+  void PrintError() {
+    std::cout << webcc::DescribeError(rest_client_.error());
+    if (rest_client_.timeout_occurred()) {
+      std::cout << " (timeout)";
+    }
+    std::cout << std::endl;
+  }
+
+  webcc::RestClient rest_client_;
+};
+
+// -----------------------------------------------------------------------------
+
+class BookListClient : public BookClientBase {
 public:
   BookListClient(const std::string& host, const std::string& port,
                  int timeout_seconds)
-      : client_(host, port) {
-    client_.set_timeout_seconds(timeout_seconds);
+      : BookClientBase(host, port, timeout_seconds) {
   }
 
   bool ListBooks() {
+    PrintSeparateLine();
     std::cout << "ListBooks" << std::endl;
 
-    if (!client_.Get("/books")) {
-      std::cout << webcc::DescribeError(client_.error()) << std::endl;
+    if (!rest_client_.Get("/books")) {
+      PrintError();
       return false;
     }
 
-    std::cout << client_.response_content() << std::endl;
+    std::cout << rest_client_.response_content() << std::endl;
     return true;
   }
 
   bool CreateBook(const std::string& id,
                   const std::string& title,
                   double price) {
+    PrintSeparateLine();
     std::cout << "CreateBook: " << id << ", " << title << ", " << price
               << std::endl;
 
@@ -46,35 +77,32 @@ public:
     json["title"] = title;
     json["price"] = price;
 
-    if (!client_.Post("/books", JsonToString(json))) {
-      std::cout << webcc::DescribeError(client_.error()) << std::endl;
+    if (!rest_client_.Post("/books", JsonToString(json))) {
+      PrintError();
       return false;
     }
 
-    std::cout << client_.response_status() << std::endl;
+    std::cout << rest_client_.response_status() << std::endl;
 
     return true;
   }
-
-private:
-  webcc::RestClient client_;
 };
 
 // -----------------------------------------------------------------------------
 
-class BookDetailClient {
+class BookDetailClient : public BookClientBase {
 public:
   BookDetailClient(const std::string& host, const std::string& port,
                    int timeout_seconds)
-      : rest_client_(host, port) {
-    rest_client_.set_timeout_seconds(timeout_seconds);
+      : BookClientBase(host, port, timeout_seconds) {
   }
 
   bool GetBook(const std::string& id) {
+    PrintSeparateLine();
     std::cout << "GetBook: " << id << std::endl;
 
     if (!rest_client_.Get("/book/" + id)) {
-      std::cout << webcc::DescribeError(rest_client_.error()) << std::endl;
+      PrintError();
       return false;
     }
 
@@ -85,6 +113,7 @@ public:
   bool UpdateBook(const std::string& id,
                   const std::string& title,
                   double price) {
+    PrintSeparateLine();
     std::cout << "UpdateBook: " << id << ", " << title << ", " << price
               << std::endl;
 
@@ -94,7 +123,7 @@ public:
     json["price"] = price;
 
     if (!rest_client_.Put("/book/" + id, JsonToString(json))) {
-      std::cout << webcc::DescribeError(rest_client_.error()) << std::endl;
+      PrintError();
       return false;
     }
 
@@ -103,19 +132,17 @@ public:
   }
 
   bool DeleteBook(const std::string& id) {
+    PrintSeparateLine();
     std::cout << "DeleteBook: " << id << std::endl;
 
     if (!rest_client_.Delete("/book/" + id)) {
-      std::cout << webcc::DescribeError(rest_client_.error()) << std::endl;
+      PrintError();
       return false;
     }
 
     std::cout << rest_client_.response_status() << std::endl;
     return true;
   }
-
-private:
-  webcc::RestClient rest_client_;
 };
 
 // -----------------------------------------------------------------------------

src/webcc/globals.cc

@@ -34,34 +34,19 @@ const std::string kHttpDelete = "DELETE";
 const char* DescribeError(Error error) {
   switch (error) {
     case kHostResolveError:
-      return "Cannot resolve the host.";
-
+      return "Host resolve error";
     case kEndpointConnectError:
-      return "Cannot connect to remote endpoint.";
-
-    case kSocketTimeoutError:
-      return "Operation timeout.";
-
+      return "Endpoint connect error";
     case kSocketReadError:
-      return "Socket read error.";
-
+      return "Socket read error";
     case kSocketWriteError:
-      return "Socket write error.";
-
-    case kHttpStartLineError:
-      return "[HTTP Response] Start line is invalid.";
-
-    case kHttpStatusError:
-      return "[HTTP Response] Status is not OK.";
-
-    case kHttpContentLengthError:
-      return "[HTTP Response] Content-Length is invalid or missing.";
-
+      return "Socket write error";
+    case kHttpError:
+      return "HTTP error";
     case kXmlError:
       return "XML error";
-
     default:
-      return "No error";
+      return "";
   }
 }
 

src/webcc/globals.h

@@ -67,24 +67,11 @@ struct HttpStatus {
 // Error codes.
 enum Error {
   kNoError = 0,
-
   kHostResolveError,
   kEndpointConnectError,
-
-  kSocketTimeoutError,
-
   kSocketReadError,
   kSocketWriteError,
-
-  // Invalid start line in the HTTP response.
-  kHttpStartLineError,
-
-  // Status is not 200 in the HTTP response.
-  kHttpStatusError,
-
-  // Invalid or missing Content-Length in the HTTP response.
-  kHttpContentLengthError,
-
+  kHttpError,
   kXmlError,
 };
 

src/webcc/http_async_client.cc

@@ -122,15 +122,13 @@ void HttpAsyncClient::ReadHandler(boost::system::error_code ec,
 
   // Parse the response piece just read.
   // If the content has been fully received, |finished()| will be true.
-  Error error = response_parser_->Parse(buffer_.data(), length);
-
-  if (error != kNoError) {
-    response_handler_(response_, error);
+  if (!response_parser_->Parse(buffer_.data(), length)) {
+    response_handler_(response_, kHttpError);
     return;
   }
 
   if (response_parser_->finished()) {
-    response_handler_(response_, error);
+    response_handler_(response_, kHttpError);
     return;
   }
 

src/webcc/http_client.cc

@@ -24,15 +24,17 @@ HttpClient::HttpClient()
       timeout_seconds_(kMaxReceiveSeconds),
       deadline_timer_(io_context_) {
   deadline_timer_.expires_at(boost::posix_time::pos_infin);
+}
 
+bool HttpClient::Request(const HttpRequest& request) {
   response_.reset(new HttpResponse());
   response_parser_.reset(new HttpResponseParser(response_.get()));
 
+  timeout_occurred_ = false;
+
   // Start the persistent actor that checks for deadline expiry.
   CheckDeadline();
-}
 
-bool HttpClient::Request(const HttpRequest& request) {
   if ((error_ = Connect(request)) != kNoError) {
     return false;
   }
@@ -87,11 +89,10 @@ Error HttpClient::Connect(const HttpRequest& request) {
   // check whether the socket is still open before deciding if we succeeded
   // or failed.
   if (ec || !socket_.is_open()) {
-    if (ec) {
-      return kEndpointConnectError;
-    } else {
-      return kSocketTimeoutError;
+    if (!ec) {
+      timeout_occurred_ = true;
     }
+    return kEndpointConnectError;
   }
 
   return kNoError;
@@ -113,6 +114,7 @@ Error HttpClient::SendReqeust(const HttpRequest& request) {
     io_context_.run_one();
   } while (ec == boost::asio::error::would_block);
 
+  // TODO: timeout_occurred_
   if (ec) {
     return kSocketWriteError;
   }
@@ -121,30 +123,39 @@ Error HttpClient::SendReqeust(const HttpRequest& request) {
 }
 
 Error HttpClient::ReadResponse() {
+  Error error = kNoError;
+  DoReadResponse(&error);
+
+  if (error == kNoError) {
+    LOG_VERB("HTTP response:\n%s", response_->Dump(4, "> ").c_str());
+  }
+
+  return error;
+}
+
+void HttpClient::DoReadResponse(Error* error) {
   deadline_timer_.expires_from_now(
       boost::posix_time::seconds(timeout_seconds_));
 
   boost::system::error_code ec = boost::asio::error::would_block;
-  Error error = kNoError;
 
   socket_.async_read_some(
       boost::asio::buffer(buffer_),
-      [this, &ec, &error](boost::system::error_code inner_ec,
-                          std::size_t length) {
+      [this, &ec, error](boost::system::error_code inner_ec,
+                         std::size_t length) {
         ec = inner_ec;
 
         if (inner_ec || length == 0) {
+          *error = kSocketReadError;
           LOG_ERRO("Socket read error.");
-          error = kSocketReadError;
           return;
         }
 
         // Parse the response piece just read.
         // If the content has been fully received, next time flag "finished_"
         // will be set.
-        error = response_parser_->Parse(buffer_.data(), length);
-
-        if (error != kNoError) {
+        if (!response_parser_->Parse(buffer_.data(), length)) {
+          *error = kHttpError;
           LOG_ERRO("Failed to parse HTTP response.");
           return;
         }
@@ -155,19 +166,13 @@ Error HttpClient::ReadResponse() {
           return;
         }
 
-        ReadResponse();
+        DoReadResponse(error);
       });
 
   // Block until the asynchronous operation has completed.
   do {
     io_context_.run_one();
   } while (ec == boost::asio::error::would_block);
-
-  if (error == kNoError) {
-    LOG_VERB("HTTP response:\n%s", response_->Dump(4, "> ").c_str());
-  }
-
-  return error;
 }
 
 void HttpClient::CheckDeadline() {
@@ -179,6 +184,9 @@ void HttpClient::CheckDeadline() {
     boost::system::error_code ignored_ec;
     socket_.close(ignored_ec);
 
+    // TODO
+    timeout_occurred_ = true;
+
     deadline_timer_.expires_at(boost::posix_time::pos_infin);
   }
 

src/webcc/http_client.h

@@ -31,6 +31,8 @@ class HttpClient {
 
   Error error() const { return error_; }
 
+  bool timeout_occurred() const { return timeout_occurred_; }
+
   // Connect to server, send request, wait until response is received.
   bool Request(const HttpRequest& request);
 
@@ -41,6 +43,8 @@ class HttpClient {
 
   Error ReadResponse();
 
+  void DoReadResponse(Error* error);
+
   void CheckDeadline();
 
   boost::asio::io_context io_context_;
@@ -54,6 +58,9 @@ class HttpClient {
 
   Error error_ = kNoError;
 
+  // If the error was caused by timeout or not.
+  bool timeout_occurred_ = false;
+
   // Maximum seconds to wait before the client cancels the operation.
   // Only for receiving response from server.
   int timeout_seconds_;

src/webcc/http_connection.cc

@@ -55,9 +55,7 @@ void HttpConnection::ReadHandler(boost::system::error_code ec,
     return;
   }
 
-  Error error = request_parser_.Parse(buffer_.data(), length);
-
-  if (error != kNoError) {
+  if (!request_parser_.Parse(buffer_.data(), length)) {
     // Bad request.
     response_ = HttpResponse::Fault(HttpStatus::kBadRequest);
     AsyncWrite();

src/webcc/http_parser.cc

@@ -16,7 +16,7 @@ HttpParser::HttpParser(HttpMessage* message)
       finished_(false) {
 }
 
-Error HttpParser::Parse(const char* data, std::size_t len) {
+bool HttpParser::Parse(const char* data, std::size_t len) {
   if (header_parsed_) {
     // Add the data to the content.
     AppendContent(data, len);
@@ -26,7 +26,7 @@ Error HttpParser::Parse(const char* data, std::size_t len) {
       Finish();
     }
 
-    return kNoError;
+    return true;
   }
 
   pending_data_.append(data, len);
@@ -48,9 +48,8 @@ Error HttpParser::Parse(const char* data, std::size_t len) {
 
     if (!start_line_parsed_) {
       start_line_parsed_ = true;
-      Error error = ParseStartLine(line);
-      if (error != kNoError) {
-        return error;
+      if (!ParseStartLine(line)) {
+        return false;
       }
     } else {
       // Currently, only Content-Length is important to us.
@@ -69,11 +68,11 @@ Error HttpParser::Parse(const char* data, std::size_t len) {
     if (!content_length_parsed_) {
       // No Content-Length, no content. (TODO: Support chucked data)
       Finish();
-      return kNoError;
+      return true;
     } else {
       // Invalid Content-Length in the request.
       if (content_length_ == kInvalidLength) {
-        return kHttpContentLengthError;
+        return false;
       }
     }
 
@@ -88,7 +87,7 @@ Error HttpParser::Parse(const char* data, std::size_t len) {
     pending_data_ = pending_data_.substr(off);
   }
 
-  return kNoError;
+  return true;
 }
 
 void HttpParser::ParseContentLength(const std::string& line) {

src/webcc/http_parser.h

@@ -22,11 +22,10 @@ class HttpParser {
     return finished_;
   }
 
-  Error Parse(const char* data, std::size_t len);
+  bool Parse(const char* data, std::size_t len);
 
  protected:
-  // Parse HTTP start line.
-  virtual Error ParseStartLine(const std::string& line) = 0;
+  virtual bool ParseStartLine(const std::string& line) = 0;
 
   void ParseContentLength(const std::string& line);
 

src/webcc/http_request_parser.cc

@@ -11,20 +11,20 @@ HttpRequestParser::HttpRequestParser(HttpRequest* request)
     : HttpParser(request), request_(request) {
 }
 
-Error HttpRequestParser::ParseStartLine(const std::string& line) {
+bool HttpRequestParser::ParseStartLine(const std::string& line) {
   std::vector<std::string> strs;
   boost::split(strs, line, boost::is_any_of(" "), boost::token_compress_on);
 
   if (strs.size() != 3) {
-    return kHttpStartLineError;
+    return false;
   }
 
   request_->set_method(strs[0]);
   request_->set_url(strs[1]);
 
-  // HTTP version is currently ignored.
+  // HTTP version is ignored.
 
-  return kNoError;
+  return true;
 }
 
 }  // namespace webcc

src/webcc/http_request_parser.h

@@ -16,7 +16,7 @@ class HttpRequestParser : public HttpParser {
   ~HttpRequestParser() override = default;
 
  private:
-  Error ParseStartLine(const std::string& line) override;
+  bool ParseStartLine(const std::string& line) override;
 
   HttpRequest* request_;
 };

src/webcc/http_response_parser.cc

@@ -2,6 +2,7 @@
 
 #include "boost/lexical_cast.hpp"
 
+#include "webcc/logger.h"
 #include "webcc/http_response.h"
 
 namespace webcc {
@@ -10,14 +11,14 @@ HttpResponseParser::HttpResponseParser(HttpResponse* response)
     : HttpParser(response), response_(response) {
 }
 
-Error HttpResponseParser::ParseStartLine(const std::string& line) {
+bool HttpResponseParser::ParseStartLine(const std::string& line) {
   response_->set_start_line(line + "\r\n");
 
   std::size_t off = 0;
 
   std::size_t pos = line.find(' ');
   if (pos == std::string::npos) {
-    return kHttpStartLineError;
+    return false;
   }
 
   // HTTP version
@@ -26,7 +27,7 @@ Error HttpResponseParser::ParseStartLine(const std::string& line) {
 
   pos = line.find(' ', off);
   if (pos == std::string::npos) {
-    return kHttpStartLineError;
+    return false;
   }
 
   // Status code
@@ -35,14 +36,15 @@ Error HttpResponseParser::ParseStartLine(const std::string& line) {
   try {
     response_->set_status(boost::lexical_cast<int>(status_str));
   } catch (boost::bad_lexical_cast&) {
-    return kHttpStartLineError;
+    LOG_ERRO("Invalid HTTP status: %s", status_str.c_str());
+    return false;
   }
 
   if (response_->status() != HttpStatus::kOK) {
-    return kHttpStatusError;
+    return false;
   }
 
-  return kNoError;
+  return true;
 }
 
 }  // namespace webcc

src/webcc/http_response_parser.h

@@ -17,7 +17,7 @@ class HttpResponseParser : public HttpParser {
 
  private:
   // Parse HTTP start line; E.g., "HTTP/1.1 200 OK".
-  Error ParseStartLine(const std::string& line) override;
+  bool ParseStartLine(const std::string& line) override;
 
   // The result response message.
   HttpResponse* response_;

src/webcc/rest_client.cc

@@ -2,13 +2,16 @@
 
 #include "webcc/http_client.h"
 #include "webcc/http_request.h"
-#include "webcc/http_response.h"
 
 namespace webcc {
 
 bool RestClient::Request(const std::string& method,
                          const std::string& url,
                          const std::string& content) {
+  response_.reset();
+  error_ = kNoError;
+  timeout_occurred_ = false;
+
   HttpRequest request;
 
   request.set_method(method);
@@ -24,10 +27,9 @@ bool RestClient::Request(const std::string& method,
   HttpClient http_client;
   http_client.set_timeout_seconds(timeout_seconds_);
 
-  error_ = kNoError;
-
   if (!http_client.Request(request)) {
     error_ = http_client.error();
+    timeout_occurred_ = http_client.timeout_occurred();
     return false;
   }
 

src/webcc/rest_client.h

@@ -1,6 +1,7 @@
 #ifndef WEBCC_REST_CLIENT_H_
 #define WEBCC_REST_CLIENT_H_
 
+#include <cassert>
 #include <string>
 
 #include "webcc/globals.h"
@@ -19,11 +20,21 @@ class RestClient {
   }
 
   HttpResponsePtr response() const { return response_; }
-  int response_status() const { return response_->status(); }
-  const std::string& response_content() const { return response_->content(); }
+
+  int response_status() const {
+    assert(response_);
+    return response_->status();
+  }
+
+  const std::string& response_content() const {
+    assert(response_);
+    return response_->content();
+  }
 
   Error error() const { return error_; }
 
+  bool timeout_occurred() const { return timeout_occurred_; }
+
   bool Get(const std::string& url) {
     return Request(kHttpGet, url, "");
   }
@@ -58,6 +69,9 @@ class RestClient {
   HttpResponsePtr response_;
 
   Error error_ = kNoError;
+
+  // If the error was caused by timeout or not.
+  bool timeout_occurred_ = false;
 };
 
 }  // namespace webcc