From 743f77d3161676c9c62ac648d8d85fac93da6259 Mon Sep 17 00:00:00 2001
From: Dan Goodliffe <dan@randomdan.homeip.net>
Date: Sat, 10 Mar 2018 13:45:40 +0000
Subject: Tighten error handling

---
 service/api.ice                           |  9 +++++++--
 service/apiImpl.cpp                       | 13 +++++++++++--
 service/ex.cpp                            |  6 ++++++
 service/fixtures/filesearching/empty.html |  0
 service/fixtures/filesearching/not.html   |  1 +
 service/test.cpp                          | 12 +++++++++++-
 6 files changed, 36 insertions(+), 5 deletions(-)
 create mode 100644 service/fixtures/filesearching/empty.html
 create mode 100644 service/fixtures/filesearching/not.html

(limited to 'service')

diff --git a/service/api.ice b/service/api.ice
index 994fa28..a8c50e9 100644
--- a/service/api.ice
+++ b/service/api.ice
@@ -9,10 +9,15 @@ module MirrorSearch {
 		string msg;
 	};
 
+	["cpp:ice_print"]
+	exception CurlError {
+		string msg;
+	};
+
 	interface Search {
 		idempotent SearchServices getServices();
-		idempotent SearchHits getMatches(string filename) throws XmlError;
-		idempotent optional(0) string feelingLucky(string filename) throws XmlError;
+		idempotent SearchHits getMatches(string filename) throws XmlError, CurlError;
+		idempotent optional(0) string feelingLucky(string filename) throws XmlError, CurlError;
 	};
 };
 
diff --git a/service/apiImpl.cpp b/service/apiImpl.cpp
index 9795769..42a7918 100644
--- a/service/apiImpl.cpp
+++ b/service/apiImpl.cpp
@@ -42,7 +42,7 @@ namespace MirrorSearch {
 	void
 	curlErrorHandler(const std::string & fn, const char * errbuf, const P & ... p)
 	{
-		throw XmlError(Fmt::get(fn, errbuf, p...));
+		throw CurlError(Fmt::get(fn, errbuf, p...));
 	}
 	template<typename Fmt, typename ... P>
 	auto cEHB(const char * errbuf, const P & ... p)
@@ -97,11 +97,17 @@ namespace MirrorSearch {
 		CESSO(curl, CURLOPT_ACCEPT_ENCODING, "");
 		CESSO(curl, CURLOPT_HTTP_CONTENT_DECODING, 1L);
 		CESSO(curl, CURLOPT_TCP_FASTOPEN, 1L);
+		CESSO(curl, CURLOPT_FAILONERROR, 1L);
 		if (curl_easy_perform(curl.get()) != CURLE_OK) {
 			curlErrorHandler<Read>(failingFunction((void*)&curl_easy_perform), errbuf, url);
 		}
-		BOOST_VERIFY_MSG(ctx, "No ctx and no previous error should never happen.");
+		if (!ctx) {
+			throw CurlError("Did not retrieve any data.");
+		}
 		htmlParseChunk(ctx.get(), "", 0, 1);
+		if (!ctx->myDoc) {
+			throw XmlError("Could not construct a document.");
+		}
 
 		UPtr<xmlDoc> doc = { ctx->myDoc, xmlFreeDoc };
 		return doc;
@@ -128,6 +134,9 @@ namespace MirrorSearch {
 		auto doc = getDoc(s, fn);
 		auto xpathCtx = getXPathCxt(doc);
 		auto xpathObj = getXPathObj(s->listxpath, xpathCtx, xmlXPathObjectType::XPATH_NODESET);
+		if (!xpathObj->nodesetval) {
+			throw XmlError("Nodeset is null");
+		}
 		log->messagebf(LOG::INFO, "%d nodes matched %s", xpathObj->nodesetval->nodeNr, s->listxpath);
 		for (int row = 0; row < xpathObj->nodesetval->nodeNr; row += 1) {
 			xpathCtx->node = xpathObj->nodesetval->nodeTab[row];
diff --git a/service/ex.cpp b/service/ex.cpp
index ec32416..f0e8e2e 100644
--- a/service/ex.cpp
+++ b/service/ex.cpp
@@ -6,5 +6,11 @@ namespace MirrorSearch {
 	{
 		s << msg;
 	}
+
+	void
+	CurlError::ice_print(std::ostream&s) const
+	{
+		s << msg;
+	}
 }
 
diff --git a/service/fixtures/filesearching/empty.html b/service/fixtures/filesearching/empty.html
new file mode 100644
index 0000000..e69de29
diff --git a/service/fixtures/filesearching/not.html b/service/fixtures/filesearching/not.html
new file mode 100644
index 0000000..dde9312
--- /dev/null
+++ b/service/fixtures/filesearching/not.html
@@ -0,0 +1 @@
+This is not XML
diff --git a/service/test.cpp b/service/test.cpp
index 979d43d..5be34e7 100644
--- a/service/test.cpp
+++ b/service/test.cpp
@@ -50,7 +50,17 @@ BOOST_AUTO_TEST_CASE(getServices)
 
 BOOST_AUTO_TEST_CASE(getMatches_failure)
 {
-	BOOST_REQUIRE_THROW(s->getMatches("no.fixture"), MirrorSearch::XmlError);
+	BOOST_REQUIRE_THROW(s->getMatches("no.fixture"), MirrorSearch::CurlError);
+}
+
+BOOST_AUTO_TEST_CASE(getMatches_empty_response)
+{
+	BOOST_REQUIRE_THROW(s->getMatches("empty"), MirrorSearch::CurlError);
+}
+
+BOOST_AUTO_TEST_CASE(getMatches_not_html)
+{
+	BOOST_REQUIRE_THROW(s->getMatches("not"), MirrorSearch::XmlError);
 }
 
 BOOST_AUTO_TEST_CASE(getMatches_zstd_notfound)
-- 
cgit v1.2.3