RazorPages page directives missing quotes should alert user

[ryanbrandenburg]

Edit: Reopening after some discussion.

The issue here is that if we can't parse the route we can't assign what the user 'expects' the route to be to the page. So if the user can't visit the page because it doesn't get the URL that they expect, then how will they know that it's invalid?

---

Original

As per #5838 (comment) we should have a think about how we handle the case of @page Url/Here. Right now we ignore the URL because it's not in quotes, but there's some concern that that will confuse the users and that we should either throw an exception or at least log a message.

[rynowak]

This should be a parsing error coming from Razor.

[rynowak]

aspnet/Razor#980 is the proper issue for this.

[Eilon]

There are three distinct cases to consider:

1. Directive isn't even formatted correctly.
   • The page will get its default route (based on its file path), and when you run it you see a parser error.
2. Directive is formatted correctly, but route within it is not a valid route.
   • The startup will fail when the invalid route is being added to the system.
3. Directive is formatted correctly, route is also formatted correctly, but it's not what you meant.
   • No errors will happen anywhere. (We could consider in the future to have some route table / route debugger feature as part of diagnostics)

The work for this item is:

• For cases (1) and (2), log as Warning the problem
• Add tests that confirm all the above behavior (and if not, fix as needed)

[ryanbrandenburg]

(1) already logs and has a test here. Do we want a functional test for it in MVC as well or is that enough?

(2) When you say not a valid route how do you mean? Do you just mean something like @page "Broken?" with a banned character in it, or do you mean it would route to something that's already covered by another page/controller?

It doesn't sound like you want anything done for (3), but for posterity how would we know if it's "not what you meant".

[rynowak]

1. How is this logging? It's a compilation error which you will never see because you don't actually visit the page

2. Yes, we mean a totally invalid route.

3. Yeah, we're saying there's no action to take here.

[pranavkm]

@ajaybhargavb \ @NTaylorMullen, doesn't the directive parser already validate this?

[Eilon]

At the very least we need to make sure we have tests for all the scenarios listed here: #5868 (comment)

[NTaylorMullen]

@pranavkm It sees the problem but it doesn't log the issue anywhere. Essentially the error goes nowhere.

[rynowak]

For cases (1) and (2), log as Warning the problem

This means ILogger not a diagnostic from Razor.

[pranavkm]

@rynowak - would Razor depend on Logging or would we add diagnostics with error levels and print them from Mvc?

[rynowak]

Razor depend on Logging

Razor will not depend on logging.

would we add diagnostics with error levels and print them from Mvc?

I'm not convinced that what we're discussing here is complicated. If it is complicated, then we can probably punt this or close it.

[Eilon]

@rynowak - if Razor doesn't depend on logging, how does the "log" flow to the user's ILoggerFactory?

[rynowak]

It comes from Microsoft.AspNetCore.Mvc.RazorPages.

[Eilon]

Ah ok.