Skip to content

Use CreateAsyncScope instead of CreateScope for ExceptionHandler middleware #52687

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Feb 13, 2024
Merged

Use CreateAsyncScope instead of CreateScope for ExceptionHandler middleware #52687

merged 1 commit into from
Feb 13, 2024

Conversation

Kahbazi
Copy link
Member

@Kahbazi Kahbazi commented Dec 9, 2023

No description provided.

@ghost ghost added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions community-contribution Indicates that the PR has been added by a community member labels Dec 9, 2023
@ghost
Copy link

ghost commented Dec 9, 2023

Thanks for your PR, @Kahbazi. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@Kahbazi Kahbazi added feature-diagnostics Diagnostic middleware and pages (except EF diagnostics) and removed area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions labels Dec 9, 2023
@ghost ghost added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Dec 20, 2023
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment Feb 13, 2024
@dotnet-policy-service dotnet-policy-service bot added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Feb 13, 2024
@halter73 halter73 merged commit 0e9ab17 into dotnet:main Feb 13, 2024
@halter73
Copy link
Member

Thanks!

@dotnet-policy-service dotnet-policy-service bot added this to the 9.0-preview2 milestone Feb 13, 2024
@Kahbazi Kahbazi deleted the kahbazi/disposeAsyncScope branch February 13, 2024 01:35
@ScarletKuro ScarletKuro mentioned this pull request Feb 23, 2024
Closed
2 tasks
@ScarletKuro
Copy link

ScarletKuro commented Feb 23, 2024 •
edited
Loading

Hi.
I'm wondering if this could be back ported to .NET8 and preferably .NET7, this is causing inconvenience in MudBlazor (MudBlazor/MudBlazor#7879) because many of our services implement only IAsyncDisposable for a reason.
This seems like a bug to me, that it's calling CreateScope instead of CreateAsyncScope. Also this problem is "hard" to discover because it only shows up when UseExceptionHandler is used with CreateScopeForErrors which is usually enabled only when the app.Environment.IsDevelopment() is not true.

I don't see any breaking change risks here.

@ScarletKuro ScarletKuro mentioned this pull request May 26, 2024
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@halter73 halter73 halter73 approved these changes

@BrennanConroy BrennanConroy Awaiting requested review from BrennanConroy BrennanConroy is a code owner

Assignees

@adityamandaleeka adityamandaleeka

Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares community-contribution Indicates that the PR has been added by a community member feature-diagnostics Diagnostic middleware and pages (except EF diagnostics)
Projects
None yet
Milestone
9.0-preview2
Development

Successfully merging this pull request may close these issues.
5 participants
@Kahbazi @halter73 @ScarletKuro @adityamandaleeka @wtgodbe