lix-project/lix
20
57
Fork 31
Code Issues 347 Code Review (Gerrit) Projects 3 Releases Wiki Activity

libstore: do not retry FileTransfer uploads

Browse source 
this only ever worked for empty uploads, and there it worked only by
complete accident: curl was asked to send more data than the wrapper
would provide, which curl would not like and report as an error. the
error would cause a retry with even less data to send, until finally
failing by running into the retry limit. let's just forbid all this.

Change-Id: I229a94b3b8b33e2c6cdb8ea19edd57cd6740e6c6
This commit is contained in:
eldritch horrors 2024-11-09 01:17:28 +01:00
parent 40be91afbf
commit 5d02800e57
2 changed files with 27 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
src/libstore/filetransfer.cc
View file

@ -486,6 +486,7 @@ struct curlFileTransfer : public FileTransfer
sink, we can only retry if the server supports
ranged requests. */
if (err == Transient
&& !uploadData.has_value()
&& attempt < tries
&& (!this->dataCallback
|| writtenToSink == 0

30
tests/unit/libstore/filetransfer.cc
View file

@ -122,9 +122,10 @@ serveHTTP(std::vector<Reply> replies)
std::thread([=, conn{std::move(conn)}] {
auto send = [&](std::string_view bit) {
while (!bit.empty()) {
auto written = ::write(conn.get(), bit.data(), bit.size());
auto written = ::send(conn.get(), bit.data(), bit.size(), MSG_NOSIGNAL);
if (written < 0) {
throw SysError(errno, "write() failed");
debug("send() failed: %s", strerror(errno));
return;
}
bit.remove_prefix(written);
}
@ -164,13 +165,14 @@ serveHTTP(std::vector<Reply> replies)
::shutdown(conn.get(), SHUT_WR);
for (;;) {
char buf[1];
switch (read(conn.get(), buf, 1)) {
switch (recv(conn.get(), buf, 1, MSG_NOSIGNAL)) {
case 0:
return; // remote closed
case 1:
continue; // connection still held open by remote
default:
throw SysError(errno, "read() failed");
debug("recv() failed: %s", strerror(errno));
return;
}
}
}).detach();
@ -369,4 +371,24 @@ TEST(FileTransfer, doesntRetryTransferForever)
ASSERT_THROW(ft->download(fmt("http://[::1]:%d", port)).second->drain(), FileTransferError);
}
TEST(FileTransfer, doesntRetryUploads)
{
auto ft = makeFileTransfer(0);
{
auto [port, srv] = serveHTTP({
{"429 try again later", "", [] { return ""; }},
{"200 ok", "", [] { return ""; }},
});
ASSERT_THROW(ft->upload(fmt("http://[::1]:%d", port), ""), FileTransferError);
}
{
auto [port, srv] = serveHTTP({
{"429 try again later", "", [] { return ""; }},
{"200 ok", "", [] { return ""; }},
});
ASSERT_THROW(ft->upload(fmt("http://[::1]:%d", port), "foo"), FileTransferError);
}
}
}