Change error handling in HttpClient

src/cpp-utils/network/CurlHttpClient.cpp

@@ -45,7 +45,7 @@ namespace cpputils {
         curl_easy_cleanup(curl);
     }
 
-    optional <string> CurlHttpClient::get(const string &url, optional<long> timeoutMsec) {
+    string CurlHttpClient::get(const string &url, optional<long> timeoutMsec) {
         curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
         // example.com is redirected, so we tell libcurl to follow redirection
         curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
@@ -61,7 +61,7 @@ namespace cpputils {
         CURLcode res = curl_easy_perform(curl);
         // Check for errors
         if (res != CURLE_OK) {
-            return none;
+			throw std::runtime_error("Curl Error " + std::to_string(res) + ": " + curl_easy_strerror(res));
         }
         return out.str();
     }

src/cpp-utils/network/CurlHttpClient.h

@@ -15,7 +15,7 @@ namespace cpputils {
 
         ~CurlHttpClient();
 
-        boost::optional <std::string> get(const std::string &url, boost::optional<long> timeoutMsec = boost::none) override;
+        std::string get(const std::string &url, boost::optional<long> timeoutMsec = boost::none) override;
 
     private:
 		// When the first object of this class is created, it will initialize curl using curl_global_init().

src/cpp-utils/network/FakeHttpClient.cpp

@@ -12,11 +12,11 @@ namespace cpputils {
         _sites[url] = content;
     }
 
-    optional<string> FakeHttpClient::get(const string &url, optional<long> timeoutMsec) {
+    string FakeHttpClient::get(const string &url, optional<long> timeoutMsec) {
         UNUSED(timeoutMsec);
         auto found = _sites.find(url);
         if (found == _sites.end()) {
-            return none;
+			throw std::runtime_error("Website doesn't exist in FakeHttpClient.");
         }
         return found->second;
     }

src/cpp-utils/network/FakeHttpClient.h

@@ -13,7 +13,7 @@ namespace cpputils {
 
         void addWebsite(const std::string &url, const std::string &content);
 
-        boost::optional<std::string> get(const std::string &url, boost::optional<long> timeoutMsec = boost::none) override;
+        std::string get(const std::string &url, boost::optional<long> timeoutMsec = boost::none) override;
 
     private:
         std::map<std::string, std::string> _sites;

src/cpp-utils/network/HttpClient.h

@@ -9,7 +9,7 @@ namespace cpputils {
     public:
         virtual ~HttpClient() {}
 
-        virtual boost::optional<std::string> get(const std::string& url, boost::optional<long> timeoutMsec = boost::none) = 0;
+        virtual std::string get(const std::string& url, boost::optional<long> timeoutMsec = boost::none) = 0;
     };
 };
 

src/cryfs-cli/VersionChecker.cpp

@@ -16,8 +16,7 @@ using namespace cpputils::logging;
 namespace cryfs {
 
     VersionChecker::VersionChecker(HttpClient* httpClient)
-            : _versionInfo(_getVersionInfo(httpClient)) {
+            : _versionInfo(_getVersionInfo(httpClient)) {}
-    }
 
     optional<string> VersionChecker::newestVersion() const {
         if (_versionInfo == none) {
@@ -48,11 +47,15 @@ namespace cryfs {
 
     optional<ptree> VersionChecker::_getVersionInfo(HttpClient* httpClient) {
         long timeoutMsec = 2000;
-        optional<string> response = httpClient->get("https://www.cryfs.org/version_info.json", timeoutMsec);
+		string response;
-        if (response == none) {
+		try {
-            return none;
+			response = httpClient->get("https://www.cryfs.org/version_info.json", timeoutMsec);
-        }
+		}
-        return _parseJson(*response);
+		catch (const std::exception& e) {
+			LOG(WARN, "HTTP Error: {}", e.what());
+			return none;
+		}
+        return _parseJson(response);
     }
 
     optional<ptree> VersionChecker::_parseJson(const string &json) {

test/cpp-utils/network/FakeHttpClientTest.cpp

@@ -7,33 +7,33 @@ using boost::none;
 using namespace cpputils;
 
 TEST(FakeHttpClientTest, Empty) {
-        EXPECT_EQ(none, FakeHttpClient().get("http://example.com"));
+        EXPECT_ANY_THROW(FakeHttpClient().get("http://example.com"));
 }
 
 TEST(FakeHttpClientTest, Nonexisting) {
         FakeHttpClient client;
         client.addWebsite("http://existing.com", "content");
-        EXPECT_EQ(none, client.get("http://notexisting.com"));
+        EXPECT_ANY_THROW(client.get("http://notexisting.com"));
 }
 
 TEST(FakeHttpClientTest, Existing) {
         FakeHttpClient client;
         client.addWebsite("http://existing.com", "content");
-        EXPECT_EQ("content", client.get("http://existing.com").value());
+        EXPECT_EQ("content", client.get("http://existing.com"));
 }
 
 TEST(FakeHttpClientTest, TwoExisting) {
         FakeHttpClient client;
         client.addWebsite("http://firstexisting.com", "first_content");
         client.addWebsite("http://secondexisting.com", "second_content");
-        EXPECT_EQ("first_content", client.get("http://firstexisting.com").value());
+        EXPECT_EQ("first_content", client.get("http://firstexisting.com"));
-        EXPECT_EQ("second_content", client.get("http://secondexisting.com").value());
+        EXPECT_EQ("second_content", client.get("http://secondexisting.com"));
-        EXPECT_EQ(none, client.get("http://notexisting.com"));
+		EXPECT_ANY_THROW(client.get("http://notexisting.com"));
 }
 
 TEST(FakeHttpClientTest, Overwriting) {
         FakeHttpClient client;
         client.addWebsite("http://existing.com", "content");
         client.addWebsite("http://existing.com", "new_content");
-        EXPECT_EQ("new_content", client.get("http://existing.com").value());
+        EXPECT_EQ("new_content", client.get("http://existing.com"));
 }