Introducing IProblemDetailsService.TryWriteAsync (#46680)

* Introducing IProblemDetailsService.TryWriteAsync

* Change everywhere to use TryWriteAsync

* API review updates

* Update PublicAPI.Unshipped.txt

* Update PublicAPI.Unshipped.txt

Author: brunolins16

src/Http/Http.Abstractions/src/ProblemDetails/IProblemDetailsService.cs

@@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Http;
 public interface IProblemDetailsService
 {
     /// <summary>
-    /// Try to write a <see cref="Mvc.ProblemDetails"/> response to the current context,
+    /// Write a <see cref="Mvc.ProblemDetails"/> response to the current context,
     /// using the registered <see cref="IProblemDetailsWriter"/> services.
     /// </summary>
     /// <param name="context">The <see cref="ProblemDetailsContext"/> associated with the current request/response.</param>
@@ -21,5 +21,24 @@ public interface IProblemDetailsService
     /// <item><description>All <see cref="IProblemDetailsWriter"/> were executed and none of them was able to write the response successfully.</description></item>
     /// </list>
     /// </remarks>
+    /// <exception cref="InvalidOperationException">If no <see cref="IProblemDetailsWriter"/> can write to the given context.</exception>
     ValueTask WriteAsync(ProblemDetailsContext context);
+
+    /// <summary>
+    /// Try to write a <see cref="Mvc.ProblemDetails"/> response to the current context,
+    /// using the registered <see cref="IProblemDetailsWriter"/> services.
+    /// </summary>
+    /// <param name="context">The <see cref="ProblemDetailsContext"/> associated with the current request/response.</param>
+    /// <remarks>The <see cref="IProblemDetailsWriter"/> registered services
+    /// are processed in sequence and the processing is completed when:
+    /// <list type="bullet">
+    /// <item><description>One of them reports that the response was written successfully, or.</description></item>
+    /// <item><description>All <see cref="IProblemDetailsWriter"/> were executed and none of them was able to write the response successfully.</description></item>
+    /// </list>
+    /// </remarks>
+    async ValueTask<bool> TryWriteAsync(ProblemDetailsContext context)
+    {
+        await WriteAsync(context);
+        return context.HttpContext.Response.HasStarted;
+    }
 }

src/Http/Http.Abstractions/src/PublicAPI.Unshipped.txt

@@ -5,5 +5,6 @@ Microsoft.AspNetCore.Http.DefaultEndpointFilterInvocationContext.DefaultEndpoint
 Microsoft.AspNetCore.Http.HttpResults.EmptyHttpResult
 Microsoft.AspNetCore.Http.HttpResults.EmptyHttpResult.ExecuteAsync(Microsoft.AspNetCore.Http.HttpContext! httpContext) -> System.Threading.Tasks.Task!
 Microsoft.AspNetCore.Http.HttpValidationProblemDetails.Errors.set -> void
+Microsoft.AspNetCore.Http.IProblemDetailsService.TryWriteAsync(Microsoft.AspNetCore.Http.ProblemDetailsContext! context) -> System.Threading.Tasks.ValueTask<bool>
 Microsoft.AspNetCore.Mvc.ProblemDetails.Extensions.set -> void
 static Microsoft.AspNetCore.Http.HttpResults.EmptyHttpResult.Instance.get -> Microsoft.AspNetCore.Http.HttpResults.EmptyHttpResult!

src/Http/Http.Extensions/src/ProblemDetailsService.cs

@@ -15,44 +15,33 @@ public ProblemDetailsService(
         _writers = writers.ToArray();
     }
 
-    public ValueTask WriteAsync(ProblemDetailsContext context)
+    public async ValueTask WriteAsync(ProblemDetailsContext context)
     {
-        ArgumentNullException.ThrowIfNull(context);
-        ArgumentNullException.ThrowIfNull(context.ProblemDetails);
-        ArgumentNullException.ThrowIfNull(context.HttpContext);
-
-        if (context.HttpContext.Response.HasStarted ||
-            context.HttpContext.Response.StatusCode < 400 ||
-            _writers.Length == 0)
+        if (!await TryWriteAsync(context))
         {
-            return ValueTask.CompletedTask;
+            throw new InvalidOperationException("Unable to find a registered `IProblemDetailsWriter` that can write to the given context.");
         }
+    }
 
-        IProblemDetailsWriter? selectedWriter = null;
-
-        if (_writers.Length == 1)
-        {
-            selectedWriter = _writers[0];
-
-            return selectedWriter.CanWrite(context) ?
-                selectedWriter.WriteAsync(context) :
-                ValueTask.CompletedTask;
-        }
+    public async ValueTask<bool> TryWriteAsync(ProblemDetailsContext context)
+    {
+        ArgumentNullException.ThrowIfNull(context);
+        ArgumentNullException.ThrowIfNull(context.ProblemDetails);
+        ArgumentNullException.ThrowIfNull(context.HttpContext);
 
+        // Try to write using all registered writers
+        // sequentially and stop at the first one that
+        // `canWrite`.
         for (var i = 0; i < _writers.Length; i++)
         {
-            if (_writers[i].CanWrite(context))
+            var selectedWriter = _writers[i];
+            if (selectedWriter.CanWrite(context))
             {
-                selectedWriter = _writers[i];
-                break;
+                await selectedWriter.WriteAsync(context);
+                return true;
             }
         }
 
-        if (selectedWriter != null)
-        {
-            return selectedWriter.WriteAsync(context);
-        }
-
-        return ValueTask.CompletedTask;
+        return false;
     }
 }

src/Http/Http.Extensions/test/ProblemDetailsServiceTest.cs

@@ -34,25 +34,49 @@ public async Task WriteAsync_Skip_NextWriters_WhenResponseAlreadyStarted()
     }
 
     [Fact]
-    public async Task WriteAsync_Skip_WhenNoWriterRegistered()
+    public async Task TryWriteAsync_ReturnsTrue_WhenAtLeastOneWriterCanWrite()
     {
         // Arrange
-        var service = CreateService();
+        var service = CreateService(
+            writers: new List<IProblemDetailsWriter>
+            {
+                new MetadataBasedWriter("FirstWriter", canWrite: false),
+                new MetadataBasedWriter("SecondWriter"),
+                new MetadataBasedWriter("FirstWriter"),
+            });
+
+        var metadata = new EndpointMetadataCollection(new SampleMetadata() { ContentType = "application/problem+json" });
         var stream = new MemoryStream();
         var context = new DefaultHttpContext()
         {
             Response = { Body = stream, StatusCode = StatusCodes.Status400BadRequest },
         };
 
         // Act
-        await service.WriteAsync(new() { HttpContext = context });
+        var result = await service.TryWriteAsync(new() { HttpContext = context, AdditionalMetadata = metadata });
 
         // Assert
-        Assert.Equal(string.Empty, Encoding.UTF8.GetString(stream.ToArray()));
+        Assert.True(result);
+        Assert.Equal("\"SecondWriter\"", Encoding.UTF8.GetString(stream.ToArray()));
+    }
+
+    [Fact]
+    public async Task WriteAsync_Throws_WhenNoWriterRegistered()
+    {
+        // Arrange
+        var service = CreateService();
+        var stream = new MemoryStream();
+        var context = new DefaultHttpContext()
+        {
+            Response = { Body = stream, StatusCode = StatusCodes.Status400BadRequest },
+        };
+
+        // Act
+        await Assert.ThrowsAsync<InvalidOperationException>(async () => await service.WriteAsync(new() { HttpContext = context }));
     }
 
     [Fact]
-    public async Task WriteAsync_Skip_WhenNoWriterCanWrite()
+    public async Task WriteAsync_Throws_WhenNoWriterCanWrite()
     {
         // Arrange
         var service = CreateService(
@@ -63,36 +87,46 @@ public async Task WriteAsync_Skip_WhenNoWriterCanWrite()
             Response = { Body = stream, StatusCode = StatusCodes.Status400BadRequest },
         };
 
+        // Act & Assert
+        await Assert.ThrowsAsync<InvalidOperationException>(async () => await service.WriteAsync(new() { HttpContext = context }));
+    }
+
+    [Fact]
+    public async Task TryWriteAsync_ReturnsFalse_WhenNoWriterRegistered()
+    {
+        // Arrange
+        var service = CreateService();
+        var stream = new MemoryStream();
+        var context = new DefaultHttpContext()
+        {
+            Response = { Body = stream, StatusCode = StatusCodes.Status400BadRequest },
+        };
+
         // Act
-        await service.WriteAsync(new() { HttpContext = context });
+        var result = await service.TryWriteAsync(new() { HttpContext = context });
 
         // Assert
+        Assert.False(result);
         Assert.Equal(string.Empty, Encoding.UTF8.GetString(stream.ToArray()));
     }
 
-    [Theory]
-    [InlineData(StatusCodes.Status100Continue)]
-    [InlineData(StatusCodes.Status200OK)]
-    [InlineData(StatusCodes.Status300MultipleChoices)]
-    [InlineData(399)]
-    public async Task WriteAsync_Skip_WhenSuccessStatusCode(int statusCode)
+    [Fact]
+    public async Task TryWriteAsync_ReturnsFalse_WhenNoWriterCanWrite()
     {
         // Arrange
         var service = CreateService(
             writers: new List<IProblemDetailsWriter> { new MetadataBasedWriter() });
         var stream = new MemoryStream();
         var context = new DefaultHttpContext()
         {
-            Response = { Body = stream, StatusCode = statusCode },
+            Response = { Body = stream, StatusCode = StatusCodes.Status400BadRequest },
         };
-        var metadata = new EndpointMetadataCollection(new SampleMetadata() { ContentType = "application/problem+json" });
-        context.SetEndpoint(new Endpoint(context => Task.CompletedTask, metadata, null));
 
-        // Act
-        await service.WriteAsync(new() { HttpContext = context });
+        // Act & Assert
+        var result = await service.TryWriteAsync(new() { HttpContext = context });
 
         // Assert
-        Assert.Equal(string.Empty, Encoding.UTF8.GetString(stream.ToArray()));
+        Assert.False(result);
     }
 
     private static ProblemDetailsService CreateService(

src/Middleware/Diagnostics/src/DeveloperExceptionPage/DeveloperExceptionPageMiddlewareImpl.cs

@@ -184,19 +184,8 @@ private async Task DisplayExceptionContent(ErrorContext errorContext)
     {
         var httpContext = errorContext.HttpContext;
 
-        if (_problemDetailsService != null)
-        {
-            var problemDetails = CreateProblemDetails(errorContext, httpContext);
-
-            await _problemDetailsService.WriteAsync(new()
-            {
-                HttpContext = httpContext,
-                ProblemDetails = problemDetails
-            });
-        }
-
-        // If the response has not started, assume the problem details was not written.
-        if (!httpContext.Response.HasStarted)
+        if (_problemDetailsService == null ||
+            !await _problemDetailsService.TryWriteAsync(new() { HttpContext = httpContext, ProblemDetails = CreateProblemDetails(errorContext, httpContext) }))
         {
             httpContext.Response.ContentType = "text/plain; charset=utf-8";
 

src/Middleware/Diagnostics/src/ExceptionHandler/ExceptionHandlerMiddlewareImpl.cs

@@ -154,13 +154,14 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed
             context.Response.StatusCode = DefaultStatusCode;
             context.Response.OnStarting(_clearCacheHeadersDelegate, context.Response);
 
+            var problemDetailsWritten = false;
             if (_options.ExceptionHandler != null)
             {
                 await _options.ExceptionHandler!(context);
             }
             else
             {
-                await _problemDetailsService!.WriteAsync(new ()
+                problemDetailsWritten = await _problemDetailsService!.TryWriteAsync(new()
                 {
                     HttpContext = context,
                     AdditionalMetadata = exceptionHandlerFeature.Endpoint?.Metadata,
@@ -169,7 +170,7 @@ private async Task HandleException(HttpContext context, ExceptionDispatchInfo ed
             }
 
             // If the response has already started, assume exception handler was successful.
-            if (context.Response.HasStarted || context.Response.StatusCode != StatusCodes.Status404NotFound || _options.AllowStatusCode404Response)
+            if (context.Response.HasStarted || problemDetailsWritten || context.Response.StatusCode != StatusCodes.Status404NotFound || _options.AllowStatusCode404Response)
             {
                 const string eventName = "Microsoft.AspNetCore.Diagnostics.HandledException";
                 if (_diagnosticListener.IsEnabled() && _diagnosticListener.IsEnabled(eventName))

src/Middleware/Diagnostics/src/StatusCodePage/StatusCodePagesOptions.cs

@@ -23,19 +23,12 @@ public StatusCodePagesOptions()
         HandleAsync = async context =>
         {
             var statusCode = context.HttpContext.Response.StatusCode;
+            var problemDetailsService = context.HttpContext.RequestServices.GetService<IProblemDetailsService>();
 
-            if (context.HttpContext.RequestServices.GetService<IProblemDetailsService>() is { } problemDetailsService)
-            {
-                await problemDetailsService.WriteAsync(new ()
-                {
-                    HttpContext = context.HttpContext,
-                    ProblemDetails = { Status = statusCode }
-                });
-            }
-
-            // TODO: Render with a pre-compiled html razor view.
-            if (!context.HttpContext.Response.HasStarted)
+            if (problemDetailsService == null ||
+                !await problemDetailsService.TryWriteAsync(new() { HttpContext = context.HttpContext, ProblemDetails = { Status = statusCode } }))
             {
+                // TODO: Render with a pre-compiled html razor view
                 var body = BuildResponseBody(statusCode);
 
                 context.HttpContext.Response.ContentType = "text/plain";

src/Mvc/Mvc.Core/src/Formatters/TextOutputFormatter.cs

@@ -132,7 +132,7 @@ public override Task WriteAsync(OutputFormatterWriteContext context)
 
             if (context.HttpContext.RequestServices.GetService<IProblemDetailsService>() is { } problemDetailsService)
             {
-                return problemDetailsService.WriteAsync(new ()
+                return problemDetailsService.TryWriteAsync(new ()
                 {
                     HttpContext = context.HttpContext,
                     ProblemDetails = { Status = statusCode }

src/Mvc/Mvc.Core/src/Infrastructure/ObjectResultExecutor.cs

@@ -103,7 +103,7 @@ private Task ExecuteAsyncCore(ActionContext context, ObjectResult result, Type?
 
             if (context.HttpContext.RequestServices.GetService<IProblemDetailsService>() is { } problemDetailsService)
             {
-                return problemDetailsService.WriteAsync(new()
+                return problemDetailsService.TryWriteAsync(new()
                 {
                     HttpContext = context.HttpContext,
                     ProblemDetails = { Status = statusCode }