S3 downloads are not interruptable

[edolstra]

Describe the bug

For instance, hitting Ctrl-C during

nix eval --impure --expr 'builtins.fetchurl "s3://highlighter-public/405b3ba02ccb2f07ec6aba9f233d4194.mp4?region=ap-southeast-2"'

does nothing.

Steps To Reproduce

Expected behavior

Nix should exit with error: interrupted by the user quickly.

Probably the easiest way to do this is to have S3Helper::getObject() use an std::stringstream wrapper that calls checkInterrupt().

nix-env --version output

Additional context

Add any other context about the problem here.

Priorities

Add 👍 to issues you find important.

[KekmaTime]

@edolstra ill work on this issue if you can assign me this

[KekmaTime]

As a first-time contributor, I'd like some help. Where should I implement the new InterruptibleStringStream class (like in libutils or a specific folder)? Also, is this 👇🏽 where is the S3Helper is implemented ?

nix/src/libstore/s3-binary-cache-store.cc

Line 92 in dbcd4cd

S3Helper::S3Helper(

[xokdvium]

I'm somewhat confused by the current implementation. Why does S3 fetching live inside curlFileTransfer, while having absolutely nothing to do with curl. The code is already littered with FIXME/TODO and the "easy" fix would pile even more complexity on this class. SRP is totally broken here. Maybe the better thing to do would be to define an abstract interface for fetching and move the code related to S3 into its own implementation?

[xokdvium]

Also S3 fetching is already problematic and it's not clear how it handles compressed streams? Does the AWS SDK handle the decompression (can it even do that?) or should it be handled the same way that it's done for regular HTTP with libarchive and TarArchive and Sinks/Sources? #11261

It would be very helpful to define interface boundaries so that there are clear and defined requirements and api for file transfers/stream decompression/progress tracking. This would be better in the long run than piling amazingly complex logic into a single class.

[RossComputerGuy]

I've made an attempt in #12227, it's a little delayed at closing out but it does seem to improve the problem.