Octoprint: Improve error reporting

4 files changed, 29 insertions, 20 deletions

diff --git a/xs/src/slic3r/Utils/Http.cpp b/xs/src/slic3r/Utils/Http.cpp
index 47021d39f..1a202b2ad 100644
--- a/xs/src/slic3r/Utils/Http.cpp
+++ b/xs/src/slic3r/Utils/Http.cpp
@@ -201,7 +201,6 @@ std::string Http::priv::body_size_error()
 
 void Http::priv::http_perform()
 {
-	::curl_easy_setopt(curl, CURLOPT_FAILONERROR, 1L);
 	::curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
 	::curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, writecb);
 	::curl_easy_setopt(curl, CURLOPT_WRITEDATA, static_cast<void*>(this));
@@ -230,8 +229,6 @@ void Http::priv::http_perform()
 	}
 
 	CURLcode res = ::curl_easy_perform(curl);
-	long http_status = 0;
-	::curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_status);
 
 	if (res != CURLE_OK) {
 		if (res == CURLE_ABORTED_BY_CALLBACK) {
@@ -242,17 +239,22 @@ void Http::priv::http_perform()
 				if (progressfn) { progressfn(dummyprogress, cancel); }
 			} else {
 				// The abort comes from the CURLOPT_READFUNCTION callback, which means reading file failed
-				if (errorfn) { errorfn(std::move(buffer), "Error reading file for file upload", http_status); }
+				if (errorfn) { errorfn(std::move(buffer), "Error reading file for file upload", 0); }
 			}
 		}
 		else if (res == CURLE_WRITE_ERROR) {
-			if (errorfn) { errorfn(std::move(buffer), body_size_error(), http_status); }
+			if (errorfn) { errorfn(std::move(buffer), body_size_error(), 0); }
 		} else {
-			if (errorfn) { errorfn(std::move(buffer), curl_error(res), http_status); }
+			if (errorfn) { errorfn(std::move(buffer), curl_error(res), 0); }
 		};
 	} else {
-		if (completefn) {
-			completefn(std::move(buffer), http_status);
+		long http_status = 0;
+		::curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &http_status);
+		
+		if (http_status >= 400) {
+			if (errorfn) { errorfn(std::move(buffer), std::string(), http_status); }
+		} else {
+			if (completefn) { completefn(std::move(buffer), http_status); }
 		}
 	}
 }
diff --git a/xs/src/slic3r/Utils/Http.hpp b/xs/src/slic3r/Utils/Http.hpp
index 73656bf88..0724da9ee 100644
--- a/xs/src/slic3r/Utils/Http.hpp
+++ b/xs/src/slic3r/Utils/Http.hpp
@@ -29,7 +29,16 @@ public:
 
 	typedef std::shared_ptr<Http> Ptr;
 	typedef std::function<void(std::string /* body */, unsigned /* http_status */)> CompleteFn;
+	
+	// A HTTP request may fail at various stages of completeness (URL parsing, DNS lookup, TCP connection, ...).
+	// If the HTTP request could not be made or failed before completion, the `error` arg contains a description
+	// of the error and `http_status` is zero.
+	// If the HTTP request was completed but the response HTTP code is >= 400, `error` is empty and `http_status` contains the response code.
+	// In either case there may or may not be a body.
 	typedef std::function<void(std::string /* body */, std::string /* error */, unsigned /* http_status */)> ErrorFn;
+
+	// See the Progress struct above.
+	// Writing true to the `cancel` reference cancels the request in progress.
 	typedef std::function<void(Progress, bool& /* cancel */)> ProgressFn;
 
 	Http(Http &&other);
diff --git a/xs/src/slic3r/Utils/OctoPrint.cpp b/xs/src/slic3r/Utils/OctoPrint.cpp
index 86049de16..97b4123d4 100644
--- a/xs/src/slic3r/Utils/OctoPrint.cpp
+++ b/xs/src/slic3r/Utils/OctoPrint.cpp
@@ -78,10 +78,10 @@ bool OctoPrint::test(wxString &msg) const
 
 	auto http = Http::get(std::move(url));
 	set_auth(http);
-	http.on_error([&](std::string, std::string error, unsigned status) {
-			BOOST_LOG_TRIVIAL(error) << boost::format("Octoprint: Error getting version: %1% (HTTP %2%)") % error % status;
+	http.on_error([&](std::string body, std::string error, unsigned status) {
+			BOOST_LOG_TRIVIAL(error) << boost::format("Octoprint: Error getting version: %1%, HTTP %2%, body: `%3%`") % error % status % body;
 			res = false;
-			msg = format_error(error, status);
+			msg = format_error(body, error, status);
 		})
 		.on_complete([&](std::string body, unsigned) {
 			BOOST_LOG_TRIVIAL(debug) << boost::format("Octoprint: Got version: %1%") % body;
@@ -140,8 +140,8 @@ bool OctoPrint::send_gcode(const std::string &filename) const
 			progress_dialog.Update(PROGRESS_RANGE);
 		})
 		.on_error([&](std::string body, std::string error, unsigned status) {
-			BOOST_LOG_TRIVIAL(error) << boost::format("Octoprint: Error uploading file: %1% (HTTP %2%)") % error % status;
-			auto errormsg = wxString::Format("%s: %s", errortitle, format_error(error, status));
+			BOOST_LOG_TRIVIAL(error) << boost::format("Octoprint: Error uploading file: %1%, HTTP %2%, body: `%3%`") % error % status % body;
+			auto errormsg = wxString::Format("%s: %s", errortitle, format_error(body, error, status));
 			GUI::show_error(&progress_dialog, std::move(errormsg));
 			res = false;
 		})
@@ -183,15 +183,13 @@ std::string OctoPrint::make_url(const std::string &path) const
 	}
 }
 
-wxString OctoPrint::format_error(const std::string &error, unsigned status)
+wxString OctoPrint::format_error(const std::string &body, const std::string &error, unsigned status)
 {
-	auto wxerror = wxString::FromUTF8(error.data());
-
 	if (status != 0) {
-		return wxString::Format("HTTP %u: %s", status,
-			(status == 401 ? _(L("Invalid API key")) : wxerror));
+		auto wxbody = wxString::FromUTF8(body.data());
+		return wxString::Format("HTTP %u: %s", status, wxbody);
 	} else {
-		return wxerror;
+		return wxString::FromUTF8(error.data());
 	}
 }
 
diff --git a/xs/src/slic3r/Utils/OctoPrint.hpp b/xs/src/slic3r/Utils/OctoPrint.hpp
index a8599faa9..1e2098ae3 100644
--- a/xs/src/slic3r/Utils/OctoPrint.hpp
+++ b/xs/src/slic3r/Utils/OctoPrint.hpp
@@ -26,7 +26,7 @@ private:
 
 	void set_auth(Http &http) const;
 	std::string make_url(const std::string &path) const;
-	static wxString format_error(const std::string &error, unsigned status);
+	static wxString format_error(const std::string &body, const std::string &error, unsigned status);
 };