4

What is the proper way to show a message dialog due to a caught exception?

I originally tried

try
{
    await DoSomething();
}
catch(InvalidOperation ex)
{
    await MessageDialog(ex.Message).ShowAsync();
}
catch(CommunicationException)
{
    await MessageDialog(StringResourceLoader.LoginError).ShowAsync();
}

This did not work because you cannot await inside of a try block. Taking the await commands out makes the compiler shows the following warning:

Because this call is not awaited, execution of the current method continues before the call is completed. Consider applying the 'await' operator to the result of the call

I don't like keeping those warnings in my code, because in several spots people have forgotten to use await and thus had hard to find bugs.

Changing the message dialog statement to var task = new MessageDialog(ex.Message).ShowAsync().AsTask(); gets rid of all warnings and errors, but I am not sure that is a good way to go about it (and technically is bad for the same reason it wants me to await the call)

Finally, I tried storing the exception and doing my logic of what to display to the user (including all logic on determining what type of exception was thrown) outside of the catch, via:

Exception thrownException = null;
try
{
    await DoSomething();
}
catch(Exception ex)
{
    thrownException = ex;
}

if (thrownException is InvalidOperationException)
    await MessageDialog(ex.Message).ShowAsync();
else if (thrownException is CommunicationException)
    await MessageDialog(StringResourceLoader.LoginError).ShowAsync();

I am not sure I feel that this is the best way to go about it either. Any ideas how this should be done?

KallDrexx
  • 27,229
  • 33
  • 143
  • 254
  • 2
    There's more than one problem here, MessageDialog also gets very cranky when your app is already displaying one. In other words, you can never be sure the user will actually see it. The most likely outcome is an app that bombs to the desktop for no perceivable reason. Really solving this requires a queue that displays messages one after another. Or to put it in another way, showing exceptions with a message dialog just isn't a very good idea. – Hans Passant Jul 23 '13 at 18:23
  • That's a good point, and an interesting idea about having a queue... – KallDrexx Jul 23 '13 at 18:25
  • Make sure to check a similar question [here](http://stackoverflow.com/questions/14488587/how-to-allow-for-multiple-popups-at-once-in-winrt). – Filip Skakun Jul 24 '13 at 00:40
  • ah I hadn't come across that question – KallDrexx Jul 24 '13 at 02:46

1 Answers1

4

Edit: After Hans' comment about other issues, I ended up solving it by creating the following class:

public static class MessageDialogDisplayer
{
    private static readonly ConcurrentQueue<MessageDialog> _dialogQueue;
    private static readonly CancellationTokenSource _cancellationTokenSource;

    static MessageDialogDisplayer()
    {
        _dialogQueue = new ConcurrentQueue<MessageDialog>();
        _cancellationTokenSource = new CancellationTokenSource();

        new Task(DisplayQueuedDialogs, _cancellationTokenSource.Token).Start();
    }

    public static void DisplayDialog(MessageDialog dialog)
    {
        _dialogQueue.Enqueue(dialog);
    }

    private static async void DisplayQueuedDialogs()
    {
        const int millisecondsBetweenDialogChecks = 500;

        while (!_cancellationTokenSource.Token.IsCancellationRequested)
        {
            MessageDialog dialogToDisplay;
            if (_dialogQueue.TryDequeue(out dialogToDisplay))
            {
                await dialogToDisplay.ShowAsync();
            }
            else
            {
                await Task.Delay(millisecondsBetweenDialogChecks);
            }
        }
    }
}

This has changed my try/catch statement to

MessageDialog errorDialog = null;
try
{
    await DoSomething();
}
catch(InvalidOperation ex)
{
    MessageDialogDisplayer.DisplayDialog(new MessageDialog(ex.Message));
}
catch(CommunicationException)
{
    MessageDialogDisplayer.DisplayDialog(new MessageDialog(StringResourceLoader.LoginError));
}

Which makes things more stable in the long run (once I convert all message dialog callers to use this instead of showAsyncing themselves) and makes the try/catch block a lot less messy (imo).

M. Jahedbozorgan
  • 6,914
  • 2
  • 46
  • 51
KallDrexx
  • 27,229
  • 33
  • 143
  • 254
  • Hi, is this really the best way to solve this? From what I understand, the MessageDialogDisplayer creates an always running Task that checks for messages every half a second? When the task is cancelled, then we can never display any other messages because the while loop stops. Has this worked for you (since it's been almost a year since you wrote this)? – Chris Leyva Jun 27 '14 at 18:05
  • 1
    While I no longer work for the company I wrote this for to my knowledge this is still working *good enough*. I don't think the cancellation is an issue because honestly I can't see any reason you'd want to explicitly cancel the dialog runner (in fact no way to do so is exposed, so it's kinda pointless). I'm sure there could be better solutions but in the end we just needed this to only display errors, so there was no use over-engineering it for other use cases. – KallDrexx Jun 27 '14 at 21:44