Printhost: Polish error handling, bugfixes

src/slic3r/GUI/PrintHostDialogs.cpp

@@ -102,7 +102,7 @@ PrintHostQueueDialog::PrintHostQueueDialog(wxWindow *parent)
     job_list->AppendTextColumn("Filename", wxDATAVIEW_CELL_INERT);
 
     auto *btnsizer = new wxBoxSizer(wxHORIZONTAL);
-    auto *btn_cancel = new wxButton(this, wxID_DELETE, _(L("Cancel selected")));
+    auto *btn_cancel = new wxButton(this, wxID_DELETE, _(L("Cancel selected")));   // TODO: enable based on status ("show error" for failed jobs)
     auto *btn_close = new wxButton(this, wxID_CANCEL, _(L("Close")));
     btnsizer->Add(btn_cancel, 0, wxRIGHT, SPACING);
     btnsizer->AddStretchSpacer();
@@ -140,7 +140,13 @@ void PrintHostQueueDialog::on_error(Event &evt)
 {
     wxCHECK_RET(evt.job_id < job_list->GetItemCount(), "Out of bounds access to job list");
 
-    // TODO
+    job_list->SetValue(wxVariant(0), evt.job_id, 1);
+    job_list->SetValue(wxVariant(_(L("Error"))), evt.job_id, 2);
+
+    // TODO: keep the error for repeated display
+
+    auto errormsg = wxString::Format("%s\n%s", _(L("Error uploading to print host:")), evt.error);
+    GUI::show_error(nullptr, std::move(errormsg));
 }
 
 

src/slic3r/Utils/Duet.cpp

@@ -46,7 +46,7 @@ bool Duet::test(wxString &msg) const
 
 wxString Duet::get_test_ok_msg () const
 {
-	return wxString::Format("%s", _(L("Connection to Duet works correctly.")));
+	return _(L("Connection to Duet works correctly."));
 }
 
 wxString Duet::get_test_failed_msg (wxString &msg) const
@@ -135,7 +135,7 @@ wxString Duet::get_test_failed_msg (wxString &msg) const
 // 	return res;
 // }
 
-bool Duet::upload(PrintHostUpload upload_data, Http::ProgressFn prorgess_fn, Http::ErrorFn error_fn) const
+bool Duet::upload(PrintHostUpload upload_data, ProgressFn prorgess_fn, ErrorFn error_fn) const
 {
 	// XXX: TODO
 	throw "unimplemented";

src/slic3r/Utils/Duet.hpp

@@ -22,7 +22,7 @@ public:
 	virtual bool test(wxString &curl_msg) const;
 	virtual wxString get_test_ok_msg () const;
 	virtual wxString get_test_failed_msg (wxString &msg) const;
-	virtual bool upload(PrintHostUpload upload_data, Http::ProgressFn prorgess_fn, Http::ErrorFn error_fn) const;
+	virtual bool upload(PrintHostUpload upload_data, ProgressFn prorgess_fn, ErrorFn error_fn) const;
 	virtual bool has_auto_discovery() const;
 	virtual bool can_test() const;
 	virtual std::string get_host() const { return host; }

src/slic3r/Utils/Http.cpp

@@ -64,6 +64,7 @@ struct Http::priv
 	static int xfercb_legacy(void *userp, double dltotal, double dlnow, double ultotal, double ulnow);
 	static size_t form_file_read_cb(char *buffer, size_t size, size_t nitems, void *userp);
 
+	void set_timeout(long timeout);
 	void form_add_file(const char *name, const fs::path &path, const char* filename);
 	void set_post_body(const fs::path &path);
 
@@ -85,7 +86,7 @@ Http::priv::priv(const std::string &url)
 		throw std::runtime_error(std::string("Could not construct Curl object"));
 	}
 
-	::curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, DEFAULT_TIMEOUT);
+	set_timeout(DEFAULT_TIMEOUT);
 	::curl_easy_setopt(curl, CURLOPT_URL, url.c_str());   // curl makes a copy internally
 	::curl_easy_setopt(curl, CURLOPT_USERAGENT, SLIC3R_FORK_NAME "/" SLIC3R_VERSION);
 	::curl_easy_setopt(curl, CURLOPT_ERRORBUFFER, &error_buffer.front());
@@ -169,6 +170,12 @@ size_t Http::priv::form_file_read_cb(char *buffer, size_t size, size_t nitems, v
 	return stream->gcount();
 }
 
+void Http::priv::set_timeout(long timeout)
+{
+	::curl_easy_setopt(curl, CURLOPT_CONNECTTIMEOUT, timeout);
+	::curl_easy_setopt(curl, CURLOPT_TIMEOUT, timeout);
+}
+
 void Http::priv::form_add_file(const char *name, const fs::path &path, const char* filename)
 {
 	// We can't use CURLFORM_FILECONTENT, because curl doesn't support Unicode filenames on Windows
@@ -205,10 +212,10 @@ void Http::priv::set_post_body(const fs::path &path)
 
 std::string Http::priv::curl_error(CURLcode curlcode)
 {
-	return (boost::format("%1% (%2%): %3%")
+	return (boost::format("%1%:\n%2%\n[Error %3%]")
 		% ::curl_easy_strerror(curlcode)
+		% error_buffer.c_str()
 		% curlcode
-		% error_buffer
 	).str();
 }
 
@@ -298,7 +305,7 @@ Http::~Http()
 Http& Http::timeout(long timeout)
 {
 	if (timeout < 1) { timeout = priv::DEFAULT_TIMEOUT; }
-	if (p) { ::curl_easy_setopt(p->curl, CURLOPT_CONNECTTIMEOUT, timeout); }
+	if (p) { p->set_timeout(timeout); }
 	return *this;
 }
 

src/slic3r/Utils/OctoPrint.cpp

@@ -62,7 +62,7 @@ bool OctoPrint::test(wxString &msg) const
                 const auto text = ptree.get_optional<std::string>("text");
                 res = validate_version_text(text);
                 if (! res) {
-                    msg = wxString::Format("Mismatched type of print host: %s", text ? *text : "OctoPrint");
+                    msg = wxString::Format(_(L("Mismatched type of print host: %s")), text ? *text : "OctoPrint");
                 }
             }
             catch (...) {
@@ -77,28 +77,24 @@ bool OctoPrint::test(wxString &msg) const
 
 wxString OctoPrint::get_test_ok_msg () const
 {
-    return wxString::Format("%s", _(L("Connection to OctoPrint works correctly.")));
+    return _(L("Connection to OctoPrint works correctly."));
 }
 
 wxString OctoPrint::get_test_failed_msg (wxString &msg) const
 {
     return wxString::Format("%s: %s\n\n%s",
-                        _(L("Could not connect to OctoPrint")), msg, _(L("Note: OctoPrint version at least 1.1.0 is required.")));
+        _(L("Could not connect to OctoPrint")), msg, _(L("Note: OctoPrint version at least 1.1.0 is required.")));
 }
 
-bool OctoPrint::upload(PrintHostUpload upload_data, Http::ProgressFn prorgess_fn, Http::ErrorFn error_fn) const
+bool OctoPrint::upload(PrintHostUpload upload_data, ProgressFn prorgess_fn, ErrorFn error_fn) const
 {
     const auto upload_filename = upload_data.upload_path.filename();
     const auto upload_parent_path = upload_data.upload_path.parent_path();
 
     wxString test_msg;
     if (! test(test_msg)) {
-
+        error_fn(std::move(test_msg));
-        // FIXME:
+        return false;
-
-        // auto errormsg = wxString::Format("%s: %s", errortitle, test_msg);
-        // GUI::show_error(&progress_dialog, std::move(errormsg));
-        // return false;
     }
 
     bool res = true;
@@ -122,7 +118,8 @@ bool OctoPrint::upload(PrintHostUpload upload_data, Http::ProgressFn prorgess_fn
         })
         .on_error([&](std::string body, std::string error, unsigned status) {
             BOOST_LOG_TRIVIAL(error) << boost::format("Octoprint: Error uploading file: %1%, HTTP %2%, body: `%3%`") % error % status % body;
-            error_fn(std::move(body), std::move(error), status);
+            // error_fn(std::move(body), std::move(error), status);
+            error_fn(format_error(body, error, status));
             res = false;
         })
         .on_progress([&](Http::Progress progress, bool &cancel) {
@@ -192,7 +189,7 @@ SLAHost::~SLAHost() {}
 
 wxString SLAHost::get_test_ok_msg () const
 {
-    return wxString::Format("%s", _(L("Connection to Prusa SLA works correctly.")));
+    return _(L("Connection to Prusa SLA works correctly."));
 }
 
 wxString SLAHost::get_test_failed_msg (wxString &msg) const

src/slic3r/Utils/OctoPrint.hpp

@@ -23,7 +23,7 @@ public:
     virtual bool test(wxString &curl_msg) const;
     virtual wxString get_test_ok_msg () const;
     virtual wxString get_test_failed_msg (wxString &msg) const;
-    virtual bool upload(PrintHostUpload upload_data, Http::ProgressFn prorgess_fn, Http::ErrorFn error_fn) const;
+    virtual bool upload(PrintHostUpload upload_data, ProgressFn prorgess_fn, ErrorFn error_fn) const;
     virtual bool has_auto_discovery() const;
     virtual bool can_test() const;
     virtual std::string get_host() const { return host; }

src/slic3r/Utils/PrintHost.cpp

@@ -2,10 +2,12 @@
 
 #include <vector>
 #include <thread>
+#include <exception>
 #include <boost/optional.hpp>
 #include <boost/log/trivial.hpp>
 #include <boost/filesystem.hpp>
 
+#include <wx/string.h>
 #include <wx/app.h>
 
 #include "libslic3r/PrintConfig.hpp"
@@ -58,7 +60,6 @@ struct PrintHostJobQueue::priv
     void start_bg_thread();
     void bg_thread_main();
     void progress_fn(Http::Progress progress, bool &cancel);
-    void error_fn(std::string body, std::string error, unsigned http_status);
     void perform_job(PrintHostJob the_job);
 };
 
@@ -100,6 +101,9 @@ void PrintHostJobQueue::priv::bg_thread_main()
             }
             job_id++;
         }
+    } catch (const std::exception &e) {
+        auto evt = new PrintHostQueueDialog::Event(GUI::EVT_PRINTHOST_ERROR, queue_dialog->GetId(), job_id, e.what());
+        wxQueueEvent(queue_dialog, evt);
     } catch (...) {
         wxTheApp->OnUnhandledException();
     }
@@ -136,28 +140,28 @@ void PrintHostJobQueue::priv::progress_fn(Http::Progress progress, bool &cancel)
     }
 }
 
-void PrintHostJobQueue::priv::error_fn(std::string body, std::string error, unsigned http_status)
-{
-    // TODO
-}
-
 void PrintHostJobQueue::priv::perform_job(PrintHostJob the_job)
 {
     if (bg_exit || the_job.empty()) { return; }
 
-    BOOST_LOG_TRIVIAL(debug) << boost::format("PrintHostJobQueue/bg_thread: Got job: `%1%` -> `%1%`")
+    BOOST_LOG_TRIVIAL(debug) << boost::format("PrintHostJobQueue/bg_thread: Got job: `%1%` -> `%2%`")
         % the_job.upload_data.upload_path
         % the_job.printhost->get_host();
 
     const fs::path gcode_path = the_job.upload_data.source_path;
 
-    the_job.printhost->upload(std::move(the_job.upload_data),
+    bool success = the_job.printhost->upload(std::move(the_job.upload_data),
         [this](Http::Progress progress, bool &cancel) { this->progress_fn(std::move(progress), cancel); },
-        [this](std::string body, std::string error, unsigned http_status) { this->error_fn(std::move(body), std::move(error), http_status); }
+        [this](wxString error) {
+            auto evt = new PrintHostQueueDialog::Event(GUI::EVT_PRINTHOST_ERROR, queue_dialog->GetId(), job_id, std::move(error));
+            wxQueueEvent(queue_dialog, evt);
+        }
     );
 
-    auto evt = new PrintHostQueueDialog::Event(GUI::EVT_PRINTHOST_PROGRESS, queue_dialog->GetId(), job_id, 100);
+    if (success) {
-    wxQueueEvent(queue_dialog, evt);
+        auto evt = new PrintHostQueueDialog::Event(GUI::EVT_PRINTHOST_PROGRESS, queue_dialog->GetId(), job_id, 100);
+        wxQueueEvent(queue_dialog, evt);
+    }
 
     boost::system::error_code ec;
     fs::remove(gcode_path, ec);

src/slic3r/Utils/PrintHost.hpp

@@ -3,6 +3,7 @@
 
 #include <memory>
 #include <string>
+#include <functional>
 #include <boost/filesystem/path.hpp>
 
 #include <wx/string.h>
@@ -28,10 +29,13 @@ class PrintHost
 public:
     virtual ~PrintHost();
 
+    typedef Http::ProgressFn ProgressFn;
+    typedef std::function<void(wxString /* error */)> ErrorFn;
+
     virtual bool test(wxString &curl_msg) const = 0;
     virtual wxString get_test_ok_msg () const = 0;
     virtual wxString get_test_failed_msg (wxString &msg) const = 0;
-    virtual bool upload(PrintHostUpload upload_data, Http::ProgressFn prorgess_fn, Http::ErrorFn error_fn) const = 0;
+    virtual bool upload(PrintHostUpload upload_data, ProgressFn prorgess_fn, ErrorFn error_fn) const = 0;
     virtual bool has_auto_discovery() const = 0;
     virtual bool can_test() const = 0;
     virtual std::string get_host() const = 0;