0.3.2 --- download_to_file reports the writes the disk refused (#19)

* fix(http): download_to_file reports the writes the disk refused

download_to_file called ofs.write for each body chunk without looking at the
stream, added each chunk's size to bytesWritten from what the network delivered,
and set bytesWritten after ofs.close() without looking at that either. On a full
disk (ENOSPC) the transfer therefore succeeded: a 420,831,054 byte download left
220,979,200 bytes on disk while bytesWritten said 420,831,054 and ok() was true,
and the caller blamed the source (a checksum mismatch) for what was the local
disk.

Each chunk is now flushed and the stream checked. On the first failure the
transfer stops reading, error becomes "write <path>: <reason>" with the reason
taken from errno, and the connection is dropped instead of returned to the pool
with the rest of the body still on it. Closing the file is checked the same way.

Additive: DownloadToFileResult::writeFailed says the fault is the destination
and not the source (a failed write, a failed close, or a file that could not be
opened), and bytesReceived carries the network count. bytesWritten now counts
bytes the file accepted; the two are equal for every transfer that did not fail
to write.

Tests: a download into /dev/full (Linux) against the in-process TLS server
asserts not ok, writeFailed, bytesWritten == 0, the ENOSPC text, that reading
stopped after the first refused chunk, and that the next request opens a new
connection. Verified by mutation: with the stream checks removed it reports
ok() and bytesWritten == 65536.

* 0.3.2 --- download_to_file reports the writes the disk refused

Version, CHANGELOG entry, and the README install lines name this release.

* ci: bump mcpp to 2026.9.28.3 to satisfy the index floor

The index now requires mcpp >= 2026.9.18.3, and MCPP_VERSION pinned 2026.8.29.1,
so every `mcpp` invocation that needed the index ended at

    error: index requires mcpp >= 2026.9.18.3 but this is mcpp 2026.8.29.1 [E0006]

That is the failure the "Smoke-test the project templates" step has reported on
master since the scheduled run of 2026-09-21. MCPP_VERSION is the only place the
version is named; both jobs' install steps and both cache keys read it.

---------

Co-authored-by: speak-agent <248744407+speak-agent@users.noreply.github.com>

Author: speak-agent

.github/workflows/ci.yml

@@ -31,7 +31,7 @@ on:
   workflow_dispatch:
 
 env:
-  MCPP_VERSION: 2026.8.29.1
+  MCPP_VERSION: 2026.9.28.3
   XLINGS_VERSION: v2026.8.17.2
   XLINGS_NON_INTERACTIVE: '1'
 

CHANGELOG.md

@@ -1,5 +1,35 @@
 # Changelog
 
+## 0.3.2
+
+`download_to_file` no longer reports success for a file the disk did not keep.
+
+It called `ofs.write` for each body chunk without looking at the stream, added
+each chunk's size to `bytesWritten` from what the network delivered, and set
+`bytesWritten` after `ofs.close()` without looking at that either. On a full disk
+(`ENOSPC`) the transfer therefore succeeded: a 420,831,054 byte download left
+220,979,200 bytes on disk while `bytesWritten` said 420,831,054 and `ok()` was
+true, and the caller blamed the source (a checksum mismatch) for what was the
+local disk.
+
+* Each chunk is flushed and the stream checked. On the first failure the
+  transfer stops reading, `error` becomes `write <path>: <reason>` (the reason
+  is `errno` as `std::generic_category` words it, for example `No space left on
+  device`), and the connection is dropped rather than returned to the pool with
+  the rest of the body still on it. Closing the file is checked the same way.
+* `DownloadToFileResult::writeFailed` is new and is true when the fault is the
+  destination rather than the source: the write failed, the close failed, or the
+  file could not be opened (`error` is still `Cannot open file: <path>` for the
+  last).
+* `DownloadToFileResult::bytesReceived` is new: the bytes of body the
+  connection delivered.
+* `DownloadToFileResult::bytesWritten` changes meaning, and this is the one
+  behaviour change: it now counts bytes the file accepted, where it used to
+  count bytes the network delivered. The two are equal for every transfer that
+  did not fail to write. When a write fails, the chunk that failed is not
+  counted, though the file may hold part of it, so the value is a floor on the
+  file's size.
+
 ## 0.3.1
 
 The socket interface is selected by the C library rather than by the operating

README.md

@@ -114,14 +114,14 @@ cd examples/openkal && mcpp run
 ### 添加依赖
 
 ```bash
-mcpp add tinyhttps@0.3.0
+mcpp add tinyhttps@0.3.2
 ```
 
 或在 `mcpp.toml` 中手动添加：
 
 ```toml
 [dependencies]
-tinyhttps = "0.3.1"
+tinyhttps = "0.3.2"
 ```
 
 ### 构建
@@ -140,7 +140,10 @@ mcpplibs::tinyhttps::HttpClient client;
 auto result = client.download_to_file(
     "https://example.com/big.tar.gz", "out/big.tar.gz",
     [](std::int64_t total, std::int64_t done) { /* progress */ });
-if (!result.ok()) { /* result.error */ }
+if (!result.ok()) {
+    // result.error says why; result.writeFailed says the local file is at
+    // fault (a full disk, a refused write) and not the server.
+}
 ```
 
 ## License

mcpp.toml

@@ -1,7 +1,7 @@
 [package]
 namespace   = "mcpplibs"
 name        = "tinyhttps"
-version     = "0.3.1"
+version     = "0.3.2"
 description = "Minimal C++23 HTTP/HTTPS client with SSE streaming support"
 license     = "Apache-2.0"
 repo        = "https://github.com/mcpplibs/tinyhttps"

src/http.cppm

@@ -1,3 +1,9 @@
+module;
+
+// `import std` carries no `errno`: it is a macro, and a module does not export
+// macros. The download path reads it to say why the local file refused a write.
+#include <cerrno>
+
 export module mcpplibs.tinyhttps:http;
 
 import :tls;
@@ -105,11 +111,39 @@ export using DownloadProgressFn = std::function<void(std::int64_t total, std::in
 export struct DownloadToFileResult {
     int statusCode { 0 };
     std::string error;
+
+    // Bytes that reached the destination file: the sum of the body chunks whose
+    // write the stream accepted. It is a statement about the file and not about
+    // the network, which is `bytesReceived`.
+    //
+    // It used to be the network count. On a full disk every write after the
+    // first failure was discarded while this kept rising, so a 420,831,054 byte
+    // download that left 220,979,200 bytes on disk reported 420,831,054 and
+    // `ok()`. When a write fails, the chunk that failed is not counted, though
+    // the file may hold part of it; the value is a floor on the file's size and
+    // never above it.
     std::int64_t bytesWritten { 0 };
+
     std::optional<std::int64_t> expectedBytes;
     std::string finalUrl;
     std::string etag;
     std::string lastModified;
+
+    // Bytes of body the connection delivered, whether or not the file took them.
+    // Equal to `bytesWritten` unless `writeFailed`.
+    std::int64_t bytesReceived { 0 };
+
+    // TRUE WHEN THE FAULT IS THE DESTINATION AND NOT THE SOURCE — the file could
+    // not be opened, a write to it failed (a full disk, a quota, a device that
+    // refuses), or closing it reported that buffered data did not reach it.
+    //
+    // `error` then names the path and the reason, and the transfer stops at the
+    // first failure instead of reading on and discarding what it reads. It is
+    // what lets a caller tell "the server sent something wrong" from "this
+    // machine could not keep it" without matching text in `error`; a checksum
+    // failure downstream of a truncated file otherwise blames the wrong party.
+    bool writeFailed { false };
+
     bool ok() const { return statusCode >= 200 && statusCode < 300 && error.empty(); }
 };
 
@@ -1222,34 +1256,82 @@ private:
         std::ofstream ofs(destFile, std::ios::binary);
         if (!ofs) {
             result.error = "Cannot open file: " + destFile.string();
+            result.writeFailed = true;
             settle_without_body(exchange, hasBody, guard);
             return result;
         }
 
-        if (!hasBody) {
+        // What the file refused, in the words of the system that refused it.
+        // `errno` is cleared before each operation that can set it, because the
+        // socket reads between writes leave their own values behind and a stale
+        // EAGAIN reported as the reason a disk filled is worse than no reason.
+        int writeErrno = 0;
+        bool writeFailed = false;
+        auto close_file = [&] {
+            errno = 0;
             ofs.close();
+            if (!writeFailed && ofs.fail()) {
+                writeFailed = true;
+                writeErrno = errno;
+            }
+        };
+        auto write_failure = [&] {
+            return "write " + destFile.string() + ": " +
+                   (writeErrno != 0
+                        ? std::generic_category().message(writeErrno)
+                        : std::string("stream error"));
+        };
+
+        if (!hasBody) {
+            close_file();
+            if (writeFailed) {
+                result.error = write_failure();
+                result.writeFailed = true;
+                return result;
+            }
             if (config_.keepAlive && !exchange.head.connectionClose) guard.keep();
             return result;
         }
 
         const std::int64_t totalBytes =
             exchange.head.contentLength > 0 ? exchange.head.contentLength : 0;
-        std::int64_t downloaded = 0;
+        std::int64_t received = 0;
+        std::int64_t written = 0;
         bool cancelled = false;
 
         auto outcome = read_body(
             *exchange.sock, exchange.head, config_.readTimeoutMs,
             std::numeric_limits<std::int64_t>::max(),
             [&](std::string_view data) -> bool {
+                const auto size = static_cast<std::int64_t>(data.size());
+                received += size;
+
+                // Flushed per chunk, and the stream checked after it. A write
+                // into the stream's buffer succeeds whatever the disk has left;
+                // the failure surfaces at the flush that hands the buffer to the
+                // operating system, and without one here it surfaced at
+                // `close()`, after every chunk had been counted as written.
+                // `read_body` hands over slices of at most 8 KiB, about the size
+                // of the stream's own buffer, so this costs no more system calls
+                // than the buffering it replaces.
+                errno = 0;
                 ofs.write(data.data(), static_cast<std::streamsize>(data.size()));
-                downloaded += static_cast<std::int64_t>(data.size());
-                if (onProgress) onProgress(totalBytes, downloaded);
+                ofs.flush();
+                if (!ofs) {
+                    writeFailed = true;
+                    writeErrno = errno;
+                    return false;   // reading on would only discard what it reads
+                }
+
+                written += size;
+                if (onProgress) onProgress(totalBytes, written);
                 if (isCancelled && isCancelled()) { cancelled = true; return false; }
                 return true;
             });
 
-        ofs.close();
-        result.bytesWritten = downloaded;
+        close_file();
+        result.bytesWritten = written;
+        result.bytesReceived = received;
 
         switch (outcome.end) {
             case BodyEnd::Complete:
@@ -1264,6 +1346,14 @@ private:
                 result.error = outcome.error;
                 break;
         }
+
+        // The destination's failure outranks whatever the switch recorded: a
+        // stop that the file asked for is not a cancellation, and a body that
+        // arrived whole into a file that lost it has not succeeded.
+        if (writeFailed) {
+            result.error = write_failure();
+            result.writeFailed = true;
+        }
         return result;
     }
 

tests/test_pool.cpp

@@ -624,6 +624,94 @@ TEST_F(PoolTest, ADownloadDelimitedByTheCloseReportsSuccess) {
     std::filesystem::remove_all(dir, ec);
 }
 
+// ── a destination that refuses the bytes ─────────────────────────────────────
+
+// `/dev/full` accepts an open and answers every write with ENOSPC, which is what
+// a full disk does at the moment it fills — without needing one.
+//
+// Before 0.3.2 the transfer "succeeded": `ofs.write` was never checked, the
+// count came from the network, and `bytesWritten` said the whole body when the
+// file held none of it. The caller then blamed the source (a checksum mismatch)
+// for what was the local disk. Verified by mutation: with the stream check
+// removed, `ok()` is true and `bytesWritten` is the body's length.
+#ifdef __linux__
+TEST_F(PoolTest, ADownloadIntoAFullDeviceFailsAsALocalWriteAndCountsNothing) {
+    if (!std::filesystem::exists("/dev/full")) GTEST_SKIP() << "no /dev/full";
+
+    const std::string payload(64 * 1024, 'F');
+    tls_test::Server server([payload](tls_test::Conn& conn, int) {
+        conn.write(tls_test::ok_response(payload));
+        return true;
+    });
+    ASSERT_FALSE(server.failed());
+
+    https::HttpClient client(test_config());
+    auto result = client.download_to_file(server.url("/big"), "/dev/full");
+
+    EXPECT_FALSE(result.ok()) << "a file that took nothing was reported as downloaded";
+    EXPECT_TRUE(result.writeFailed)
+        << "the caller must be able to tell the disk from the source; error: "
+        << result.error;
+    EXPECT_EQ(result.statusCode, 200);
+    EXPECT_EQ(result.bytesWritten, 0);
+    EXPECT_GT(result.bytesReceived, 0);
+    EXPECT_LT(result.bytesReceived, static_cast<std::int64_t>(payload.size()))
+        << "the transfer read on after the first refused write";
+    EXPECT_EQ(result.error.rfind("write /dev/full: ", 0), 0u) << result.error;
+    EXPECT_NE(result.error.find(
+                  std::make_error_code(std::errc::no_space_on_device).message()),
+              std::string::npos)
+        << result.error;
+
+    // The rest of the body was left on the socket, so the connection is not
+    // handed to the next request. The next request also shows the client is
+    // not left in a state where a good destination fails too.
+    auto dir = std::filesystem::temp_directory_path() / "tinyhttps_pool_full";
+    std::filesystem::create_directories(dir);
+    auto dest = dir / "ok.bin";
+    auto good = client.download_to_file(server.url("/big"), dest);
+    EXPECT_TRUE(good.ok()) << "error: " << good.error;
+    EXPECT_FALSE(good.writeFailed);
+    EXPECT_EQ(good.bytesWritten, static_cast<std::int64_t>(payload.size()));
+    EXPECT_EQ(good.bytesReceived, good.bytesWritten);
+    EXPECT_EQ(server.accepts(), 2)
+        << "the connection with an unread body was reused";
+
+    std::error_code ec;
+    std::filesystem::remove_all(dir, ec);
+}
+#endif
+
+// A destination that cannot be opened is the same kind of failure: nothing was
+// wrong with what the server sent.
+TEST_F(PoolTest, ADestinationThatCannotBeOpenedIsALocalFailure) {
+    tls_test::Server server([](tls_test::Conn& conn, int) {
+        conn.write(tls_test::ok_response("payload"));
+        return true;
+    });
+    ASSERT_FALSE(server.failed());
+
+    auto dir = std::filesystem::temp_directory_path() / "tinyhttps_pool_noopen";
+    std::filesystem::create_directories(dir);
+    {
+        // A regular file where a directory is needed: no parent can be made.
+        std::ofstream blocker(dir / "blocker", std::ios::binary);
+        blocker << "not a directory";
+    }
+
+    https::HttpClient client(test_config());
+    auto result = client.download_to_file(server.url("/x"),
+                                          dir / "blocker" / "out.bin");
+
+    EXPECT_FALSE(result.ok());
+    EXPECT_TRUE(result.writeFailed) << "error: " << result.error;
+    EXPECT_EQ(result.error.rfind("Cannot open file: ", 0), 0u) << result.error;
+    EXPECT_EQ(result.bytesWritten, 0);
+
+    std::error_code ec;
+    std::filesystem::remove_all(dir, ec);
+}
+
 // ── interim responses ────────────────────────────────────────────────────────
 
 // A 1xx is not the answer. RFC 9112 §2.1 requires a client to read past one or