Adding ProblemDetailsService#42384
Conversation
Co-authored-by: Brennan <brecon@microsoft.com>
src/Http/Http.Abstractions/src/ProblemDetails/ProblemDetailsContext.cs
Outdated
Show resolved
Hide resolved
|
What does the performance of this look like? I see context objects and state machines that can't be avoided? |
I was expecting this question :) ... I ran crank and the results are here: The benchmark was against a simple Also, having the default ExceptionHandler middleware performance is better but using I will open an issue to work on improvements of those middleware. |
halter73
left a comment
There was a problem hiding this comment.
Looks good!
If we're always going to return HasStarted from TryWriteAsync, we might as well have it not even return a bool. I think it's fine to assume WriteAsync methods did in fact write something in our IProblemDetailsWriter implementations.
src/Http/Http.Abstractions/src/Microsoft.AspNetCore.Http.Abstractions.csproj
Outdated
Show resolved
Hide resolved
src/Middleware/MiddlewareAnalysis/test/MiddlewareAnalysisTests.cs
Outdated
Show resolved
Hide resolved
src/Mvc/Mvc.Core/src/Infrastructure/DefaultApiProblemDetailsWriter.cs
Outdated
Show resolved
Hide resolved
src/Mvc/Mvc.Core/src/Infrastructure/DefaultApiProblemDetailsWriter.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Stephen Halter <halter73@gmail.com>
| var httpContext = context.HttpContext; | ||
| var acceptHeader = httpContext.Request.Headers.Accept.GetList<MediaTypeHeaderValue>(); | ||
|
|
||
| if (acceptHeader?.Any(h => _jsonMediaType.IsSubsetOf(h) || _problemDetailsJsonMediaType.IsSubsetOf(h)) == true) |
There was a problem hiding this comment.
This lambda probably allocates per call, we should hand write .Any
|
/backport to release/7.0-preview7 |
|
Hi @brunolins16. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
|
Started backporting to release/7.0-preview7: https://github.com/dotnet/aspnetcore/actions/runs/2668518163 |
|
@brunolins16 backporting to release/7.0-preview7 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: MVC Changes
Applying: ExceptionHandler changes
.git/rebase-apply/patch:126: trailing whitespace.
///
warning: 1 line adds whitespace errors.
Using index info to reconstruct a base tree...
M src/Middleware/Diagnostics/src/PublicAPI.Unshipped.txt
Falling back to patching base and 3-way merge...
Auto-merging src/Middleware/Diagnostics/src/PublicAPI.Unshipped.txt
CONFLICT (content): Merge conflict in src/Middleware/Diagnostics/src/PublicAPI.Unshipped.txt
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 ExceptionHandler changes
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
* MVC Changes * ExceptionHandler changes * Routing changes * Http.Extensions changes * Minimal APi draft changes * HttpResults changes * New ProblemDetails project * Using ProblemDetailsOptions in mvc * ProblemDetails project simplification * Using metadata * Using metadata * Latest version * Removing changes * Initial cleanup * Clean up * Public api clean up * Updating public API * More clean ups * Adding initial unit tests * Updating unit tests * Adding more unit tests * Cleanup * Clean up * clean up * clean up * Removing nullable * Simplifying public api * API Review feedback * API review feedback * Clean up * Clean up * clean up * Reusing Endpoint & EndpointMetadataCollection * Fix build issues * Updates based on docs/Trimming.md * Adding Functional tests * Fix unittest * Seal context * Seal ProblemMetadata * PR Feeback * API Review * Clean up * Fixing publicapi warnings * Clean up * PR Feedback * Fixing build * Fix unit test * Fix unit tests * PR review * Fixing JsonSerializationContext issues * Adding statuscode 405 * Update src/Http/Http.Extensions/src/ProblemDetailsDefaultWriter.cs Co-authored-by: Brennan <brecon@microsoft.com> * PR review * Apply suggestions from code review Co-authored-by: Stephen Halter <halter73@gmail.com> * Fixing bad merge * Fix bad merge * Adding analysis.NextMiddlewareName * PR review * Changing to CanWrite/WriteAsync Co-authored-by: Brennan <brecon@microsoft.com> Co-authored-by: Stephen Halter <halter73@gmail.com>
* MVC Changes * ExceptionHandler changes * Routing changes * Http.Extensions changes * Minimal APi draft changes * HttpResults changes * New ProblemDetails project * Using ProblemDetailsOptions in mvc * ProblemDetails project simplification * Using metadata * Using metadata * Latest version * Removing changes * Initial cleanup * Clean up * Public api clean up * Updating public API * More clean ups * Adding initial unit tests * Updating unit tests * Adding more unit tests * Cleanup * Clean up * clean up * clean up * Removing nullable * Simplifying public api * API Review feedback * API review feedback * Clean up * Clean up * clean up * Reusing Endpoint & EndpointMetadataCollection * Fix build issues * Updates based on docs/Trimming.md * Adding Functional tests * Fix unittest * Seal context * Seal ProblemMetadata * PR Feeback * API Review * Clean up * Fixing publicapi warnings * Clean up * PR Feedback * Fixing build * Fix unit test * Fix unit tests * PR review * Fixing JsonSerializationContext issues * Adding statuscode 405 * Update src/Http/Http.Extensions/src/ProblemDetailsDefaultWriter.cs Co-authored-by: Brennan <brecon@microsoft.com> * PR review * Apply suggestions from code review Co-authored-by: Stephen Halter <halter73@gmail.com> * Fixing bad merge * Fix bad merge * Adding analysis.NextMiddlewareName * PR review * Changing to CanWrite/WriteAsync Co-authored-by: Brennan <brecon@microsoft.com> Co-authored-by: Stephen Halter <halter73@gmail.com> Co-authored-by: Brennan <brecon@microsoft.com> Co-authored-by: Stephen Halter <halter73@gmail.com>
Close #42212