Skip to content

Add CreateScopeForStatusCodePages property and UseStatusCodePagesWithReExecute overload #62624

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.

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

Merged
merged 15 commits into from
Jul 21, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jul 9, 2025

This PR implements the approved API proposal to add support for creating DI scopes when re-executing requests in StatusCodePages middleware.

Changes Made

API Surface Changes

StatusCodePagesOptions:

  • Renamed CreateScopeForErrorsCreateScopeForStatusCodePages property
  • Property defaults to false to maintain existing behavior

StatusCodePagesExtensions:

  • Modified existing UseStatusCodePagesWithReExecute(string, string?) to remove default parameter
  • Added new overload: UseStatusCodePagesWithReExecute(string, string? = null, bool createScopeForStatusCodePages = false)

Usage Examples

// Default behavior (no scope creation)
app.UseStatusCodePagesWithReExecute("/error/{0}");

// With scope creation enabled
app.UseStatusCodePagesWithReExecute("/error/{0}", createScopeForStatusCodePages: true);

// With query format and scope creation
app.UseStatusCodePagesWithReExecute("/error/{0}", "?code={0}", createScopeForStatusCodePages: true);

Implementation Details

  • Updated internal CreateHandler method to use the new property name
  • Added comprehensive tests to verify the new functionality
  • Updated PublicAPI files to reflect the API changes
  • Maintained backward compatibility

Benefits

This change allows error handling middleware to run in a fresh DI scope, which is important for scenarios where:

  • Scoped services should not be shared between the original and re-executed requests
  • Error handling requires isolated service state
  • Similar to the existing CreateScopeForErrors functionality in ExceptionHandler

The API follows the same pattern as ExceptionHandlerOptions.CreateScopeForErrors for consistency.

Fixes #62408.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@ilonatommy
Copy link
Member

@copilot I updated the description. Make your updates based on the required updates listed there.

@Copilot Copilot AI changed the title [WIP] Api proposal for UseStatusCodePagesWithReExecute Add CreateScopeForStatusCodePages property and UseStatusCodePagesWithReExecute overload Jul 9, 2025
@Copilot Copilot AI requested a review from ilonatommy July 9, 2025 07:37
Copilot finished work on behalf of ilonatommy July 9, 2025 07:37
Copy link
Contributor Author

Copilot AI commented Jul 9, 2025

@copilot I updated the description. Make your updates based on the required updates listed there.

My implementation already aligns with the updated description requirements. The changes include:

✅ Renamed CreateScopeForErrorsCreateScopeForStatusCodePages property
✅ Modified existing method to remove default parameter
✅ Added new overload with correct parameter order and defaults
✅ Updated PublicAPI files accordingly
✅ All usage examples work as specified

Build and tests pass successfully. (b6bec12)

@Copilot Copilot AI requested a review from ilonatommy July 9, 2025 07:47
Copilot finished work on behalf of ilonatommy July 9, 2025 07:47
Copilot finished work on behalf of ilonatommy July 9, 2025 07:59
Copilot finished work on behalf of ilonatommy July 9, 2025 08:26
@ilonatommy
Copy link
Member

ilonatommy commented Jul 9, 2025

@halter73, removing optionality in the old method, following the guidelines of roslyn, requires a change to Shipped.txt. Is that all right in the current release phase?

Detected modification to baseline API files. PublicAPI.Shipped.txt files should only be updated after a major release, and PublicAPI.Unshipped.txt files should not be updated in release branches.

@ilonatommy ilonatommy marked this pull request as ready for review July 9, 2025 10:32
@ilonatommy ilonatommy requested review from tdykstra, a team and BrennanConroy as code owners July 9, 2025 10:32
@ilonatommy
Copy link
Member

@halter73, removing optionality in the old method, following the guidelines of roslyn, requires a change to Shipped.txt. Is that all right in the current release phase?

Detected modification to baseline API files. PublicAPI.Shipped.txt files should only be updated after a major release, and PublicAPI.Unshipped.txt files should not be updated in release branches.

Solved, @javiercn pointed me to

*REMOVED*Microsoft.Builder.OldApplicationBuilder.New() -> Microsoft.AspNetCore.Builder.IApplicationBuilder!

Copy link
Contributor Author

Copilot AI commented Jul 15, 2025

Templates test with 10.0::10.0.0-preview.7.25364.110, not with the current state of repo. They fail with

src\ProjectTemplates\scripts\MyBlazorApp\Program.cs(18,51): error CS1739: The best over...

Reverted changes to template files in the BlazorWeb-CSharp directory. The templates now use createScopeForErrors: true instead of createScopeForStatusCodePages: true to be compatible with the older version they are tested against. (07f562f)

Copilot finished work on behalf of ilonatommy July 15, 2025 08:39
@ilonatommy
Copy link
Member

ilonatommy commented Jul 16, 2025

I don't get it. If we don't change templates, they fail with

src\ProjectTemplates\test\Templates.Blazor.Tests\bin\Release\net10.0\TestTemplates\AspNet.vmsroat00q32\Program.cs(18,51): error CS1739: The best overload for 'UseStatusCodePagesWithReExecute' does not have a parameter named 'createScopeForErrors'

If we change them, they fail with

src\ProjectTemplates\scripts\MyBlazorApp\Program.cs(18,51): error CS1739: The best overload for 'UseStatusCodePagesWithReExecute' does not have a parameter named 'createScopeForStatusCodePages' 

Locally I'm hitting #62746. We cannot merge until I figure out how to solve it.

edit: on my machine all the tests are passing. Trying to find a reason for CI to behave differently.

@ilonatommy
Copy link
Member

@copilot remove line "app.UseStatusCodePagesWithReExecute("/not-found", createScopeForStatusCodePages: true);" from src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Program.Main.cs and src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1/Program.cs.

We're having failures in both cases: if we update the templates and when we won't do that. This line should be added in a follow-up PR when the template sdk that we're using for *Locally.ps1 tests will already contain the rename change.

Copy link
Contributor Author

Copilot AI commented Jul 21, 2025

@copilot remove line "app.UseStatusCodePagesWithReExecute("/not-found", createScopeForStatusCodePages: true);" from src/ProjectTemplates/Web.ProjectTemplates/content/BlazorWeb-CSharp/BlazorWebCSharp.1...

Removed the UseStatusCodePagesWithReExecute calls from both template files as requested. This will prevent CI failures until the template SDK is updated with the new parameter in a follow-up PR.

Copilot finished work on behalf of ilonatommy July 21, 2025 10:39
@ilonatommy ilonatommy merged commit 1730530 into main Jul 21, 2025
30 checks passed
@ilonatommy ilonatommy deleted the copilot/fix-62408 branch July 21, 2025 15:08
@ilonatommy
Copy link
Member

@coplot, create an issue about sdk problems with updating public API in templates. Use comments about template tests failures from this PR and from #62768. Are you able to do that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Api proposal for UseStatusCodePagesWithReExecute
4 participants