TerraSync: report errors during downloading

Report various out-of-memory and IO failure conditions; especially,
failure to write downloaded to disk, which was previously not 
recorded.

simgear/debug/ErrorReportingCallback.hxx

@@ -35,18 +35,25 @@ using ErrorReportCallback = std::function<void(const std::string& msg, const std
 
 void setErrorReportCallback(ErrorReportCallback cb);
 
+/** kinds of failures we can report. This is *how* (or why) something failed. Extend
+  as necessary but update the correponsdings string translations if you do. More detail isn't
+ necessariyl useful here: better to provide that in the 'details' string
+ */
 enum class LoadFailure {
     Unknown,
     NotFound,
     OutOfMemory,
     BadHeader,
     BadData,
-    Misconfigured
+    Misconfigured,
+    IOError, // disk full, permissions error, etc
+    NetworkError
 };
 
 /**
  @brief enum of the operations which can fail. This should be extended as necessary: it maps to
- translated error messages for the user.
+ translated error messages for the user. This describes what failed, the enum above gives why/how. The
+ combination of what+why should be something at the user level: use details for debug-level information.
  */
 enum class ErrorCode {
     LoadEffectsShaders,
@@ -58,9 +65,10 @@ enum class ErrorCode {
     GUIDialog,
     AudioFX,
     XMLLoadCommand,
-    AircraftSystems,
+    AircraftSystems, // autopilot, hydrualics, instruments
     InputDeviceConfig,
-    AITrafficSchedule
+    AITrafficSchedule,
+    TerraSync
 };
 /**
  @brief Define an error-reporting context value, for the duration of this

simgear/io/HTTPRepository.cxx

@@ -35,11 +35,12 @@
 #include "simgear/debug/logstream.hxx"
 #include "simgear/misc/strutils.hxx"
 
-#include <simgear/misc/sg_dir.hxx>
+#include <simgear/debug/ErrorReportingCallback.hxx>
 #include <simgear/io/HTTPClient.hxx>
+#include <simgear/io/iostreams/sgstream.hxx>
 #include <simgear/io/sg_file.hxx>
 #include <simgear/io/untar.hxx>
-#include <simgear/io/iostreams/sgstream.hxx>
+#include <simgear/misc/sg_dir.hxx>
 #include <simgear/structure/exception.hxx>
 #include <simgear/timing/timestamp.hxx>
 
@@ -218,8 +219,7 @@ public:
             return;
         }
 
-        char* buf = nullptr;
+        std::string buf;
-        size_t bufSize = 0;
 
         for (auto& child : children) {
             if (child.type != HTTPRepository::FileType)
@@ -240,17 +240,34 @@ public:
             src.open(SG_IO_IN);
             dst.open(SG_IO_OUT);
 
-            if (bufSize < cp.sizeInBytes()) {
+            const auto sizeToCopy = cp.sizeInBytes();
-                bufSize = cp.sizeInBytes();
+            if (buf.size() < sizeToCopy) {
-                free(buf);
+                try {
-                buf = (char*)malloc(bufSize);
+                    buf.resize(sizeToCopy);
-                if (!buf) {
+                } catch (std::bad_alloc) {
-                    continue;
+                    simgear::reportFailure(simgear::LoadFailure::OutOfMemory, simgear::ErrorCode::TerraSync,
+                                           "copyInstalledChildren: couldn't allocation copy buffer of size:" + std::to_string(sizeToCopy),
+                                           child.path);
+                    return;
                 }
             }
 
-            src.read(buf, cp.sizeInBytes());
+            const auto r = src.read(buf.data(), sizeToCopy);
-            dst.write(buf, cp.sizeInBytes());
+            if (r != sizeToCopy) {
+                simgear::reportFailure(simgear::LoadFailure::IOError, simgear::ErrorCode::TerraSync,
+                                       "copyInstalledChildren: read underflow, got:" + std::to_string(r),
+                                       cp);
+                return;
+            }
+
+            const auto written = dst.write(buf.data(), sizeToCopy);
+            if (written != sizeToCopy) {
+                simgear::reportFailure(simgear::LoadFailure::IOError, simgear::ErrorCode::TerraSync,
+                                       "copyInstalledChildren: write underflow, wrote:" + std::to_string(r),
+                                       child.path);
+                return;
+            }
+
             src.close();
             dst.close();
 
@@ -261,8 +278,6 @@ public:
             std::string hash = computeHashForPath(child.path);
             updatedFileContents(child.path, hash);
         }
-
-                free(buf);
     }
 
     /// helper to check and erase 'fooBar' from paths, if passed fooBar.zip, fooBar.tgz, etc.
@@ -1019,13 +1034,20 @@ HTTPRepository::failure() const
             if (!file.get()) {
                 const bool ok = createOutputFile();
                 if (!ok) {
+                    ioFailureOccurred = true;
                     _directory->repository()->http->cancelRequest(
                         this, "Unable to create output file:" + pathInRepo.utf8Str());
                 }
             }
 
             sha1_write(&hashContext, s, n);
-            file->write(s, n);
+            const auto written = file->write(s, n);
+            if (written != n) {
+                SG_LOG(SG_TERRASYNC, SG_WARN, "Underflow writing to " << pathInRepo);
+                ioFailureOccurred = true;
+                _directory->repository()->http->cancelRequest(
+                    this, "Unable to write to output file:" + pathInRepo.utf8Str());
+            }
         }
 
         bool createOutputFile()
@@ -1082,9 +1104,16 @@ HTTPRepository::failure() const
 
       void onFail() override {
         HTTPRepository::ResultCode code = HTTPRepository::REPO_ERROR_SOCKET;
+
+        // -1 means request cancelled locally
         if (responseCode() == -1) {
+            if (ioFailureOccurred) {
+                // cancelled by code above due to IO error
+                code = HTTPRepository::REPO_ERROR_IO;
+            } else {
                 code = HTTPRepository::REPO_ERROR_CANCELLED;
             }
+        }
 
         if (file) {
           file->close();
@@ -1120,6 +1149,10 @@ HTTPRepository::failure() const
         SGPath pathInRepo;
         simgear::sha1nfo hashContext;
         std::unique_ptr<SGBinaryFile> file;
+
+        /// becuase we cancel() in the case of an IO failure, we need to a way to distuinguish
+        /// user initated cancellation and IO-failure cancellation in onFail. This flag lets us do that
+        bool ioFailureOccurred = false;
     };
 
     class DirGetRequest : public HTTPRepoGetRequest
@@ -1373,6 +1406,9 @@ HTTPRepository::failure() const
         if (st == HTTPRepository::REPO_ERROR_FILE_NOT_FOUND) {
             status = HTTPRepository::REPO_ERROR_NOT_FOUND;
         } else {
+            simgear::reportFailure(simgear::LoadFailure::NetworkError, simgear::ErrorCode::TerraSync,
+                                   "failed to get TerraSync repository root:" + innerResultCodeAsString(st),
+                                   sg_location{baseUrl});
             SG_LOG(SG_TERRASYNC, SG_WARN, "Failed to get root of repo:" << baseUrl << " " << st);
             status = st;
         }
@@ -1389,6 +1425,10 @@ HTTPRepository::failure() const
                "failed to update entry:" << relativePath << " status/code: "
                                          << innerResultCodeAsString(fileStatus)
                                          << "/" << fileStatus);
+
+        simgear::reportFailure(simgear::LoadFailure::NetworkError, simgear::ErrorCode::TerraSync,
+                               "failed to update entry:" + innerResultCodeAsString(fileStatus),
+                               sg_location{relativePath});
       }
 
       HTTPRepository::Failure f;