Skip to content
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

XF: NavigationService swallowing Exceptions #1024

Closed
dansiegel opened this issue Apr 18, 2017 · 9 comments
Closed

XF: NavigationService swallowing Exceptions #1024

dansiegel opened this issue Apr 18, 2017 · 9 comments
Labels

Comments

@dansiegel
Copy link
Member

When there is some sort of initialization Exception being thrown the NavigationService is swallowing the exception leaving the dev to wonder what on earth happened. When this occurs on the initial page, the app will crash when no page is pushed resulting in an even more confusing error since we have no real idea why the Page never loaded.

This should be fixed by a two prong approach:

  1. We need to make sure that the NavigationService will catch and then log exceptions.
  2. ViewModels that fail to resolve should be logged. We could either do this by default or introduce a way for dev's to control the behavior. Some possibilities would be a boolean property in PrismApplicationBase like EnableViewModelLocatorLogging. Another possibility would be to introduce a virtual method in PrismApplicationBase that we could pass any exceptions to and the dev could then have full control of how they want to log the exception.
@brianlagunas
Copy link
Member

Are you sure they are swallowed? Are they just not being caught? If I await and wrap navigation in a try/catch I get the exceptions. Can you reproduce an instance where that is not the case?

@dansiegel
Copy link
Member Author

The scenario is:

protected override void OnInitialized()
{
    NavigationService.NavigateAsync("MainPage");
}

I believe you are correct actually, it's more that the task swallows the exception resulting in the only exception the dev sees being no page. This is more of a thought that we could improve the logging within the framework so that it is easier for dev's to see what happened without having to throw in a try catch every 10 lines.

@brianlagunas
Copy link
Member

brianlagunas commented Apr 18, 2017

We should probably update the templates to provide guidance on this. Maybe update the app to this:

		protected override async void OnInitialized()
		{
			InitializeComponent();

			try
			{
				await NavigationService.NavigateAsync("MainPage");
			}
			catch (Exception ex)
			{
				//TODO: handle exception
			}
		}

@brianlagunas
Copy link
Member

We could also add an event handler to catch unhandled exceptions and add a virtual method in the base class so they can be overridden

TaskScheduler.UnobservedTaskException += TaskScheduler_UnobservedTaskException;
	        virtual void TaskScheduler_UnobservedTaskException(Object sender, UnobservedTaskExceptionEventArgs e)
		{
			if (!e.Observed)
			{
				// prevents the app domain from being torn down
				e.SetObserved();

				// TODO: handle exception
				//var exception = e.Exception.Flatten().GetBaseException();
			}
		}

@brianlagunas
Copy link
Member

We could also log more in the NavigationService. I mean we have the logger there. Might as well use it.

@dansiegel
Copy link
Member Author

Actually I think your idea of adding a TaskScheduler.UnobservedTaskException handler may be the way to go there. Also yes we do have the logger in the NavigationService, we should probably use it more, especially with how critical Navigation is.

@brianlagunas
Copy link
Member

We could actually reset the Application.MainPage to an instance of a new ContentPage that contains the error message by default. Do you think this would be too intrusive, or a good thing to help developers?

@dansiegel
Copy link
Member Author

In short yes, I do think we should provide a mechanism for introducing an error page so that you don't end up with an app crash because no page was pushed onto the stack. I'm not sure though if I prefer introducing an interface specifically for failed navigation handling or if we should incorporate it as part of a larger INavigationServiceOptions that could be used to inject behaviors to pages or any other features we may think of later?

Either way I would say we should probably introduce the page by providing a default View/ViewModel in Prism.Forms that can be registered for Navigation by PrismApplication and then updated by a dev if they provide their own View with a key like PrismErrorPage

@lock
Copy link

lock bot commented Jan 30, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants