Skip to content

Investigate interaction between UseProxyToSpaDevelopmentServer and UseEndpoints #45903

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

Closed
rafaelfgx opened this issue Jan 5, 2023 · 14 comments
Closed
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates

Comments

@rafaelfgx
Copy link

rafaelfgx commented Jan 5, 2023

No description provided.

@mkArtakMSFT
Copy link
Contributor

@captainsafia, @JamesNK do you know if is this a newly added analyzer suggesting this change and is the proposed change truly equivalent to the original code?

@mkArtakMSFT mkArtakMSFT added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-routing labels Jan 8, 2023
@javiercn
Copy link
Member

javiercn commented Jan 9, 2023

@rafaelfgx thanks for contacting us.

You shouldn't need to call app.UseRouting nor app.UseEndpoints if you are using minimal hosting (as you seem to be doing) as the host already wraps the pipeline with app.UseRouting and app.UseEndpoints.

You can call MapControllers directly on the builder. I believe mix and matching is not supported.

@javiercn javiercn added feature-minimal-hosting old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels and removed area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-routing labels Jan 9, 2023
@brunolins16
Copy link
Member

@rafaelfgx I have tested without app.UseRouting and app.UseEndpoints, as mentioned, and it should work. Can you share a repro?

@brunolins16 brunolins16 added the Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. label Jan 12, 2023
@ghost
Copy link

ghost commented Jan 12, 2023

Hi @rafaelfgx. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

@halter73
Copy link
Member

@brunolins16 Did you try with UseRouting but not UseEndpoints? That should also work.

@HarelM
Copy link

HarelM commented Jan 16, 2023

I've stumbled across this issue as well I believe as can be read in my comment here: #42937 (comment)
For some reason the commit is not in the post, the following is the relevant commit I did in order to revert back to a working state:
IsraelHikingMap/Site@419b98b
I did some workarounds as can be seen in the following commits:
IsraelHikingMap/Site@788a961
to bypass this but I believe the docs should mention the need to remove UseRouting as well so that developers will fix the warning properly without breaking the entire app like I did.
After the removal of UseRouting I believe the app is working as expected (although I'm not very confident it does as I have done some changes I don't fully understand)...

@HarelM
Copy link

HarelM commented Jan 17, 2023

Nope, I was celebrating too soon...
One of my controllers that doesn't require authentication is not called in my latest code: https://github.com/IsraelHikingMap/Site/tree/32146b91931e6fd3a392413063be3e091685f5ae
Generally speaking, this analyzer brought more damage than good from my point of view, unfortunately... :-(

@ghost ghost added Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. and removed Needs: Author Feedback The author of this issue needs to respond in order for us to continue investigating this issue. labels Jan 18, 2023
@captainsafia
Copy link
Member

captainsafia commented Jan 19, 2023

OK, while looking into this, I observed one buggy behavior with the analyzer which is that it will warn on code like:

app.UseEndpoints(e => {});

Which is not desirable since the user might want to be explicitly invoking UseEndpoints with a certain ordering. Marking as a bug to fix this issue.

I'm not using minimal hosting. I'm using top-level statements in program.cs class.

Minimal hosting refers to the use of the WebApplicationBuilder and WebApplication constructs here. You can read more about them here.

I've tried calling MapControllers directly, but it doesn't work at runtime. Angular does not identify the controller endpoint and returns 404.

I'm not a total expert on the SPA templates here but I suspect it might have something to do with the order of the other middlewares in your application. Top-level invocations are working in the built-in Angular templates.

@captainsafia captainsafia added bug This issue describes a behavior which is not expected - a bug. and removed Needs: Attention 👋 This issue needs the attention of a contributor, typically because the OP has provided an update. labels Jan 19, 2023
@captainsafia captainsafia added this to the 8.0-preview2 milestone Jan 19, 2023
@captainsafia captainsafia self-assigned this Jan 19, 2023
@brunolins16
Copy link
Member

I'm not a total expert on the SPA templates here but I suspect it might have something to do with the order of the other middlewares in your application. Top-level invocations are working in the built-in Angular templates.

I believe you are right. I am not expert as well but the call to UseAngularCliServer will configures the UseProxyToSpaDevelopmentServer that will proxy all request to SPA and I believe in this case MapControllers need to be called with UseEndpoints.

Could be related to this discussion: #44244

@javiercn @mkArtakMSFT maybe you could confirm this information.

@captainsafia
Copy link
Member

Nope, I was celebrating too soon... One of my controllers that doesn't require authentication is not called in my latest code: https://github.com/IsraelHikingMap/Site/tree/32146b91931e6fd3a392413063be3e091685f5ae Generally speaking, this analyzer brought more damage than good from my point of view, unfortunately... :-(

@HarelM Do you mind opening another issue for your dilemma so we can track if there is a similar problem there? I don't want it to get lost between the comment on the PR and this issue.

@captainsafia captainsafia changed the title [BUG] ASP0014: Suggest using top level route registrations Investigate interaction between UseProxyToSpaDevelopmentServer and UseEndpoints Jan 26, 2023
@captainsafia captainsafia removed this from the 8.0-preview2 milestone Jan 26, 2023
@captainsafia captainsafia removed their assignment Jan 26, 2023
@captainsafia captainsafia removed feature-minimal-hosting old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Jan 26, 2023
@captainsafia
Copy link
Member

I split up the analyzer problem into a separate bug.

Moving this to the Blazor area to resolve why the SPA setup used in user code is causing issues with MapControllers.

cc: @mkArtakMSFT

@Nick-Stanton
Copy link
Member

@SteveSandersonMS when convenient could you please check this out?

@HarelM
Copy link

HarelM commented Jan 26, 2023

@captainsafia I'm not sure what the other bug should say if I need to open a new one, the bottom line of this bug describes my problem.
The obsolete message was confusing and somewhat incorrect from my point of view.
I needed a few changes to happen for it to work properly.
I had a middleware after the controllers which causes issues and keeping the useRouting also caused issues.
Hope this helps you guys narrow down the issue. Good luck! :-)

@SteveSandersonMS
Copy link
Member

I've looked into it, and it appears to match with what @JamesNK says in #44244. That is, UseSpa is terminal middleware, so if you want other middleware to run for certain URLs, you have to register that other middleware earlier in the pipeline. Calling application.MapControllers() directly (outside UseEndpoints) registers the controller middleware too late, so it isn't compatible with UseSpa. This is not a breaking change - it's always been the case as far as I can tell (verified back to .NET 6 anyway).

@captainsafia Do any problems occur if people call application.UseEndpoints(endpoints => endpoints.MapControllers())? That's compatible with UseSpa but I don't know if it has any drawbacks besides triggering the Roslyn suggestion.

@HarelM You might already be aware, but UseSpa is from Microsoft.AspNetCore.SpaServices.Extensions which is not the latest and recommended way to host a SPA inside ASP.NET Core. As of .NET 6, this was replaced with the Microsoft.AspNetCore.SpaProxy package - read more at https://learn.microsoft.com/en-us/aspnet/core/release-notes/aspnetcore-6.0?view=aspnetcore-7.0#improved-single-page-app-spa-templates. We're not actively working on the older Microsoft.AspNetCore.SpaServices.Extensions package so if possible you'd be better off upgrading, which will implicitly fix the issue you're facing here.

@SteveSandersonMS SteveSandersonMS added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates and removed bug This issue describes a behavior which is not expected - a bug. investigate feature-spa area-blazor Includes: Blazor, Razor Components labels Jan 31, 2023
@SteveSandersonMS SteveSandersonMS removed their assignment Jan 31, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Mar 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

No branches or pull requests

9 participants