Graceful handling of download cancel/interruption (#4146)

Author: Klaim

libmamba/include/mamba/core/error_handling.hpp

@@ -63,15 +63,17 @@ namespace mamba
         using base_type = mamba_error;
         using error_list_t = std::vector<mamba_error>;
 
-        explicit mamba_aggregated_error(error_list_t&& error_list);
+        explicit mamba_aggregated_error(error_list_t&& error_list, bool with_bug_report_info = true);
 
         const char* what() const noexcept override;
 
+        bool has_only_error(mamba_error_code code) const;
+
     private:
 
         error_list_t m_error_list;
         mutable std::string m_aggregated_message;
-        static constexpr const char* m_base_message = "Multiple errors occurred:\n";
+        bool m_with_bug_report_message = true;
     };
 
     /********************************

libmamba/include/mamba/core/logging.hpp

@@ -955,6 +955,7 @@ namespace mamba
 
             static void activate_buffer();
             static void deactivate_buffer();
+            static bool is_buffer_enabled();
             static void print_buffer(std::ostream& ostream);
 
         private:

libmamba/include/mamba/download/mirror.hpp

@@ -74,8 +74,11 @@ namespace mamba::download
         std::size_t running_transfers = 0;
         std::size_t successful_transfers = 0;
         std::size_t failed_transfers = 0;
+        std::size_t stopped_transfers = 0;
     };
 
+    static_assert(std::default_initializable<MirrorStats>);
+
     // A Mirror represents a location from where an asset can be downloaded.
     // It handles the generation of required requests to get the asset, and
     // provides some statistics about its usage.
@@ -97,9 +100,11 @@ namespace mamba::download
         request_generator_list
         get_request_generators(const std::string& url_path, const std::string& spec_sha256) const;
 
+        // Note: values from below accessors are obsolete as soon as acquired, because of
+        // concurrency.
+
         std::size_t max_retries() const;
-        std::size_t successful_transfers() const;
-        std::size_t failed_transfers() const;
+        MirrorStats capture_stats() const;
 
         bool can_accept_more_connections() const;
         bool can_retry_with_fewer_connections() const;
@@ -119,8 +124,6 @@ namespace mamba::download
         MirrorID m_id;
         size_t m_max_retries;
 
-        static_assert(std::default_initializable<MirrorStats>);
-
         util::synchronized_value<MirrorStats> m_stats;
     };
 

libmamba/include/mamba/download/request.hpp

@@ -57,8 +57,14 @@ namespace mamba::download
         std::optional<std::size_t> retry_wait_seconds = std::nullopt;
         std::optional<TransferData> transfer = std::nullopt;
         std::size_t attempt_number = std::size_t(1);
+        bool is_stop = false;
     };
 
+    inline auto make_stop_error() -> Error
+    {
+        return { .message = "Stopped by user request", .is_stop = true };
+    }
+
     using Result = tl::expected<Success, Error>;
     using MultiResult = std::vector<Result>;
 
@@ -86,6 +92,7 @@ namespace mamba::download
         // TODO: remove these functions when we plug a library with continuation
         using on_success_callback_t = std::function<expected_t<void>(const Success&)>;
         using on_failure_callback_t = std::function<void(const Error&)>;
+        using on_stopped_callback_t = std::function<void()>;
 
         std::string name;
         // If filename is not initialized, the data will be downloaded
@@ -100,6 +107,7 @@ namespace mamba::download
         std::optional<progress_callback_t> progress = std::nullopt;
         std::optional<on_success_callback_t> on_success = std::nullopt;
         std::optional<on_failure_callback_t> on_failure = std::nullopt;
+        std::optional<on_stopped_callback_t> on_stopped = std::nullopt;
 
     protected:
 

libmamba/src/api/channel_loader.cpp

@@ -240,7 +240,7 @@ namespace mamba
                 error_list.push_back(std::move(error));
                 if (ec == mamba_error_code::user_interrupted)
                 {
-                    return tl::unexpected(mamba_aggregated_error(std::move(error_list)));
+                    return tl::unexpected(mamba_aggregated_error(std::move(error_list), false));
                 }
             }
 

libmamba/src/api/install.cpp

@@ -627,13 +627,13 @@ namespace mamba
             auto maybe_load = load_channels(ctx, channel_context, db, package_caches);
             if (!maybe_load)
             {
-                throw std::runtime_error(maybe_load.error().what());
+                throw maybe_load.error();
             }
 
             auto maybe_prefix_data = PrefixData::create(ctx.prefix_params.target_prefix, channel_context);
             if (!maybe_prefix_data)
             {
-                throw std::runtime_error(maybe_prefix_data.error().what());
+                throw maybe_prefix_data.error();
             }
             PrefixData& prefix_data = maybe_prefix_data.value();
 

libmamba/src/core/error_handling.cpp

@@ -1,3 +1,5 @@
+#include <algorithm>
+
 #include "mamba/core/error_handling.hpp"
 #include "mamba/core/logging.hpp"
 
@@ -55,36 +57,57 @@ namespace mamba
         return m_data;
     }
 
-    constexpr const char* mamba_aggregated_error::m_base_message;  // = "Many errors occurred:\n";
+    namespace
+    {
+        constexpr auto* message_aggregated_error_top = "Multiple errors occurred:\n";
+        constexpr auto* message_aggregated_bug_report = "If you run into this error repeatedly, your package cache may be corrupted.\n"
+                                                        "Please try running `mamba clean -a` to remove this cache before retrying the operation.\n"
+                                                        "\n"
+                                                        "If you still are having issues, please report the error on `mamba-org/mamba`'s issue tracker:\n"
+                                                        "https://github.com/mamba-org/mamba/issues/new?assignees=&labels=&projects=&template=bug.yml";
+
+    }
 
-    mamba_aggregated_error::mamba_aggregated_error(error_list_t&& error_list)
-        : base_type(mamba_aggregated_error::m_base_message, mamba_error_code::aggregated)
+    mamba_aggregated_error::mamba_aggregated_error(error_list_t&& error_list, bool with_bug_report_info)
+        : base_type(message_aggregated_error_top, mamba_error_code::aggregated)
         , m_error_list(std::move(error_list))
         , m_aggregated_message()
+        , m_with_bug_report_message(with_bug_report_info)
     {
     }
 
     const char* mamba_aggregated_error::what() const noexcept
     {
         if (m_aggregated_message.empty())
         {
-            m_aggregated_message = m_base_message;
+            // If we have only one error, don't say it's multiple errors.
+            if (m_error_list.size() > 1)
+            {
+                m_aggregated_message = message_aggregated_error_top;
+            }
 
             for (const mamba_error& er : m_error_list)
             {
                 m_aggregated_message += er.what();
                 m_aggregated_message += "\n";
             }
 
-            m_aggregated_message += "If you run into this error repeatedly, your package cache may be corrupted.\n"
-                                    "Please try running `mamba clean -a` to remove this cache before retrying the operation.\n"
-                                    "\n"
-                                    "If you still are having issues, please report the error on `mamba-org/mamba`'s issue tracker:\n"
-                                    "https://github.com/mamba-org/mamba/issues/new?assignees=&labels=&projects=&template=bug.yml";
+            if (m_with_bug_report_message)
+            {
+                m_aggregated_message += message_aggregated_bug_report;
+            }
         }
         return m_aggregated_message.c_str();
     }
 
+    bool mamba_aggregated_error::has_only_error(const mamba_error_code code) const
+    {
+        return std::ranges::all_of(
+            m_error_list,
+            [&](const auto& err) { return err.error_code() == code; }
+        );
+    }
+
     tl::unexpected<mamba_error> make_unexpected(const char* msg, mamba_error_code ec)
     {
         return tl::make_unexpected(mamba_error(msg, ec));

libmamba/src/core/logging.cpp

@@ -245,6 +245,11 @@ namespace mamba::logging
         details::message_logger_use_buffer = false;
     }
 
+    bool MessageLogger::is_buffer_enabled()
+    {
+        return details::message_logger_use_buffer;
+    }
+
     void MessageLogger::print_buffer(std::ostream& /*out*/)
     {
         details::MessageLoggerBuffer::buffer tmp;

libmamba/src/core/subdir_index.cpp

@@ -557,7 +557,8 @@ namespace mamba
             // missing subdirs (e.g. local path as a `noarch` but no `linux-64`).
             for (auto& result : results)
             {
-                if (!result.has_value())
+                // Don't log if it's a user interruption.
+                if (!result.has_value() and not result.error().is_stop)
                 {
                     LOG_WARNING << "Failed to load subdir: " << result.error().message;
                 }

libmamba/src/core/transaction.cpp

@@ -389,16 +389,36 @@ namespace mamba
             return true;
         }
 
+        // before user confirmation
+        if (is_sig_interrupted())
+        {
+            Console::stream() << "Interrupted by user - stopped before transaction.";
+            return true;
+        }
+
         auto lf = LockFile(ctx.prefix_params.target_prefix / "conda-meta");
         clean_trash_files(ctx.prefix_params.target_prefix, false);
 
+        // after user confirmation or ctrl-c
+        if (is_sig_interrupted())
+        {
+            Console::stream() << "Interrupted by user - stopped before transaction.";
+            return true;
+        }
+
         Console::stream() << "\nTransaction starting";
         fetch_extract_packages(ctx, channel_context);
 
+        if (is_sig_interrupted())
+        {
+            Console::stream() << "Interrupted by user - transaction stopped.";
+            return true;
+        }
+
         if (ctx.download_only)
         {
             Console::stream()
-                << "Download only - packages are downloaded and extracted. Skipping the linking phase.";
+                << "Download only - packages download and extraction is done. Skipping the linking phase.";
             return true;
         }