diff --git a/src/slic3r/Utils/PrintHost.cpp b/src/slic3r/Utils/PrintHost.cpp index 41241abb10..2eeb78a975 100644 --- a/src/slic3r/Utils/PrintHost.cpp +++ b/src/slic3r/Utils/PrintHost.cpp @@ -320,12 +320,7 @@ void PrintHostJobQueue::priv::bg_thread_main() % job.cancelled; if (! job.cancelled) { - // A failing job must not stop the worker, or later jobs would stay queued forever. - try { - perform_job(std::move(job)); - } catch (const std::exception &e) { - emit_error(e.what()); - } + perform_job(std::move(job)); } remove_source(); @@ -437,12 +432,10 @@ void PrintHostJobQueue::priv::remove_source() source_to_remove.clear(); } -void PrintHostJobQueue::priv::perform_job(PrintHostJob the_job) +bool PrintHostJobQueue::upload_job(PrintHostJob &job, PrintHost::ProgressFn progress_fn, PrintHost::ErrorFn error_fn, PrintHost::InfoFn info_fn) { - emit_progress(0); // Indicate the upload is starting - // Captured before upload_data is moved into upload() below. - const std::string upload_filename = the_job.upload_data.source_path.filename().string(); + const std::string upload_filename = job.upload_data.source_path.filename().string(); { LifecycleEventContext ctx; @@ -451,19 +444,37 @@ void PrintHostJobQueue::priv::perform_job(PrintHostJob the_job) fire_lifecycle_event(LifecycleEvent::UploadStarted, ctx); } - 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](wxString error) { this->error_fn(std::move(error)); }, - [this](wxString tag, wxString host) { this->info_fn(std::move(tag), std::move(host)); } - ); + bool success = false; + std::string error; + // A throwing upload must not stop the worker, or later jobs would stay queued forever. + try { + success = job.printhost->upload(std::move(job.upload_data), std::move(progress_fn), error_fn, std::move(info_fn)); + } catch (const std::exception &e) { + error = e.what(); + error_fn(error); + } { LifecycleEventContext ctx; ctx.name = upload_filename; ctx.code = success ? LifecycleEvtCode::Ok : LifecycleEvtCode::Error; + ctx.msg = error; fire_lifecycle_event(LifecycleEvent::UploadFinished, ctx); } + return success; +} + +void PrintHostJobQueue::priv::perform_job(PrintHostJob the_job) +{ + emit_progress(0); // Indicate the upload is starting + + bool success = PrintHostJobQueue::upload_job(the_job, + [this](Http::Progress progress, bool &cancel) { this->progress_fn(std::move(progress), cancel); }, + [this](wxString error) { this->error_fn(std::move(error)); }, + [this](wxString tag, wxString host) { this->info_fn(std::move(tag), std::move(host)); } + ); + if (success) { emit_progress(100); if (the_job.switch_to_device_tab) { diff --git a/src/slic3r/Utils/PrintHost.hpp b/src/slic3r/Utils/PrintHost.hpp index ed86f213e7..cf197ede07 100644 --- a/src/slic3r/Utils/PrintHost.hpp +++ b/src/slic3r/Utils/PrintHost.hpp @@ -152,6 +152,10 @@ public: void enqueue(PrintHostJob job); void cancel(size_t id); + // Uploads the job, firing UploadStarted and a matching UploadFinished. An exception thrown by + // the upload is reported through error_fn and makes the upload fail. + static bool upload_job(PrintHostJob &job, PrintHost::ProgressFn progress_fn, PrintHost::ErrorFn error_fn, PrintHost::InfoFn info_fn); + private: struct priv; std::shared_ptr p; diff --git a/tests/slic3rutils/test_printhost.cpp b/tests/slic3rutils/test_printhost.cpp index a102d83770..c4df56ebb6 100644 --- a/tests/slic3rutils/test_printhost.cpp +++ b/tests/slic3rutils/test_printhost.cpp @@ -1,10 +1,13 @@ #include +#include +#include #include #include #include +#include "libslic3r/LifecycleEvents.hpp" #include "slic3r/Utils/PrintHost.hpp" #include "slic3r/Utils/Flashforge.hpp" @@ -28,6 +31,41 @@ public: std::string get_host() const override { return {}; } }; +class ThrowingPrintHost : public TestPrintHost +{ +public: + bool upload(PrintHostUpload, ProgressFn, ErrorFn, InfoFn) const override { throw std::runtime_error("reply could not be read"); } +}; + +struct UploadEvents +{ + std::vector events; + std::vector codes; + std::vector errors; + bool uploaded{false}; + + explicit UploadEvents(std::unique_ptr host) + { + set_lifecycle_hook_fn([this](LifecycleEvent event, const LifecycleEventContext& ctx) { + events.push_back(event); + codes.push_back(ctx.code); + }); + + PrintHostJob job; + job.printhost = std::move(host); + job.upload_data.source_path = "plate.gcode"; + try { + uploaded = PrintHostJobQueue::upload_job(job, [](Http::Progress, bool&) {}, + [this](wxString error) { errors.push_back(error.ToStdString()); }, + [](wxString, wxString) {}); + } catch (...) { + set_lifecycle_hook_fn(nullptr); + throw; + } + set_lifecycle_hook_fn(nullptr); + } +}; + std::string format_error(const std::string& body, const std::string& error, unsigned status) { return TestPrintHost().format_error(body, error, status).ToStdString(); @@ -314,3 +352,23 @@ TEST_CASE("Flashforge material slots reject a reply that is not JSON", "[PrintHo CHECK_FALSE(ok); CHECK(slots.empty()); } + +TEST_CASE("An upload that throws still finishes with an error", "[PrintHost][LifecycleEvents]") +{ + UploadEvents run(std::make_unique()); + + CHECK_FALSE(run.uploaded); + CHECK(run.errors == std::vector{"reply could not be read"}); + CHECK(run.events == std::vector{LifecycleEvent::UploadStarted, LifecycleEvent::UploadFinished}); + CHECK(run.codes == std::vector{LifecycleEvtCode::Ok, LifecycleEvtCode::Error}); +} + +TEST_CASE("A successful upload finishes without an error", "[PrintHost][LifecycleEvents]") +{ + UploadEvents run(std::make_unique()); + + CHECK(run.uploaded); + CHECK(run.errors.empty()); + CHECK(run.events == std::vector{LifecycleEvent::UploadStarted, LifecycleEvent::UploadFinished}); + CHECK(run.codes == std::vector{LifecycleEvtCode::Ok, LifecycleEvtCode::Ok}); +}