Skip to content

Comments

libstore/filetransfer: Improve error handling#14591

Merged
Ericson2314 merged 7 commits intomasterfrom
filetransfer-error-handling
Nov 19, 2025
Merged

libstore/filetransfer: Improve error handling#14591
Ericson2314 merged 7 commits intomasterfrom
filetransfer-error-handling

Conversation

@xokdvium
Copy link
Contributor

Motivation

Improves error handling in filetransfer.cc. All callbacks must ensure that no exceptions escape, since that would unwind the stack right into libcurl which is bound to break things.
Also record exceptions in read/seek callbacks with the same mechanism as writeCallback does it.

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

xokdvium and others added 6 commits November 19, 2025 02:23
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 <pennae@lix.systems>
…ek callbacks

This would provide better error messages if seeking/reading ever fails.
Callbacks *must* never throw exceptions on the curl thread!
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.
@xokdvium xokdvium force-pushed the filetransfer-error-handling branch from 446500e to 36f4e29 Compare November 18, 2025 23:30
@Ericson2314 Ericson2314 added this pull request to the merge queue Nov 19, 2025
Merged via the queue into master with commit dfac44c Nov 19, 2025
20 checks passed
@Ericson2314 Ericson2314 deleted the filetransfer-error-handling branch November 19, 2025 02:24
@edolstra edolstra mentioned this pull request Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants