From 6d65f8eea29e371c1c2ca424d27e9f09c401e0fb Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 19 Nov 2025 01:30:02 +0300 Subject: [PATCH 1/7] libstore: Slightly deindent writeCallback by wrapping it in try/catch The indentation level of the code is already high enough. We can just wrap the whole function in a try/catch and mark it noexcept. Partially cherry-picked from https://gerrit.lix.systems/c/lix/+/2133 Co-authored-by: eldritch horrors --- src/libstore/filetransfer.cc | 40 +++++++++++++++++------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 5acad485c..0ec822e81 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -170,30 +170,28 @@ struct curlFileTransfer : public FileTransfer std::exception_ptr writeException; - size_t writeCallback(void * contents, size_t size, size_t nmemb) - { - try { - size_t realSize = size * nmemb; - result.bodySize += realSize; + size_t writeCallback(void * contents, size_t size, size_t nmemb) noexcept + try { + size_t realSize = size * nmemb; + result.bodySize += realSize; - if (!decompressionSink) { - decompressionSink = makeDecompressionSink(encoding, finalSink); - if (!successfulStatuses.count(getHTTPStatus())) { - // In this case we want to construct a TeeSink, to keep - // the response around (which we figure won't be big - // like an actual download should be) to improve error - // messages. - errorSink = StringSink{}; - } + if (!decompressionSink) { + decompressionSink = makeDecompressionSink(encoding, finalSink); + if (!successfulStatuses.count(getHTTPStatus())) { + // In this case we want to construct a TeeSink, to keep + // the response around (which we figure won't be big + // like an actual download should be) to improve error + // messages. + errorSink = StringSink{}; } - - (*decompressionSink)({(char *) contents, realSize}); - - return realSize; - } catch (...) { - writeException = std::current_exception(); - return 0; } + + (*decompressionSink)({(char *) contents, realSize}); + + return realSize; + } catch (...) { + writeException = std::current_exception(); + return 0; } static size_t writeCallbackWrapper(void * contents, size_t size, size_t nmemb, void * userp) From e704b8eeed7fa3490250b875933c82bd845d14e7 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 19 Nov 2025 01:31:41 +0300 Subject: [PATCH 2/7] libstore/filetransfer: Rename writeException -> callbackException --- src/libstore/filetransfer.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 0ec822e81..de7c13312 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -168,7 +168,7 @@ struct curlFileTransfer : public FileTransfer std::shared_ptr decompressionSink; std::optional errorSink; - std::exception_ptr writeException; + std::exception_ptr callbackException; size_t writeCallback(void * contents, size_t size, size_t nmemb) noexcept try { @@ -190,7 +190,7 @@ struct curlFileTransfer : public FileTransfer return realSize; } catch (...) { - writeException = std::current_exception(); + callbackException = std::current_exception(); return 0; } @@ -474,7 +474,7 @@ struct curlFileTransfer : public FileTransfer try { decompressionSink->finish(); } catch (...) { - writeException = std::current_exception(); + callbackException = std::current_exception(); } } @@ -483,8 +483,8 @@ struct curlFileTransfer : public FileTransfer httpStatus = 304; } - if (writeException) - failEx(writeException); + if (callbackException) + failEx(callbackException); else if (code == CURLE_OK && successfulStatuses.count(httpStatus)) { result.cached = httpStatus == 304; From 1e42e55fb41c1eaccb9f3087badb4801e3917b34 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 19 Nov 2025 01:35:54 +0300 Subject: [PATCH 3/7] libstore/filetransfer: Set callbackException on exceptions in read/seek callbacks This would provide better error messages if seeking/reading ever fails. --- src/libstore/filetransfer.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index de7c13312..221e162a6 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -300,6 +300,7 @@ struct curlFileTransfer : public FileTransfer } catch (EndOfFile &) { return 0; } catch (...) { + callbackException = std::current_exception(); return CURL_READFUNC_ABORT; } @@ -331,6 +332,7 @@ struct curlFileTransfer : public FileTransfer } return CURL_SEEKFUNC_OK; } catch (...) { + callbackException = std::current_exception(); return CURL_SEEKFUNC_FAIL; } From 87d3c3ba1abf9c1e4bb47b590a5bf084fd74ff06 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 19 Nov 2025 01:44:03 +0300 Subject: [PATCH 4/7] libstore/filetransfer: Handle exceptions in headerCallback Callbacks *must* never throw exceptions on the curl thread! --- src/libstore/filetransfer.cc | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 221e162a6..ba5e000b3 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -207,8 +207,8 @@ struct curlFileTransfer : public FileTransfer result.urls.push_back(effectiveUriCStr); } - size_t headerCallback(void * contents, size_t size, size_t nmemb) - { + size_t headerCallback(void * contents, size_t size, size_t nmemb) noexcept + try { size_t realSize = size * nmemb; std::string line((char *) contents, realSize); printMsg(lvlVomit, "got header for '%s': %s", request.uri, trim(line)); @@ -261,6 +261,15 @@ struct curlFileTransfer : public FileTransfer } } return realSize; + } catch (...) { +#if LIBCURL_VERSION_NUM >= 0x075700 + /* https://curl.se/libcurl/c/CURLOPT_HEADERFUNCTION.html: + You can also abort the transfer by returning CURL_WRITEFUNC_ERROR. */ + callbackException = std::current_exception(); + return CURL_WRITEFUNC_ERROR; +#else + return realSize; +#endif } static size_t headerCallbackWrapper(void * contents, size_t size, size_t nmemb, void * userp) From b3dfe37aeac0fbe3c036dcb65cf3760a0e796849 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 19 Nov 2025 01:50:19 +0300 Subject: [PATCH 5/7] libstore/filetransfer: Handle exceptions in progressCallback --- src/libstore/filetransfer.cc | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index ba5e000b3..c3ad763e4 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -277,14 +277,17 @@ struct curlFileTransfer : public FileTransfer return ((TransferItem *) userp)->headerCallback(contents, size, nmemb); } - int progressCallback(curl_off_t dltotal, curl_off_t dlnow) - { - try { - act.progress(dlnow, dltotal); - } catch (nix::Interrupted &) { - assert(getInterrupted()); - } + int progressCallback(curl_off_t dltotal, curl_off_t dlnow) noexcept + try { + act.progress(dlnow, dltotal); return getInterrupted(); + } catch (nix::Interrupted &) { + assert(getInterrupted()); + return 1; + } catch (...) { + /* Something unexpected has happened like logger throwing an exception. */ + callbackException = std::current_exception(); + return 1; } static int progressCallbackWrapper( From bd0b338e15590b8908aa56d547be3a35c0f965f2 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 19 Nov 2025 01:51:16 +0300 Subject: [PATCH 6/7] libstore/filetransfer: Swallow exceptions in debugCallback --- src/libstore/filetransfer.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index c3ad763e4..407b087c9 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -298,11 +298,14 @@ struct curlFileTransfer : public FileTransfer return item.progressCallback(isUpload ? ultotal : dltotal, isUpload ? ulnow : dlnow); } - static int debugCallback(CURL * handle, curl_infotype type, char * data, size_t size, void * userptr) - { + static int debugCallback(CURL * handle, curl_infotype type, char * data, size_t size, void * userptr) noexcept + try { if (type == CURLINFO_TEXT) vomit("curl: %s", chomp(std::string(data, size))); return 0; + } catch (...) { + /* Swallow the exception. Nothing left to do. */ + return 0; } size_t readCallback(char * buffer, size_t size, size_t nitems) noexcept From 36f4e290d0f95f7df080bffc987375fd97d6c818 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Wed, 19 Nov 2025 02:22:23 +0300 Subject: [PATCH 7/7] libstore/filetransfer: Add more context to error message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now the error message looks something like: error: … during upload of 'file:///tmp/storeabc/4yxrw9flcvca7f3fs7c5igl2ica39zaw.narinfo' error: blah blah Also makes fail and failEx themselves noexcept, since all the operations they do are noexcept and we don't want exceptions escaping from them. --- src/libstore/filetransfer.cc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 407b087c9..709cdaffb 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -151,15 +151,23 @@ struct curlFileTransfer : public FileTransfer } } - void failEx(std::exception_ptr ex) + void failEx(std::exception_ptr ex) noexcept { assert(!done); done = true; + try { + std::rethrow_exception(ex); + } catch (nix::Error & e) { + /* Add more context to the error message. */ + e.addTrace({}, "during %s of '%s'", Uncolored(request.verb()), request.uri.to_string()); + } catch (...) { + /* Can't add more context to the error. */ + } callback.rethrow(ex); } template - void fail(T && e) + void fail(T && e) noexcept { failEx(std::make_exception_ptr(std::forward(e))); }