[browser] http streaming request server error (#105709)

* wip * more
dotnet · Aug 6, 2024 · edbb2ba · edbb2ba
1 parent 6b558d9
commit edbb2ba
Show file tree

Hide file tree

Showing 9 changed files with 139 additions and 60 deletions.
diff --git a/src/libraries/Common/tests/System/Net/Configuration.Http.cs b/src/libraries/Common/tests/System/Net/Configuration.Http.cs
@@ -51,6 +51,7 @@ public static partial class Http
             private const string EmptyContentHandler = "EmptyContent.ashx";
             private const string RedirectHandler = "Redirect.ashx";
             private const string VerifyUploadHandler = "VerifyUpload.ashx";
+            private const string StatusCodeHandler = "StatusCode.ashx";
             private const string DeflateHandler = "Deflate.ashx";
             private const string GZipHandler = "GZip.ashx";
             private const string RemoteLoopHandler = "RemoteLoop";
@@ -71,6 +72,7 @@ public static Uri[] GetEchoServerList()
             public static readonly Uri RemoteVerifyUploadServer = new Uri("http://" + Host + "/" + VerifyUploadHandler);
             public static readonly Uri SecureRemoteVerifyUploadServer = new Uri("https://" + SecureHost + "/" + VerifyUploadHandler);
             public static readonly Uri Http2RemoteVerifyUploadServer = new Uri("https://" + Http2Host + "/" + VerifyUploadHandler);
+            public static readonly Uri Http2RemoteStatusCodeServer = new Uri("https://" + Http2Host + "/" + StatusCodeHandler);
 
             public static readonly Uri RemoteEmptyContentServer = new Uri("http://" + Host + "/" + EmptyContentHandler);
             public static readonly Uri RemoteDeflateServer = new Uri("http://" + Host + "/" + DeflateHandler);

diff --git a/src/libraries/Common/tests/System/Net/Http/ResponseStreamTest.cs b/src/libraries/Common/tests/System/Net/Http/ResponseStreamTest.cs
@@ -286,7 +286,6 @@ public async Task BrowserHttpHandler_Streaming()
             }
         }
 
-        [OuterLoop]
         [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsChromium))]
         public async Task BrowserHttpHandler_StreamingRequest()
         {
@@ -328,8 +327,44 @@ public async Task BrowserHttpHandler_StreamingRequest()
             }
         }
 
+        [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsChromium))]
+        public async Task BrowserHttpHandler_StreamingRequest_ServerFail()
+        {
+            var WebAssemblyEnableStreamingRequestKey = new HttpRequestOptionsKey<bool>("WebAssemblyEnableStreamingRequest");
+
+            var requestUrl = new UriBuilder(Configuration.Http.Http2RemoteStatusCodeServer) { Query = "statuscode=500&statusdescription=test&delay=100" };
+            var req = new HttpRequestMessage(HttpMethod.Post, requestUrl.Uri);
+
+            req.Options.Set(WebAssemblyEnableStreamingRequestKey, true);
+
+            int size = 1500 * 1024 * 1024;
+            int remaining = size;
+            var content = new MultipartFormDataContent();
+            content.Add(new StreamContent(new DelegateStream(
+                canReadFunc: () => true,
+                readFunc: (buffer, offset, count) => throw new FormatException(),
+                readAsyncFunc: (buffer, offset, count, cancellationToken) =>
+                {
+                    if (remaining > 0)
+                    {
+                        int send = Math.Min(remaining, count);
+                        buffer.AsSpan(offset, send).Fill(65);
+                        remaining -= send;
+                        return Task.FromResult(send);
+                    }
+                    return Task.FromResult(0);
+                })), "test");
+            req.Content = content;
+
+            req.Content.Headers.Add("Content-MD5-Skip", "browser");
+
+            using HttpClient client = CreateHttpClientForRemoteServer(Configuration.Http.RemoteHttp2Server);
+            using HttpResponseMessage response = await client.SendAsync(req);
+            Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode);
+        }
+
+
         // Duplicate of PostAsync_ThrowFromContentCopy_RequestFails using remote server
-        [OuterLoop]
         [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsChromium))]
         [InlineData(false)]
         [InlineData(true)]
@@ -357,20 +392,19 @@ public async Task BrowserHttpHandler_StreamingRequest_ThrowFromContentCopy_Reque
         }
 
         public static TheoryData CancelRequestReadFunctions
-            => new TheoryData<bool, Func<Task<int>>>
+            => new TheoryData<bool, int, bool>
             {
-                { false, () => Task.FromResult(0) },
-                { true, () => Task.FromResult(0) },
-                { false, () => Task.FromResult(1) },
-                { true, () => Task.FromResult(1) },
-                { false, () => throw new FormatException() },
-                { true, () => throw new FormatException() },
+                { false, 0, false },
+                { true,  0, false },
+                { false, 1, false },
+                { true,  1, false },
+                { false, 0, true },
+                { true,  0, true },
             };
 
-        [OuterLoop]
         [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsChromium))]
         [MemberData(nameof(CancelRequestReadFunctions))]
-        public async Task BrowserHttpHandler_StreamingRequest_CancelRequest(bool cancelAsync, Func<Task<int>> readFunc)
+        public async Task BrowserHttpHandler_StreamingRequest_CancelRequest(bool cancelAsync, int bytes, bool throwException)
         {
             var WebAssemblyEnableStreamingRequestKey = new HttpRequestOptionsKey<bool>("WebAssemblyEnableStreamingRequest");
 
@@ -397,13 +431,26 @@ public async Task BrowserHttpHandler_StreamingRequest_CancelRequest(bool cancelA
                     {
                         readCancelledCount++;
                     }
-                    return await readFunc();
+                    if (throwException)
+                    {
+                        throw new FormatException("Test");
+                    }
+                    return await Task.FromResult(bytes);
                 }));
 
             using (HttpClient client = CreateHttpClientForRemoteServer(Configuration.Http.RemoteHttp2Server))
             {
-                TaskCanceledException ex = await Assert.ThrowsAsync<TaskCanceledException>(() => client.SendAsync(req, token));
-                Assert.Equal(token, ex.CancellationToken);
+                Exception ex = await Assert.ThrowsAnyAsync<Exception>(() => client.SendAsync(req, token));
+                if(throwException)
+                {
+                    Assert.IsType<FormatException>(ex);
+                    Assert.Equal("Test", ex.Message);
+                }
+                else
+                {
+                    var tce = Assert.IsType<TaskCanceledException>(ex);
+                    Assert.Equal(token, tce.CancellationToken);
+                }
                 Assert.Equal(1, readNotCancelledCount);
                 Assert.Equal(0, readCancelledCount);
             }

diff --git a/src/libraries/Common/tests/System/Net/Prerequisites/NetCoreServer/GenericHandler.cs b/src/libraries/Common/tests/System/Net/Prerequisites/NetCoreServer/GenericHandler.cs
@@ -51,7 +51,7 @@ public async Task Invoke(HttpContext context)
 
             if (path.Equals(new PathString("/statuscode.ashx")))
             {
-                StatusCodeHandler.Invoke(context);
+                await StatusCodeHandler.InvokeAsync(context);
                 return;
             }
 

diff --git a/...braries/Common/tests/System/Net/Prerequisites/NetCoreServer/Handlers/StatusCodeHandler.cs b/...braries/Common/tests/System/Net/Prerequisites/NetCoreServer/Handlers/StatusCodeHandler.cs
@@ -3,21 +3,37 @@
 
 using System;
 using Microsoft.AspNetCore.Http;
+using System.Threading;
+using System.Threading.Tasks;
+using System.Net.Http;
 
 namespace NetCoreServer
 {
     public class StatusCodeHandler
     {
-        public static void Invoke(HttpContext context)
+        public static async Task InvokeAsync(HttpContext context)
         {
             string statusCodeString = context.Request.Query["statuscode"];
             string statusDescription = context.Request.Query["statusdescription"];
+            string delayString = context.Request.Query["delay"];
             try
             {
                 int statusCode = int.Parse(statusCodeString);
+                int delay = string.IsNullOrWhiteSpace(delayString) ? 0 : int.Parse(delayString);
+
                 context.Response.StatusCode = statusCode;
-                context.Response.SetStatusDescription(
-                    string.IsNullOrWhiteSpace(statusDescription) ? " " : statusDescription);
+                context.Response.SetStatusDescription(string.IsNullOrWhiteSpace(statusDescription) ? " " : statusDescription);
+
+                if (delay > 0)
+                {
+                    var buffer = new byte[1];
+                    if (context.Request.Method == HttpMethod.Post.Method)
+                    {
+                        await context.Request.Body.ReadExactlyAsync(buffer, CancellationToken.None);
+                    }
+                    await context.Response.StartAsync(CancellationToken.None);
+                    await Task.Delay(delay);
+                }
             }
             catch (Exception)
             {

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/BrowserHttpHandler.cs b/src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/BrowserHttpHandler.cs
@@ -160,7 +160,7 @@ public BrowserHttpController(HttpRequestMessage request, bool? allowAutoRedirect
 
                 if (!_httpController.IsDisposed)
                 {
-                    BrowserHttpInterop.AbortRequest(_httpController);
+                    BrowserHttpInterop.Abort(_httpController);
                 }
             }, httpController);
 
@@ -248,9 +248,16 @@ public async Task<HttpResponseMessage> CallFetch()
                     {
                         fetchPromise = BrowserHttpInterop.FetchStream(_jsController, uri, _headerNames, _headerValues, _optionNames, _optionValues);
                         writeStream = new BrowserHttpWriteStream(this);
-                        await _request.Content.CopyToAsync(writeStream, _cancellationToken).ConfigureAwait(false);
-                        var closePromise = BrowserHttpInterop.TransformStreamClose(_jsController);
-                        await BrowserHttpInterop.CancellationHelper(closePromise, _cancellationToken, _jsController).ConfigureAwait(false);
+                        try
+                        {
+                            await _request.Content.CopyToAsync(writeStream, _cancellationToken).ConfigureAwait(false);
+                            var closePromise = BrowserHttpInterop.TransformStreamClose(_jsController);
+                            await BrowserHttpInterop.CancellationHelper(closePromise, _cancellationToken, _jsController).ConfigureAwait(false);
+                        }
+                        catch(JSException jse) when (jse.Message.Contains("BrowserHttpWriteStream.Rejected", StringComparison.Ordinal))
+                        {
+                            // any error from pushing bytes will also appear in the fetch promise result
+                        }
                     }
                     else
                     {
@@ -344,7 +351,7 @@ public void Dispose()
             {
                 if (!_jsController.IsDisposed)
                 {
-                    BrowserHttpInterop.AbortRequest(_jsController);// aborts also response
+                    BrowserHttpInterop.Abort(_jsController);// aborts also response
                 }
                 _jsController.Dispose();
             }

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/BrowserHttpInterop.cs b/src/libraries/System.Net.Http/src/System/Net/Http/BrowserHttpHandler/BrowserHttpInterop.cs
@@ -20,13 +20,8 @@ internal static partial class BrowserHttpInterop
         [JSImport("INTERNAL.http_wasm_create_controller")]
         public static partial JSObject CreateController();
 
-        [JSImport("INTERNAL.http_wasm_abort_request")]
-        public static partial void AbortRequest(
-            JSObject httpController);
-
-        [JSImport("INTERNAL.http_wasm_abort_response")]
-        public static partial void AbortResponse(
-            JSObject httpController);
+        [JSImport("INTERNAL.http_wasm_abort")]
+        public static partial void Abort(JSObject httpController);
 
         [JSImport("INTERNAL.http_wasm_transform_stream_write")]
         public static partial Task TransformStreamWrite(
@@ -143,7 +138,7 @@ public static async Task CancellationHelper(Task promise, CancellationToken canc
                     CancelablePromise.CancelPromise(_promise);
                     if (!_jsController.IsDisposed)
                     {
-                        AbortResponse(_jsController);
+                        Abort(_jsController);
                     }
                 }, (promise, jsController)))
                 {
@@ -160,6 +155,10 @@ public static async Task CancellationHelper(Task promise, CancellationToken canc
                 {
                     throw Http.CancellationHelper.CreateOperationCanceledException(jse, CancellationToken.None);
                 }
+                if (jse.Message.Contains("BrowserHttpWriteStream.Rejected", StringComparison.Ordinal))
+                {
+                    throw; // do not translate
+                }
                 Http.CancellationHelper.ThrowIfCancellationRequested(jse, cancellationToken);
                 throw new HttpRequestException(jse.Message, jse);
             }

diff --git a/src/mono/browser/runtime/exports-internal.ts b/src/mono/browser/runtime/exports-internal.ts
@@ -6,7 +6,7 @@ import WasmEnableThreads from "consts:wasmEnableThreads";
 import { MonoObjectNull, type MonoObject } from "./types/internal";
 import cwraps, { profiler_c_functions, threads_c_functions as twraps } from "./cwraps";
 import { mono_wasm_send_dbg_command_with_parms, mono_wasm_send_dbg_command, mono_wasm_get_dbg_command_info, mono_wasm_get_details, mono_wasm_release_object, mono_wasm_call_function_on, mono_wasm_debugger_resume, mono_wasm_detach_debugger, mono_wasm_raise_debug_event, mono_wasm_change_debugger_log_level, mono_wasm_debugger_attached } from "./debug";
-import { http_wasm_supports_streaming_request, http_wasm_supports_streaming_response, http_wasm_create_controller, http_wasm_abort_request, http_wasm_abort_response, http_wasm_transform_stream_write, http_wasm_transform_stream_close, http_wasm_fetch, http_wasm_fetch_stream, http_wasm_fetch_bytes, http_wasm_get_response_header_names, http_wasm_get_response_header_values, http_wasm_get_response_bytes, http_wasm_get_response_length, http_wasm_get_streamed_response_bytes, http_wasm_get_response_type, http_wasm_get_response_status } from "./http";
+import { http_wasm_supports_streaming_request, http_wasm_supports_streaming_response, http_wasm_create_controller, http_wasm_abort, http_wasm_transform_stream_write, http_wasm_transform_stream_close, http_wasm_fetch, http_wasm_fetch_stream, http_wasm_fetch_bytes, http_wasm_get_response_header_names, http_wasm_get_response_header_values, http_wasm_get_response_bytes, http_wasm_get_response_length, http_wasm_get_streamed_response_bytes, http_wasm_get_response_type, http_wasm_get_response_status } from "./http";
 import { exportedRuntimeAPI, Module, runtimeHelpers } from "./globals";
 import { get_property, set_property, has_property, get_typeof_property, get_global_this, dynamic_import } from "./invoke-js";
 import { mono_wasm_stringify_as_error_with_stack } from "./logging";
@@ -80,8 +80,7 @@ export function export_internal (): any {
         http_wasm_create_controller,
         http_wasm_get_response_type,
         http_wasm_get_response_status,
-        http_wasm_abort_request,
-        http_wasm_abort_response,
+        http_wasm_abort,
         http_wasm_transform_stream_write,
         http_wasm_transform_stream_close,
         http_wasm_fetch,

diff --git a/src/mono/browser/runtime/http.ts b/src/mono/browser/runtime/http.ts
@@ -72,30 +72,31 @@ export function http_wasm_create_controller (): HttpController {
     return controller;
 }
 
-export function http_wasm_abort_request (controller: HttpController): void {
-    try {
-        if (controller.streamWriter) {
-            controller.streamWriter.abort();
+function handle_abort_error (promise:Promise<any>) {
+    promise.catch((err) => {
+        if (err && err !== "AbortError" && err.name !== "AbortError" ) {
+            Module.err("Unexpected error: " + err);
         }
-    } catch (err) {
-        // ignore
-    }
-    http_wasm_abort_response(controller);
+        // otherwise, it's expected
+    });
 }
 
-export function http_wasm_abort_response (controller: HttpController): void {
+export function http_wasm_abort (controller: HttpController): void {
     if (BuildConfiguration === "Debug") commonAsserts(controller);
     try {
-        controller.isAborted = true;
-        if (controller.streamReader) {
-            controller.streamReader.cancel().catch((err) => {
-                if (err && err.name !== "AbortError") {
-                    Module.err("Error in http_wasm_abort_response: " + err);
-                }
-                // otherwise, it's expected
-            });
+        if (!controller.isAborted) {
+            if (controller.streamWriter) {
+                handle_abort_error(controller.streamWriter.abort());
+                controller.isAborted = true;
+            }
+            if (controller.streamReader) {
+                handle_abort_error(controller.streamReader.cancel());
+                controller.isAborted = true;
+            }
+        }
+        if (!controller.isAborted) {
+            controller.abortController.abort("AbortError");
         }
-        controller.abortController.abort();
     } catch (err) {
         // ignore
     }
@@ -110,9 +111,12 @@ export function http_wasm_transform_stream_write (controller: HttpController, bu
     return wrap_as_cancelable_promise(async () => {
         mono_assert(controller.streamWriter, "expected streamWriter");
         mono_assert(controller.responsePromise, "expected fetch promise");
-        // race with fetch because fetch does not cancel the ReadableStream see https://bugs.chromium.org/p/chromium/issues/detail?id=1480250
-        await Promise.race([controller.streamWriter.ready, controller.responsePromise]);
-        await Promise.race([controller.streamWriter.write(copy), controller.responsePromise]);
+        try {
+            await controller.streamWriter.ready;
+            await controller.streamWriter.write(copy);
+        } catch (ex) {
+            throw new Error("BrowserHttpWriteStream.Rejected");
+        }
     });
 }
 
@@ -121,16 +125,21 @@ export function http_wasm_transform_stream_close (controller: HttpController): C
     return wrap_as_cancelable_promise(async () => {
         mono_assert(controller.streamWriter, "expected streamWriter");
         mono_assert(controller.responsePromise, "expected fetch promise");
-        // race with fetch because fetch does not cancel the ReadableStream see https://bugs.chromium.org/p/chromium/issues/detail?id=1480250
-        await Promise.race([controller.streamWriter.ready, controller.responsePromise]);
-        await Promise.race([controller.streamWriter.close(), controller.responsePromise]);
+        try {
+            await controller.streamWriter.ready;
+            await controller.streamWriter.close();
+        } catch (ex) {
+            throw new Error("BrowserHttpWriteStream.Rejected");
+        }
     });
 }
 
 export function http_wasm_fetch_stream (controller: HttpController, url: string, header_names: string[], header_values: string[], option_names: string[], option_values: any[]): ControllablePromise<void> {
     if (BuildConfiguration === "Debug") commonAsserts(controller);
     const transformStream = new TransformStream<Uint8Array, Uint8Array>();
     controller.streamWriter = transformStream.writable.getWriter();
+    handle_abort_error(controller.streamWriter.closed);
+    handle_abort_error(controller.streamWriter.ready);
     const fetch_promise = http_wasm_fetch(controller, url, header_names, header_values, option_names, option_values, transformStream.readable);
     return fetch_promise;
 }

diff --git a/src/mono/browser/runtime/loader/exit.ts b/src/mono/browser/runtime/loader/exit.ts
@@ -291,11 +291,11 @@ function logOnExit (exit_code: number, reason: any) {
         }
     }
 }
-function unhandledrejection_handler (event: any) {
+function unhandledrejection_handler (event: PromiseRejectionEvent) {
     fatal_handler(event, event.reason, "rejection");
 }
 
-function error_handler (event: any) {
+function error_handler (event: ErrorEvent) {
     fatal_handler(event, event.error, "error");
 }