Unhandled Exception about s22.imap HOT 30 CLOSED

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()

from s22.imap.

More information from IdleLoop():

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

from s22.imap.

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.

from s22.imap.

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.

from s22.imap.

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

from s22.imap.

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?

from s22.imap.

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

from s22.imap.

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?

from s22.imap.

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.

from s22.imap.

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

from s22.imap.

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.

from s22.imap.

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?

from s22.imap.

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.

from s22.imap.

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.

from s22.imap.

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

from s22.imap.

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.

from s22.imap.

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 [email protected] 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
.

from s22.imap.

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.

from s22.imap.

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?

from s22.imap.

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.

from s22.imap.

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.

from s22.imap.

Yes, you led me in that direction.

from s22.imap.

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

from s22.imap.

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.

from s22.imap.

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.

from s22.imap.

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.

from s22.imap.

@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).

from s22.imap.

@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.

from s22.imap.

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.

from s22.imap.

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.

from s22.imap.