Skip to content

warn about registering customer service /5#29221

Merged
Rick-Anderson merged 3 commits intomainfrom
probDetails
May 12, 2023
Merged

warn about registering customer service /5#29221
Rick-Anderson merged 3 commits intomainfrom
probDetails

Conversation

@Rick-Anderson
Copy link
Copy Markdown
Contributor

@Rick-Anderson Rick-Anderson commented May 10, 2023

Fixes #29152

Bruno, can't we get an ASPnnnn code analysis to detect this?

Review URL internal only


Internal previews

📄 File 🔗 Preview link
aspnetcore/fundamentals/error-handling.md Handle errors in ASP.NET Core

@Rick-Anderson Rick-Anderson marked this pull request as ready for review May 10, 2023 00:11
@Rick-Anderson Rick-Anderson requested a review from brunolins16 May 10, 2023 00:11
@brunolins16
Copy link
Copy Markdown
Member

Bruno, can't we get an ASPnnnn code analysis to detect this?

@mitchdenny @captainsafia What do you think about this?

@Rick-Anderson
Copy link
Copy Markdown
Contributor Author

cc @david-acker

2. Use a custom [`IProblemDetailsWriter`](#custom-iproblemdetailswriter)
3. Call the [`IProblemDetailsService` in a middleware](#problem-details-from-middleware)

***Note:*** When using a custom `IProblemDetailsWriter`, the custom `IProblemDetailsWriter` must be registered before calling <xref:Microsoft.Extensions.DependencyInjection.MvcServiceCollectionExtensions.AddRazorPages%2A>, <xref:Microsoft.Extensions.DependencyInjection.MvcServiceCollectionExtensions.AddControllers%2A>, or <xref:Microsoft.Extensions.DependencyInjection.MvcServiceCollectionExtensions.AddControllersWithViews%2A>.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about adding it under https://review.learn.microsoft.com/en-us/aspnet/core/fundamentals/error-handling?branch=pr-en-us-29221&view=aspnetcore-8.0#custom-iproblemdetailswriter, call out that it must be registered before AddProblemDetails as well and maybe add an example of registration?

@david-acker
Copy link
Copy Markdown
Member

It seems like adding an analyzer to detect this would be straightforward. The one for ASP0001 serves a similar purpose.

@Rick-Anderson
Copy link
Copy Markdown
Contributor Author

Rick-Anderson commented May 10, 2023

It seems like adding an analyzer to detect this would be straightforward. The one for ASP0001 serves a similar purpose.

@david-acker can you review dotnet/aspnetcore#48178 dotnet/aspnetcore#48180 and suggest any improvements?

@Rick-Anderson Rick-Anderson enabled auto-merge (squash) May 12, 2023 02:07
@Rick-Anderson Rick-Anderson merged commit c7dec65 into main May 12, 2023
@Rick-Anderson Rick-Anderson deleted the probDetails branch May 12, 2023 02:12
@david-acker
Copy link
Copy Markdown
Member

david-acker commented May 12, 2023

dotnet/aspnetcore#48180 LGTM! When I get a chance, I'll take a look at the code for the ASP0001 analyzer and think of some suggestions based on that.

@captainsafia
Copy link
Copy Markdown
Contributor

@david-acker Thanks David! You rock! 🎸

Donciavas pushed a commit to Donciavas/AspNetCore.Docs that referenced this pull request Feb 7, 2024
* warn about registering customer service /5

* prob det

* prob det
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Problem details

4 participants