From a8915ecd610a4e51cdac207a3a4a4d264de9b9ef Mon Sep 17 00:00:00 2001 From: Tobias Christiansen Date: Wed, 14 Apr 2021 20:15:28 +0200 Subject: [PATCH] FileManager+FileOperation: Report Errors when doing an file operation Report back errors from the FileOperation to the FileManager and display them in the MessageBox --- .../FileOperationProgressWidget.cpp | 17 +++++++++--- .../FileManager/FileOperationProgressWidget.h | 2 +- Userland/Services/FileOperation/main.cpp | 26 ++++++++++++++----- 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/Userland/Applications/FileManager/FileOperationProgressWidget.cpp b/Userland/Applications/FileManager/FileOperationProgressWidget.cpp index 2e3593332b..5ca27d75d3 100644 --- a/Userland/Applications/FileManager/FileOperationProgressWidget.cpp +++ b/Userland/Applications/FileManager/FileOperationProgressWidget.cpp @@ -63,12 +63,23 @@ FileOperationProgressWidget::FileOperationProgressWidget(NonnullRefPtron_ready_to_read = [this] { auto line = m_helper_pipe->read_line(); if (line.is_null()) { - did_error(); + did_error("Read from pipe returned null."); return; } + auto parts = line.split_view(' '); VERIFY(!parts.is_empty()); + if (parts[0] == "ERROR"sv) { + did_error(line.substring(6)); + return; + } + + if (parts[0] == "WARN"sv) { + did_error(line.substring(5)); + return; + } + if (parts[0] == "FINISH"sv) { did_finish(); return; @@ -101,11 +112,11 @@ void FileOperationProgressWidget::did_finish() window()->close(); } -void FileOperationProgressWidget::did_error() +void FileOperationProgressWidget::did_error(const String message) { // FIXME: Communicate more with the user about errors. close_pipe(); - GUI::MessageBox::show(window(), "An error occurred", "Error", GUI::MessageBox::Type::Error, GUI::MessageBox::InputType::OK); + GUI::MessageBox::show(window(), String::formatted("An error occurred: {}", message), "Error", GUI::MessageBox::Type::Error, GUI::MessageBox::InputType::OK); window()->close(); } diff --git a/Userland/Applications/FileManager/FileOperationProgressWidget.h b/Userland/Applications/FileManager/FileOperationProgressWidget.h index fcebebb02e..157752c4d8 100644 --- a/Userland/Applications/FileManager/FileOperationProgressWidget.h +++ b/Userland/Applications/FileManager/FileOperationProgressWidget.h @@ -41,7 +41,7 @@ private: explicit FileOperationProgressWidget(NonnullRefPtr helper_pipe); void did_finish(); - void did_error(); + void did_error(String message); void did_progress(off_t bytes_done, off_t total_byte_count, size_t files_done, size_t total_file_count, off_t current_file_done, off_t current_file_size, const StringView& current_file_name); void close_pipe(); diff --git a/Userland/Services/FileOperation/main.cpp b/Userland/Services/FileOperation/main.cpp index 28a32649c0..67cf47dab3 100644 --- a/Userland/Services/FileOperation/main.cpp +++ b/Userland/Services/FileOperation/main.cpp @@ -33,6 +33,8 @@ #include static int perform_copy(const String& source, const String& destination); +static void report_error(String message); +static void report_warning(String message); int main(int argc, char** argv) { @@ -49,7 +51,7 @@ int main(int argc, char** argv) if (operation == "Copy") return perform_copy(source, destination); - warnln("Unknown operation '{}'", operation); + report_warning(String::formatted("Unknown operation '{}'", operation)); return 0; } @@ -64,11 +66,22 @@ struct WorkItem { off_t size; }; +static void report_error(String message) +{ + outln("ERROR {}", message); +} + +static void report_warning(String message) +{ + outln("WARN {}", message); +} + static bool collect_work_items(const String& source, const String& destination, Vector& items) { struct stat st = {}; if (stat(source.characters(), &st) < 0) { - perror("stat"); + auto original_errno = errno; + report_error(String::formatted("stat: {}", strerror(original_errno))); return false; } @@ -127,7 +140,8 @@ int perform_copy(const String& source, const String& destination) if (item.type == WorkItem::Type::CreateDirectory) { outln("MKDIR {}", item.destination); if (mkdir(item.destination.characters(), 0755) < 0 && errno != EEXIST) { - perror("mkdir"); + auto original_errno = errno; + report_error(String::formatted("mkdir: {}", strerror(original_errno))); return 1; } continue; @@ -135,12 +149,12 @@ int perform_copy(const String& source, const String& destination) VERIFY(item.type == WorkItem::Type::CopyFile); auto source_file_or_error = Core::File::open(item.source, Core::File::ReadOnly); if (source_file_or_error.is_error()) { - warnln("Failed to open {} for reading: {}", item.source, source_file_or_error.error()); + report_warning(String::formatted("Failed to open {} for reading: {}", item.source, source_file_or_error.error())); return 1; } auto destination_file_or_error = Core::File::open(item.destination, (Core::IODevice::OpenMode)(Core::File::WriteOnly | Core::File::Truncate)); if (destination_file_or_error.is_error()) { - warnln("Failed to open {} for write: {}", item.destination, destination_file_or_error.error()); + report_warning(String::formatted("Failed to open {} for write: {}", item.destination, destination_file_or_error.error())); return 1; } auto& source_file = *source_file_or_error.value(); @@ -152,7 +166,7 @@ int perform_copy(const String& source, const String& destination) if (buffer.is_null()) break; if (!destination_file.write(buffer)) { - warnln("Failed to write to destination file: {}", destination_file.error_string()); + report_warning(String::formatted("Failed to write to destination file: {}", destination_file.error_string())); return 1; } item_done += buffer.size();