More review comments

Author: vicroms

include/vcpkg/base/contractual-constants.h

@@ -590,4 +590,7 @@ namespace vcpkg
     inline constexpr StringLiteral StatusInstalled = "installed";
     inline constexpr StringLiteral StatusNotInstalled = "not-installed";
     inline constexpr StringLiteral StatusPurge = "purge";
+
+    inline constexpr StringLiteral AppInsightsResponseItemsReceived = "itemsReceived";
+    inline constexpr StringLiteral AppInsightsResponseItemsAccepted = "itemsAccepted";
 }

include/vcpkg/metrics.h

@@ -199,4 +199,7 @@ namespace vcpkg
 
     void flush_global_metrics(const Filesystem&);
     bool curl_upload_metrics(const std::string& payload);
+
+    // exposed for testing
+    bool parse_metrics_response(StringView response_body);
 }

src/vcpkg-test/metrics.cpp

@@ -256,3 +256,39 @@ TEST_CASE ("payload smoke test", "[metrics]")
 )json";
     REQUIRE(expected == actual);
 }
+
+TEST_CASE ("parse metrics response", "[metrics]")
+{
+    const std::string response = R"json(
+{
+  "itemsReceived": 1,
+  "itemsAccepted": 1,
+  "errors": []
+}
+)json";
+    auto parsed = parse_metrics_response(response);
+    CHECK(parsed);
+
+    const std::string response_with_errors = R"json(
+{ 
+  "itemsReceived": 2,
+  "itemsAccepted": 1,
+  "errors": [
+    {
+      "message": "Invalid payload"
+    }
+  ]
+}
+)json";
+    auto parsed_with_errors = parse_metrics_response(response_with_errors);
+    CHECK(!parsed_with_errors);
+
+    const std::string response_with_errors2 = R"json(
+{ 
+  "itemsReceived": 2,
+  "errors": []
+}
+)json";
+    auto parsed_with_errors2 = parse_metrics_response(response_with_errors2);
+    CHECK(!parsed_with_errors2);
+}

src/vcpkg/base/downloads.cpp

@@ -362,7 +362,13 @@ namespace vcpkg
             context.report_error(format_filesystem_call_error(ec, "fopen", {file}));
             return false;
         }
-        auto file_size = fileptr.size(VCPKG_LINE_INFO);
+        ec.clear();
+        auto file_size = fileptr.size(ec);
+        if (ec)
+        {
+            context.report_error(format_filesystem_call_error(ec, "fstat", {file}));
+            return false;
+        }
 
         CurlEasyHandle handle;
         CURL* curl = handle.get();

src/vcpkg/metrics.cpp

@@ -1,4 +1,5 @@
 #include <vcpkg/base/chrono.h>
+#include <vcpkg/base/contractual-constants.h>
 #include <vcpkg/base/curl.h>
 #include <vcpkg/base/files.h>
 #include <vcpkg/base/hash.h>
@@ -517,16 +518,11 @@ namespace vcpkg
         fs.write_contents(vcpkg_metrics_txt_path, payload, ec);
         if (ec) return;
 
-        const StringLiteral executableExtension =
+        const Path temp_folder_path_exe = temp_folder_path / "vcpkg-" VCPKG_BASE_VERSION_AS_STRING
 #if defined(WIN32)
-            ".exe"
-#else
-            ""
+                                                             ".exe"
 #endif
             ;
-
-        const Path temp_folder_path_exe =
-            temp_folder_path / "vcpkg-" VCPKG_BASE_VERSION_AS_STRING + executableExtension;
         fs.copy_file(get_exe_path_of_current_process(), temp_folder_path_exe, CopyOptions::skip_existing, ec);
         if (ec) return;
 
@@ -547,14 +543,14 @@ namespace vcpkg
         return size * nmemb;
     }
 
-    static bool parse_metrics_response(StringView body)
+    bool parse_metrics_response(StringView response_body)
     {
-        auto maybe_json = Json::parse_object(body, "metrics_response");
+        auto maybe_json = Json::parse_object(response_body, "metrics_response");
         auto json = maybe_json.get();
         if (!json) return false;
 
-        auto maybe_received = json->get("itemsReceived");
-        auto maybe_accepted = json->get("itemsAccepted");
+        auto maybe_received = json->get(vcpkg::AppInsightsResponseItemsReceived);
+        auto maybe_accepted = json->get(vcpkg::AppInsightsResponseItemsAccepted);
         auto maybe_errors = json->get("errors");
 
         if (maybe_received && maybe_accepted && maybe_errors && maybe_received->is_integer() &&