Skip to content

NavigationException doesn't have a useful message. #51787

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

Open
danmoseley opened this issue Oct 31, 2023 · 10 comments
Open

NavigationException doesn't have a useful message. #51787

danmoseley opened this issue Oct 31, 2023 · 10 comments
Labels
area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-blazor-navigation help wanted Up for grabs. We would accept a PR to help resolve this issue
Milestone

Comments

@danmoseley
Copy link
Member

danmoseley commented Oct 31, 2023

this

throw new NavigationException(absoluteUriString);

creates an exception like this
image
with the URL in the Location field.

Ideally it should have a useful message that includes the URI, eg., "Could not navigate to https://localhost:7298/item/99"

the problem is that NavigationException is not overriding Message and ToString

public class NavigationException : Exception

If we want to keep the URL out of the message for security reasons, it should at least have a message like "Could not navigate to the URL".

@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Oct 31, 2023
@danmoseley
Copy link
Member Author

If we want to keep the URL out of the message for security reasons, it should at least have a message like "Could not navigate to the URL".

@blowdart what is our policy for what can go into exception messages out of aspnet ? We disable display by default, of course. But these may go into logs. If we don't want the URL in the message, is it OK in the ToString()? (after all, it's in a property on the exception object)

@javiercn
Copy link
Member

javiercn commented Nov 1, 2023

@danmoseley you are only probably seeing that because you have first chance exceptions turned on. The exception gets internally caught by the framework and handled. Is that not the case in your situation?

@blowdart
Copy link
Contributor

blowdart commented Nov 1, 2023

I'm in conversations with other teams internally about logs and error messages, which may drive some requirements for 9

@mkArtakMSFT
Copy link
Contributor

I'm following up with the Debugger team to see how we can ignore certain types of exceptions to avoid confusion here.

@danmoseley
Copy link
Member Author

danmoseley commented Nov 2, 2023

you have first chance exceptions turned on

Could well be. If it's always caught then it doesn't matter much. But first chance do still get logged in the debugger output - why not override Message.

It's possible you could ask the VS debugger team to buy default not catch this first chance. They seem to do this for certain exception types out of the box. (I'm assuming you can't avoid throw/catch here due to architecture)

@mkArtakMSFT mkArtakMSFT added this to the .NET 9 Planning milestone Nov 2, 2023
@ghost
Copy link

ghost commented Nov 2, 2023

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@mkArtakMSFT mkArtakMSFT added enhancement This issue represents an ask for new feature or an enhancement to an existing one and removed task labels Nov 2, 2023
@mkArtakMSFT
Copy link
Contributor

After some further discussion in the team we're moving forward with the following:

  1. Change the exception message to make it more user friendly
  2. Follow up with the debugger team to see how to avoid treating these as first-chance exceptions.

@ghost
Copy link

ghost commented Nov 19, 2023

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@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 from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@danmoseley danmoseley added the help wanted Up for grabs. We would accept a PR to help resolve this issue label Mar 27, 2024
Copy link
Contributor

Looks like this issue has been identified as a candidate for community contribution. If you're considering sending a PR for this issue, look for the Summary Comment link in the issue description. That comment has been left by an engineer on our team to help you get started with handling this issue. You can learn more about our Help Wanted process here

@spdebeer
Copy link

Could I possibly pick this issue up seeing that the open PR is currently not active

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one feature-blazor-navigation help wanted Up for grabs. We would accept a PR to help resolve this issue
Projects
None yet
6 participants