Skip to content

[Templating] Investigate error messages in the chrome/firefox dev console for SPAs #7812

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
javiercn opened this issue Feb 21, 2019 · 19 comments
Labels
affected-medium This issue impacts approximately half of our customers area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates blocked The work on this issue is blocked due to some dependency bug This issue describes a behavior which is not expected - a bug. External This is an issue in a component not contained in this repository. It is open for tracking purposes. feature-spa severity-minor This label is used by an internal tool
Milestone

Comments

@javiercn
Copy link
Member

On firefox and chrome, the React and Angular SPA produce a series of error during development.
We need to figure out what's happening in there.

@javiercn
Copy link
Member Author

image

@Eilon Eilon added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Feb 21, 2019
@mkArtakMSFT mkArtakMSFT added bug This issue describes a behavior which is not expected - a bug. 1 - Ready labels Feb 22, 2019
@mkArtakMSFT mkArtakMSFT added this to the 3.0.0-preview4 milestone Feb 22, 2019
@ryanbrandenburg
Copy link
Contributor

@mkArtakMSFT we've discussed this issue before. The problem is in JavaScript services, where I don't have any context. @SteveSandersonMS was on vacation the last time it was brought up, perhaps we can discuss it with him now.

@SteveSandersonMS
Copy link
Member

I thought the problem was that the dev servers (Angular CLI / create-react-app) emit code that tells the client to make a WebSocket connection back to the dev server. Since we're proxying to the dev server, the WebSocket connection actually goes to the ASP.NET Core host application (which is good and correct), but then somehow we're failing to proxy that WebSocket connection properly.

So my guess is someone needs to debug the proxying code to see why we're not letting the browser and the Angular/React dev server talk to each other in the way they expect.

@SteveSandersonMS
Copy link
Member

The error message does contain the clue "Invalid host/origin header", so maybe it's something like: the proxy code needs to rewrite the origin header so that the Angular/React dev server believes the request is coming directly to it, and not from some other origin.

@mkArtakMSFT mkArtakMSFT removed this from the 3.0.0-preview4 milestone Feb 22, 2019
@mkArtakMSFT mkArtakMSFT added this to the 3.0.0-preview6 milestone Apr 4, 2019
@mkArtakMSFT
Copy link
Contributor

@ryanbrandenburg please reevaluate this as this seems to be really bad. We should try to fix this as early as possible but the current M costing is a bit heavy.

@ryanbrandenburg
Copy link
Contributor

This seems to only happen when the app isn't published (because when we publish and are thus in Production it doesn't use webpack-dev-server).

@javiercn
Copy link
Member Author

That's fine, We don't want to see these exceptions during Development because they are unwanted noise. That's what we want to fix here.

@ryanbrandenburg
Copy link
Contributor

The Invalid Host/Origin thing is a separate, Firefox-only problem that can be solved by doing this. That said disabling the host check is dangerous, so it's unclear if we want that fix. The websocket connection problem doesn't seem solvable by re-writing the host/origin headers because even when we set --disableHostCheck=true we still get the get the warnings.

@ryanbrandenburg
Copy link
Contributor

This comment says that this warning doesn't indicate a problem and can be ignored, but that's in the context of it only happening 5% of the time (as opposed to 100%). If we still care about resolving this given that and the fact that HotLoading still works (as long as you fix the Firefox Origin thing) then what I need to proceed is a call with @SteveSandersonMS so we can figure out how to diagnose and proceed. I talked with some people in the server room and they didn't have ideas.

@mkArtakMSFT
Copy link
Contributor

Filed #9400 to handle the one dependency update to avoid many errors

@DaniJG
Copy link

DaniJG commented Apr 17, 2019

The SPA approach seems to be having ASP.NET Core in charge, with HTTP traffic always coming to ASP.NET Core who will proxy to the SPA's dev server.

Have you people considered the opposite approach? On startup, ASP.NET Core could still launch the start/serve scripts of the SPA but it can then tell the browser to direct traffic to the SPA dev server, and its the dev server the one that will proxy API requests to the ASP.NET Core process.

All the major web frameworks support the proxy option (they all end up using the webpack dev server) and it seems to me proxying API requests is simpler/faster than proxying the hot reload web sockets...

The main challenge I see is to make sure the port configured for ASP.NET Core in development is correctly passed into the devProxy setting, which in the case of angular/react seem to be static json files. It could be set when the template is first created, but the ports for IISExpress and Kestrel are different... Not sure if react/angular allow those settings to be in a regular .js file (Vue does), because then you can setup an env variable from .NET which is then read on that file as part of the npm start/serve command. Otherwise you could modify those json config files before running the npm script (which isnt great tbh)

Maybe this was already considered by the team and discarded in favour of the current approach, but if not it might be worth exploring.

@javiercn
Copy link
Member Author

@SteveSandersonMS Do you have any thoughts? We could do the proxying in the templates, but I feel thats a big departure from the way things work today.

@SteveSandersonMS
Copy link
Member

The websocket connection problem doesn't seem solvable by re-writing the host/origin headers because even when we set --disableHostCheck=true we still get the get the warnings.

I don't follow how it's possible to still get the warnings if the proxying is being done correctly. How can the browser even distinguish between having a direct connection to the SPA's dev server versus a reverse-proxy to it? If the reverse proxying is being done well enough then the browser shouldn't be able to know there's a difference.

I'd recommend trying to rewrite the origin header and seeing whether that does help. If there are still problems, look at network traces to determine what's different between having the reverse proxy versus loading the site directly in the Angular/React dev server.

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Jul 31, 2019

It's possible also that moving to newer versions of the libraries may fix it (if the problem is nothing to do with our code and instead is just purely a bug in sockjs.js or Webpack). There was a workaround applied in Webpack in https://github.com/webpack/webpack-dev-server/pull/1608/files.

@mkArtakMSFT
Copy link
Contributor

Yeah, sounds like this is exactly what @ryanbrandenburg was referring to. It seems that have been merged a while ago already. @ryanbrandenburg can you please check what dependencies we should pick up to get this fix and see whether this issue is resolved?

@mkArtakMSFT
Copy link
Contributor

Given that this has no functionality impact, and this will be resolved when impacted SPA frameworks will pick up the Webpack workaround and we'll update our dependencies, we'll just wait.

@pranavkm pranavkm removed the cost: XS label Nov 6, 2020
@captainsafia captainsafia added affected-medium This issue impacts approximately half of our customers severity-minor This label is used by an internal tool labels Feb 19, 2021
@ryanbrandenburg ryanbrandenburg removed their assignment Aug 5, 2021
@javiercn
Copy link
Member Author

This doesn't apply in 6.0

@ghost ghost locked as resolved and limited conversation to collaborators Nov 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affected-medium This issue impacts approximately half of our customers area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates blocked The work on this issue is blocked due to some dependency bug This issue describes a behavior which is not expected - a bug. External This is an issue in a component not contained in this repository. It is open for tracking purposes. feature-spa severity-minor This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests

8 participants