Unhandled Exception

[e-davidson]

I'm using the Imap client to notify me when I get a new message in gmail.

The problem I'm having is, that after a number of hours the remoter server closes the connection.
This becomes apparent in s22.imap.getresponse byte b = (byte)stream.ReadByte();

I get an unhadled exception. The message is, the connection forcibly closed by remote host.

[Umplify]

The issue most likely happens in the method below:

private void IdleLoop()

That's due to the stack trace below:

Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: System.IO.IOException
Stack:
at S22.Imap.ImapClient.IdleLoop()
at System.Threading.ThreadHelper.ThreadStart_Context(System.Object)
at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
at System.Threading.ThreadHelper.ThreadStart()

[Umplify]

More information from IdleLoop():

Unable to read data from the transport connection: An existing connection was forcibly closed by the remote host.

[e-davidson]

The main problem is, I can't handle the exception in my code. Since its not running in the same thread. There should probably be a way to let the caller handle a closed connection.

[Umplify]

Well, there is a way to handle an exception thrown from another child thread. However, I believe that throwing that exception is unnecessary unless the code developer says otherwise. I commented out "throw" keyword and compiled the code. Since then, the code has been working well without any surprised shutdown.

[e-davidson]

Ok, I'll try that. However, I'm not sure why you would not want to handle an exception if the connection was closed.

[Umplify]

If the connection is closed, wouldn't that while(true) loop re-initialize the stream and read data moving forward? Why other types are inner exceptions are digested and not thrown while any other types must be thrown out? What would be the difference? What would be your suggestion? Am I missing something?

[e-davidson]

I was not aware that the stream will be reinitialized. I don't see the line of code that will do that.

[Umplify]

string response = GetResponse(); will try to read the stream again. However what would be the difference between the socket exception, or thread exception and other types of exceptions. All of them will somehow terminate the connection. What I don't understand is that why only two types of inner exceptions are handled gracefully while the other ones must be bubbled up?

[e-davidson]

If the host closes the connection then it won't be successful.
I've know idea why they chose those. I'm just trying to get this to work for days at a time without shutting down on me.
Why do they call it bubbling up if it can't be handles anyway? At least not from my code.

[Umplify]

I know, I think we have to wait for the developer to address it.

[NiKiZe]

If the connection is closed, for whatever reason, then the connection needs to be reestablished.
It is in the current implementation impossible to do so because things like username and password is not stored in the class. (and for security reasons that's good).

So for any kind of connection error, the client needs to be restarted.

If there is anything that can not be handled internally, the errors should bubble up, and handled in user code.
I have started a prototype to handle a similar issue, so should work for this as well, but there is to many other things that have gotten in the way for me to finish.

[e-davidson]

Allowing it to bubble up does not send the exception back to the users code, as its handled in a different thread. So where would you catch it?

[Umplify]

That is correct. The ImapClient code must be modified to catch the exception within it. That requires to commit (check in) a newer version of code to this GitHub project. It is impossible to catch that exception in your own code.

[e-davidson]

How would you send an exception from a new thread back to your code?
The way I'm thinking would be to add an event handler, but I'm not sure that is the best way.

[Umplify]

Well, haven't done so! A little bit googling may help. This is the 1st result that I got from google, however it may not be applicable to our case:

http://stackoverflow.com/questions/1554181/exception-handling-in-threads

[Umplify]

The exceptions (any) must be caught and handled within the same method. There is no other way to catch it outside of the thread. If the thread was like a Task, there would be a firm workaround.

[smiley22]

Hello,

what workaround do you propose? I'm not a fan of silently attempting to
re-establish a connection which was forcefully closed by the server. If the
server drops a connection, it won't do so without reason. Any idea why the
connection is being dropped? The library regularly issues NOOP's as RFC
recommends to keep the connection alive so that shouldn't be it.

On Fri, May 3, 2013 at 12:28 PM, ArtaTechs notifications@github.com wrote:

The exceptions (any) must be caught and handled within the same method.
There is no other way to catch it outside of the thread. If the thread was
like a Task, there would be a firm workaround.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/51#issuecomment-17387831
.

[Umplify]

The exception thrown from inside the IdleLoop thread cannot be handled in the parent thread and that will make the application shutdown unexpectedly. Therefore the throw keyword must be removed and the parent thread be signaled by raising a new event. That event must be handled such that the connection to be reestablished and idling loop begun.

You are correct, the noop command has to keep the connection alive but what if the Internet connection is lost? You can test this situation by disconnecting your development laptop.

[NiKiZe]

Use AppDomain.UnhandledException and friends to be able to handle all exceptions.
Just a quick example copy from a currently open VB.net proj

 AddHandler AppDomain.CurrentDomain.UnhandledException, AddressOf UnhExceptionHandler
 AddHandler System.Windows.Forms.Application.ThreadException, AddressOf ThrExceptionHandler

But maybe the IdleLoop could raise a event containing the exception, instead of throwing a exception?
That event can then be used to reconnect in user code or what is needed.
Maybe exposing one of the connect calls so the existing ImapClient object can be reused?

[e-davidson]

I have no idea why, I'm net getting a message from the remote host.
There is very little that can be done int the client if the sever wants to end the connection.

Trying to reconnect is probably not a good idea, as you said.

The solution I'm thinking is to chain tasks. http://msdn.microsoft.com/en-us/library/dd321576.aspx
You can use the ContinueWith method on a new task instead of a thread for the idleLoop. Then set continueoptions.OnlyOnFaulted

The user can pass a task that can be executed in case of an exception.

[Umplify]

Task is a great and contemporary solution. With the current code base the fastest and more reliable solution is what I suggested in my previous post. Just my 2 cents though.

[e-davidson]

Yes, you led me in that direction.

[Umplify]

I guess it would be a good idea to share the solution by whoever implements it first including myself.

[e-davidson]

I got it to work using tasks.
Its far from perfect, it just solved my particular problem.

private Task IdleTask;
private Action ExceptionHandlerTask;

    public void SetExceptionHandlerTask(Action<Task> a)
    {
        ExceptionHandlerTask = a;
        if (IdleTask != null)
            IdleTask.ContinueWith(ExceptionHandlerTask, TaskContinuationOptions.OnlyOnFaulted);
    }

//If the cancelation is ever set, the token must be recreated and I have not done so.
private CancellationTokenSource cts = new CancellationTokenSource();

update to getresponse();

while (true) {
byte[] b = new byte[1];
//This was changed so that the task can be canceled. Unlike threads, there is no task.abort();
Task task = stream.ReadAsync(b, 0, 1, cts.Token);
task.Wait();
if (b[0] == CarriageReturn)
continue;
if (b[0] == Newline)
{
string s = Encoding.ASCII.GetString(mem.ToArray());
if (resolveLiterals)
{
s = Regex.Replace(s, @"{(\d+)}$", m =>
{
return """ + GetData(Convert.ToInt32(m.Groups[1].Value)) +
""" + GetResponse(false);
});
}
ts.TraceInformation("S -> " + s);
imapLogger.log("Processed " + s);
return s;
}
else
mem.WriteByte(b[0]);
}

Instead of starting a new thread you should start a new task.

if (IdleTask != null)
throw new ApplicationException("idleThread is not null");
IdleTask = new Task(IdleLoop);
if (ExceptionHandlerTask != null)
IdleTask.ContinueWith(ExceptionHandlerTask, TaskContinuationOptions.OnlyOnFaulted);
cts = new CancellationTokenSource();
IdleTask.Start();

Change dispose to

if (IdleTask != null)
{
//send cancellation message to the task.
//I did not reset the cancellation token so this can only work once. Tasks wont work anymore unless its reset.

            cts.Cancel();
            IdleTask = null;
        }

Finally add to user code.

task = new Action(catchExceptionHandler);
initialize imapclient.
ic.SetExceptionHandlerTask(task);

static void catchExceptionHandler(Task t)
{
//code to handle exceptions.
}

Its far from perfect.

[e-davidson]

You also have to add to Dispose()

if (noopTimer != null)
noopTimer.Stop();

If not the timer will keep sending noop to a non-existing connection.

[e-davidson]

After finally successfully running for 24 consecutive hours the gmail server sent this message
"* BYE Session expired, please login again."
Apparently google will only allow you to stay logged in for 24 hours.

[ssteiner]

@harlyd : by using the same GetResponse for "foreground actions" (e.g. get a message when your program needs it), and "background actions" (idle mechanism), you end up injecting means to abort a read that can be invalidated. Shouldn't you have two cancellation task sources? One for foreground actions (which remains constant, and is cancelled when you dispose), and one that is being cancelled and recreated every time the idling is started/stopped/paused/resumed.
Also, in PauseIdling, you need to wait for the IdleTask, then set it to null - otherwise you can run into a situation where ResumeIdling throws "idleThread is not null". And likewise, the same must be done in StopIdling (since we're replacing idleThread and all). I suppose you did all that, just didn't post it.
One more thing.. in Dispose, only cancel on cts if IdleTask.Status != TaskStatus.RanToCompletion - there's no point cancelling if the task is already done (because you may have unsubscribed).

[e-davidson]

@ssteiner sounds right, I did have some issues.
1 more point on this line of code "Task task = stream.ReadAsync(b, 0, 1, cts.Token);"
The cancellation does not cause it to return. So its pretty much pointless.

[ssteiner]

I wondered... I find the design choices they made with those token pretty weird (you need to pass them around, and you can't infer them inside a task you'd like to abort... looking at the syntax I figured maybe the async methods in the BCL would at least honor the token, but you've made the same experience as I - and I found a little explanation thereafter: http://stackoverflow.com/questions/20131434/cancel-networkstream-readasync-using-tcplistener

Makes one wonder why they add the overload if it is pointless.

[smiley22]

The ImapClient now exposes the IdleError event which is raised when an exception occurs in IdleLoop. This is much like what ArtaTechs and NiKiZe suggested. I'm closing this, but feel free to create a new issue if you run into any trouble with this solution.