-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
Navigation Exception: Introduce Clear Message & Helper Class #56644
base: main
Are you sure you want to change the base?
Conversation
@dotnet-policy-service agree company="Retro Rabbit" |
public static class NavigationRedirectHelper | ||
{ | ||
private const string RedirectExceptionMessage = | ||
"A navigation was initiated during static rendering. " + | ||
"This exception is not an error, but instead signals to the framework that a redirect should occur. " + | ||
"It should not be caught by application code."; | ||
|
||
/// <summary> | ||
/// Creates an absolute URI and throws a new <see cref="NavigationException"/>. | ||
/// </summary> | ||
|
||
[DoesNotReturn] | ||
public static void ThrowNavigationExceptionForRedirect(NavigationManager navigationManager, string uri) | ||
{ | ||
var absoluteUriString = navigationManager.ToAbsoluteUri(uri).AbsoluteUri; | ||
throw new NavigationException(absoluteUriString, RedirectExceptionMessage); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need a helper class to replace what otherwise is two lines of code that get called in two locations. The cost of adding new APIs is not worth the "savings" that we might get. Specially since this exception is entirely thrown by the framework and no one else.
/// <summary> | ||
/// Initializes a new <see cref="NavigationException"/> instance with a custom message. | ||
/// </summary> | ||
public NavigationException(string uri, string message) : base(message) | ||
{ | ||
Location = uri; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need the extra constructor; we can simply pass the message to the base class since it's a static message. There's no need for the message to be customizable.
… & NavigationRedirectHelper references & pass static message to NavigationException main constructor
Navigation Exception: Introduce Clear Message & Helper Class
Description:
This pull request is for adding a useful message to the
NavigationException
when thrown as well as a helper class that can be called instead of creating absolute URIs and throwing aNavigationException
multiple times accross the codebase.Changes:
NavigationException
constructor that accepts a custom messageNavigationException
easierNavigationException
referencesFixes #51787
NB:
I am unsure about the location of the helper file, the shared location suggested (in the previous inactive PR #54862) seems inaccessible to me, any help is appreciated.
There is a reference to
NavigationException
here that I am unsure if it needs to be updated as well