fix: report download failure when resume give-up occurs mid-stream

src/common/http/http_resumer.cpp

@@ -472,6 +472,23 @@ error::Error DownloadResumerClient::ScheduleNextResumeRequest() {
 void DownloadResumerClient::CallUserHandler(http::ExpectedIncomingResponsePtr exp_resp) {
 	if (!exp_resp) {
 		DoCancel();
+
+		// If a read is pending on the body reader, complete it with the error as well. When the
+		// body is consumed through a DownloadResumerAsyncReader (streaming download, as the
+		// update state machine does), a terminal error such as giving up on resuming after the
+		// retry backoff is exhausted only reaches the user through the user body handler below.
+		// But that handler cannot recover a streaming read, and the update client intentionally
+		// ignores errors there, relying on them surfacing through the reader instead. Without
+		// completing the pending read, that consumer blocks forever and the daemon wedges
+		// without ever reporting a deployment failure. Delivering the error here does not
+		// disturb the buffered SetBodyWriter case, where the pending read belongs to an internal
+		// AsyncCopy that merely logs the error, and the user body handler below still runs.
+		auto resumer_reader = resumer_reader_.lock();
+		if (resumer_reader && last_read_.handler) {
+			auto handler = last_read_.handler;
+			last_read_.handler = nullptr;
+			handler(expected::unexpected(exp_resp.error()));
+		}
 	}
 	if (resumer_state_->user_handlers_state == DownloadResumerUserHandlersStatus::None) {
 		resumer_state_->user_handlers_state =

tests/src/common/http_resumer/http_resumer_test.cpp

@@ -692,6 +692,114 @@ TEST_F(DownloadResumerTest, ResponseBodyReaderSmallBuffer) {
 	EXPECT_FALSE(got_read_error);
 }
 
+// Regression test for the daemon hang reported in
+// https://hub.mender.io/t/.../8299 : when the body is consumed through the async reader
+// (as the update state machine does) and the server stays down until the resumer exhausts
+// its retry backoff, the "giving up" error must be delivered through the reader's pending
+// read handler. Before the fix it was only routed to the user body handler (which the update
+// client intentionally ignores), so the pending read never completed and the daemon wedged
+// forever without reporting a deployment failure.
+TEST_F(DownloadResumerTest, ServerDownWhileReadingBodyReportsError) {
+	TestEventLoop loop(chrono::seconds(30));
+
+	http::ServerConfig server_config;
+	http::Server server(server_config, loop);
+	events::Timer kill_timer(loop);
+	bool kill_scheduled {false};
+
+	int server_num_requests = 0;
+	server.AsyncServeUrl(
+		"http://127.0.0.1:" TEST_PORT,
+		[](http::ExpectedIncomingRequestPtr exp_req) {
+			ASSERT_TRUE(exp_req) << exp_req.error().String();
+		},
+		[&server, &server_num_requests, &kill_timer, &kill_scheduled](
+			http::ExpectedIncomingRequestPtr exp_req) {
+			ASSERT_TRUE(exp_req) << exp_req.error().String();
+
+			auto result = exp_req.value()->MakeResponse();
+			ASSERT_TRUE(result);
+			auto resp = result.value();
+
+			server_num_requests++;
+
+			auto size = RangeBodyOfXes::TARGET_BODY_SIZE;
+			resp->SetStatusCodeAndMessage(200, "Success");
+			// Announce the full length, but only deliver a small part of it, so the client
+			// sees a short read and tries to resume. Keep it small so the first response
+			// finishes well before the server is taken down below.
+			resp->SetHeader("Content-Length", to_string(size));
+			auto partial_body = make_shared<RangeBodyOfXes>();
+			partial_body->SetRanges(0, 4999);
+			resp->SetBodyReader(partial_body);
+
+			resp->AsyncReply([](error::Error err) { ASSERT_EQ(error::NoError, err); });
+
+			// After the first (partial) response, take the server down and keep it down, so
+			// every resume attempt fails until the resumer gives up. Cancelling from a timer
+			// (rather than the reply handler) avoids tearing down the stream mid-write.
+			if (!kill_scheduled) {
+				kill_scheduled = true;
+				kill_timer.AsyncWait(
+					chrono::milliseconds(100), [&server](error::Error) { server.Cancel(); });
+			}
+		});
+
+	http::ClientConfig client_config;
+	shared_ptr<http_resumer::DownloadResumerClient> client =
+		make_shared<http_resumer::DownloadResumerClient>(client_config, loop);
+	// First resume waits longer than the server-kill delay above, so the server is already
+	// down by the time we retry, and the retries are then exhausted quickly.
+	client->SetSmallestWaitInterval(chrono::milliseconds(200));
+
+	auto req = make_shared<http::OutgoingRequest>();
+	req->SetMethod(http::Method::GET);
+	req->SetAddress("http://127.0.0.1:" TEST_PORT);
+
+	vector<uint8_t> buf;
+	buf.resize(1235);
+	bool got_read_error {false};
+	error::Error read_error;
+
+	client->AsyncCall(
+		req,
+		[&](http::ExpectedIncomingResponsePtr exp_resp) {
+			ASSERT_TRUE(exp_resp) << exp_resp.error().String();
+			auto resp = exp_resp.value();
+
+			auto reader = *client->MakeBodyAsyncReader(resp);
+			reader->RepeatedAsyncRead(
+				buf.begin(),
+				buf.end(),
+				// Capture `reader` to keep it alive.
+				[reader, &got_read_error, &read_error, &loop](io::ExpectedSize result) {
+					if (!result) {
+						got_read_error = true;
+						read_error = result.error();
+						loop.Stop();
+						return io::Repeat::No;
+					}
+					if (result.value() == 0) {
+						// Unexpected clean EOF; stop so the test does not hang.
+						loop.Stop();
+						return io::Repeat::No;
+					}
+					return io::Repeat::Yes;
+				});
+		},
+		[](http::ExpectedIncomingResponsePtr exp_resp) {
+			// Body handler intentionally ignores errors, exactly like the update state
+			// machine: the error must surface through the reader instead.
+		});
+
+	loop.Run();
+
+	EXPECT_TRUE(got_read_error)
+		<< "resumer gave up but the reader never reported an error (the daemon would hang here)";
+	EXPECT_EQ(read_error.code, http::MakeError(http::DownloadResumerError, "").code)
+		<< "unexpected error: " << read_error.String();
+}
+
 TEST_F(DownloadResumerTest, TwoRangesClientReuse) {
 	TestEventLoop loop(chrono::seconds(10));