dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.16k stars 4.72k forks source link

WebClient UploadProgressChanged broken? #46951

Closed endasil closed 3 years ago

endasil commented 3 years ago

To summarize the problem, UploadProgressChanged is called several times, BytesSent goes up until it reached the file size but the file is not actually SENT during that time. The file is sent AFTER.

Tried this with sendDataAsync, sendDataTaskAsync, and UploadFileTaskAsync, with the same result. When the code comes to client.UploadFileTaskAsync, UploadProgressChanged will start to be called repeatedly and I can see by the Console.Writeline that it counts up to the size of the file I'm trying to send very quickly. After this program will stand still for a minute and end. Looking at this with fiddler shows that the file is not actually sent until after the count up is done.

I can see that the value of e.TotalBytesToSend is always -1. I looked this up in MSDN but can't find anything about the meaning of -1. I tried doing this call with curl and using that I can see in fiddler that the file is being uploaded when curl is counting up the bytes sent so apparently it is possible to get it right, I just don't see how.

This is probably related to the similar issue I posted about yesterday when I tried to get progress using httpwebrequest https://stackoverflow.com/questions/65391831/showing-file-upload-progress-with-httpwebrequest

Here is a minimal program that I can reproduce the issue with.

    namespace filetransfertest
    {
        class Program
        {
            static async Task Main(string[] args)
            {
                using (var client = new WebClient())
                {
                    client.UploadProgressChanged += UploadProgressChanged;
                    var result = await client.UploadFileTaskAsync("https://postman-echo.com/post", "POST",
                        "C:/Dev/Unity/AltspaceVr test word/template.zip");
                }
            }

            private static void UploadProgressChanged(object sender, UploadProgressChangedEventArgs e)
            {
                Console.WriteLine("Sent: " + e.BytesSent + " Total: " + e.TotalBytesToSend);
            }
        }
    }

I did some .NET source stepping to find out more. Looking at the code for WebClient I find nothing changing the ProgressData.TotalBytesToSend from -1. Looking at the code around Line 1031 https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.WebClient/src/System/Net/WebClient.cs#L1031 we can see the code that updates BytesSent.

using (Stream writeStream = await request.GetRequestStreamAsync().ConfigureAwait(false)){
    await writeStream.WriteAsync(new ReadOnlyMemory<byte>(header)).ConfigureAwait(false);
    _progress.BytesSent += header.Length;
    PostProgressChanged(asyncOp, _progress);}

Looking upstream used at https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Requests/src/System/Net/RequestStream.cs#L18, it's just a memory stream. What is measured in my app is how much is written to memory.

Is there something I'm missing here? If there is no code changing TotalBytesToSend and the BytesSent is just how many bytes are written to a memory and not sent, it seems like this function is broken to me?

I zipped my application and posted it here in case someone wants to try the same thing as I did. https://www.dropbox.com/s/kllcwd79738370a/filetransfertest.zip?dl=0

scalablecory commented 3 years ago

Have you tried using HttpClient for this? WebClient is no longer in active development, though we may fix a bug if you've found one.

ghost commented 3 years ago

Tagging subscribers to this area: @dotnet/ncl See info in area-owners.md if you want to be subscribed.

Issue Details
To summarize the problem, `UploadProgressChanged` is called several times, `BytesSent` goes up until it reached the file size but the file is not actually SENT during that time. The file is sent AFTER. Tried this with sendDataAsync, sendDataTaskAsync, and UploadFileTaskAsync, with the same result. When the code comes to `client.UploadFileTaskAsync`, UploadProgressChanged will start to be called repeatedly and I can see by the `Console.Writeline` that it counts up to the size of the file I'm trying to send very quickly. After this program will stand still for a minute and end. Looking at this with fiddler shows that the file is not actually sent until after the count up is done. I can see that the value of `e.TotalBytesToSend` is always -1. I looked this up in MSDN but can't find anything about the meaning of -1. I tried doing this call with curl and using that I can see in fiddler that the file is being uploaded when curl is counting up the bytes sent so apparently it is possible to get it right, I just don't see how. This is probably related to the similar issue I posted about yesterday when I tried to get progress using `httpwebrequest` https://stackoverflow.com/questions/65391831/showing-file-upload-progress-with-httpwebrequest Here is a minimal program that I can reproduce the issue with. namespace filetransfertest { class Program { static async Task Main(string[] args) { using (var client = new WebClient()) { client.UploadProgressChanged += UploadProgressChanged; var result = await client.UploadFileTaskAsync("https://postman-echo.com/post", "POST", "C:/Dev/Unity/AltspaceVr test word/template.zip"); } } private static void UploadProgressChanged(object sender, UploadProgressChangedEventArgs e) { Console.WriteLine("Sent: " + e.BytesSent + " Total: " + e.TotalBytesToSend); } } } I did some .NET source stepping to find out more. Looking at the code for WebClient I find nothing changing the ProgressData.TotalBytesToSend from -1. Looking at the code around Line 1031 https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.WebClient/src/System/Net/WebClient.cs#L1031 we can see the code that updates BytesSent. using (Stream writeStream = await request.GetRequestStreamAsync().ConfigureAwait(false)){ await writeStream.WriteAsync(new ReadOnlyMemory(header)).ConfigureAwait(false); _progress.BytesSent += header.Length; PostProgressChanged(asyncOp, _progress);} Looking upstream used at https://github.com/dotnet/runtime/blob/master/src/libraries/System.Net.Requests/src/System/Net/RequestStream.cs#L18, it's just a memory stream. What is measured in my app is how much is written to memory. Is there something I'm missing here? If there is no code changing TotalBytesToSend and the BytesSent is just how many bytes are written to a memory and not sent, it seems like this function is broken to me? I zipped my application and posted it here in case someone wants to try the same thing as I did. https://www.dropbox.com/s/kllcwd79738370a/filetransfertest.zip?dl=0
Author: endasil
Assignees: -
Labels: `area-System.Net`, `untriaged`
Milestone: -
karelz commented 3 years ago

Triage: We believe that it can be achieved via HttpClient - make custom Stream or HttpContent which track number of bytes read. Given that WebClient is obsolete API for compat, we don't think we should extend it further with such functionality unless there are more customers who need it. Closing.

stephentoub commented 3 years ago

we don't think we should extend it further

Just to be clear, this is a bug not a feature request (I wasn't sure how to interpret "extend" here).

The comment here: https://github.com/dotnet/runtime/blob/5761dd49339911241209c1ffd61105a94edb6df1/src/libraries/System.Net.Requests/src/System/Net/RequestStream.cs#L11-L15 is wrong / out-of-date, as AllowWriteStreamBuffering is exposed: https://github.com/dotnet/runtime/blob/5761dd49339911241209c1ffd61105a94edb6df1/src/libraries/System.Net.Requests/ref/System.Net.Requests.cs#L182 it's just ignored by the HttpWebRequest implementation, so even if you set it to false, it still buffers. That's tracked by https://github.com/dotnet/runtime/issues/18632, which hasn't been addressed.