Exception handling in IDLE

[ssteiner]

Suppose you have IDLE up and running, and the noopTimer.Elapsed fires. And something goes wrong in IssueNoop. Your program goes boom and it's all over (uncaught exception bringing the entire application down). In Issue #51 we have a solution to handle errors in the IdleLoop - but that's only half the battle. If something goes wrong in IssueNoop, it still brings the house down.

The easiest way to get around that is simply wrap the whole IssueNoop in a try/catch, but that doesn't work with the rest of the design.

I think to be properly resilient in the IDLE scenario, the lib would need to detect conectivvity issue, provide a reconnect mechanism (e.g. when starting IDLE you could have a parameter telling the lib to reconnect if there's an issue, and if not, give an event to notify the user about the issue).

The same problem can arise in the EventDispatcher - if there's an issue in GetHighestUid, it can blow up the application with an uncaught exception. Since those exceptions force your app to quit, you get a resiliency issue.

Here's a trival fix with an Action that can be used to catch most issues that can arise in the idle loop in order not to bring the house down.

    /// <summary>
    /// This action is fired whenever there's an exception from the Idlee loop
    /// </summary>
    public Action<ApplicationException> IdleLoopException { get; set; }

            /// <summary>
    /// Blocks on a queue and wakes up whenever a new notification is put into the
    /// queue. The notification is then examined and dispatched as an event.
    /// </summary>
    private void EventDispatcher() {
        uint lastUid = 0;
        while (true) {
            string response = idleEvents.Dequeue();
            Match m = Regex.Match(response, @"\*\s+(\d+)\s+(\w+)");
            if (!m.Success)
                continue;

            uint numberOfMessages = Convert.ToUInt32(m.Groups[1].Value);
            uint uid = 0;
            try
            {
                uid = GetHighestUID();
                switch (m.Groups[2].Value.ToUpper())
                {
                    case "EXISTS":
                        if (lastUid != uid)
                        {
                            newMessageEvent.Raise(this,
                                new IdleMessageEventArgs(numberOfMessages, uid, this));
                        }
                        break;
                    case "EXPUNGE":
                        messageDeleteEvent.Raise(
                            this, new IdleMessageEventArgs(numberOfMessages, uid, this));
                        break;
                }
                lastUid = uid;
            }
            catch (Exception x)
            {
                ts.TraceInformation("Unable to get message id for a " + m.Groups[2] + " message in EventDispatcher: " + x.Message);
                OnIdleLoopException(new ApplicationException("Unable to get message ID for " + m.Groups[2].Value + " message", x));
            }
        }
    }

    /// <summary>
    /// Issues a NOOP command to the IMAP server. Called in the context of a
    /// System.Timer event when IDLE notifications are being received.
    /// </summary>
    /// <remarks>This is needed by the IMAP IDLE mechanism to give the server
    /// an indication the connection is still active.
    /// </remarks>
    private void IssueNoop(object sender, ElapsedEventArgs e) {
        lock (sequenceLock) {
            try
            {
                PauseIdling();
            }
            catch (Exception x)
            {
                ts.TraceInformation("Unable to pause IDLE in PausePending in IssueNoop: " + x.Message);
                OnIdleLoopException(new ApplicationException("Unable to pause IDLE", x));
            }
            string tag = GetTag();
            string response = null;
            try
            {
                response = SendCommandGetResponse(tag + "NOOP");
                while (response.StartsWith("*"))
                    response = GetResponse();
            }
            catch (IOException i) // something went wrong processing the stream
            {
                ts.TraceInformation("There's an issue sending NOOP to the server: " + i.Message);
                OnIdleLoopException(new ApplicationException("Problem in NOOP", i));
            }
            try
            {
                ResumeIdling();
            }
            catch (Exception x)
            {
                ts.TraceInformation("Unable to resume IDLE in IssueNoop: " + x.Message);
                OnIdleLoopException(new ApplicationException("Unable to resume IDLE", x));
            }
            if (!string.IsNullOrEmpty(response))
            {
                if (!IsResponseOK(response, tag))
                    throw new BadServerResponseException(response);
            }
        }
    }

    private void OnIdleLoopException(ApplicationException e)
    {
        if (IdleLoopException != null)
            IdleLoopException.Invoke(e);
    }

[ssteiner]

Turns out that was only half the battle - one inheritent problem in the lib is concurrent access. Try launching a couple of imap operations in the newmail handler will get you into severe trouble because the Idle loop.

Here's a scenario that will mess things up big time:

Subscribe for new emails, with an event handler that does the following:

1. Fetch the mail
2. Delete the mail
3. Signal to a EventWaitHandle that a new mail has been received. That in turn will trigger unsubscribe and logout
   Using another tool, deposit a mail.

See it bomb out.

There's a variety of things here. First of, using

lock(sequenceLock)

Does not guarantee that whomever tries to acquire the lock while somebody has already acquired it, will get it in the order that access was requested. So if A has the lock, and B and C try to access it, C may get it before B even though requested it first. So, in the above scenario, things may not terminate as planned and you might end up in a situation where idle is restarted when the connection was already closed (which of course causes all kinds of havoc).

I've had to solve a similar problem with asynchronous execution that needed to follow a specific order - unfortunately as it is task based, it cannot directly be transferred here.However, using something like a QueuedLock (http://pastebin.com/BTdPY2UZ) should do the trick. So, all the lock(sequenceLock) would need to be replaced by

  try 
  {   
      queuedLock.Enter();
      //execute code from within lock(sequencelock){ ..}
  }
  finally
  {
      queuedLock.Exit();
  }

As a quick fix I did the following

1. Event handling is task based.. firing off the event starts a new task (with followup task to handle errors)
2. in StopIdling(), I check if idleThread is null before calling Join (pause will set it to idle.. so since you don't have guaranteed lock access, you may be able to stop idling where idling was previously paused but not yet restarted (happens in the scenario outlined above).

[smiley22]

Hi,

the IssueNoop method is tied to the Elapsed event of the noopTimer and executes in the context of a threadpool thread. As a (in this case convenient) side-effect, the timer component catches and suppresses all exceptions thrown by event handlers for the Elapsed event; You're right about the GetHighestUID in the dispatcher thread though.

I tried to re-produce the scenario you described above, but haven't had any luck.

    static ManualResetEvent newMailEvent = new ManualResetEvent(false);

    static void Main(string[] args) {
        using (ImapClient client = new ImapClient(...)) {
            client.NewMessage += client_NewMessage;

            new Thread(SecondThread).Start(client);

            while (true) {
                string s = Console.ReadLine();
            }
        }
    }

    static void SecondThread(object obj) {
        ImapClient client = obj as ImapClient;
        newMailEvent.WaitOne();
        Console.WriteLine("SecondThread: Unsubscribe");
        client.NewMessage -= client_NewMessage;
        Console.WriteLine("SecondThread: Logout");
        client.Logout();
    }

    static void client_NewMessage(object sender, IdleMessageEventArgs e) {
        Console.WriteLine("New Message: " + e.MessageUID);
        Console.WriteLine("Downloading new message...");
        MailMessage m = e.Client.GetMessage(e.MessageUID);
        Console.WriteLine("Message Subject: " + m.Subject);
        Console.WriteLine("Deleting mail message...");
        e.Client.DeleteMessage(e.MessageUID);
        newMailEvent.Set();
    }