retry login if IMAP Servers timeout. #7844
Conversation
program/lib/Roundcube/rcube_imap.php
Outdated
| 'retry' => false | ||
| ]; | ||
|
|
||
| $data = $this->plugins->exec_hook('storage_connect', array_merge($this->options, $data)); | ||
|
|
||
| if ($attempt > 1 && !data['retry']) { |
There was a problem hiding this comment.
You don't need this if statement. We check retry flag in the while statement below. You just set it appropriately from the plugin.
There was a problem hiding this comment.
here is an example without this if statement:
*attempt=1, retry= True
-> do a $this->conn->connect
-> while loop decide to go on
*attempt=2 checks the last attempt and decides not to continue retry=False
-> still do a $this->conn->connect
-> while loop stops
as you see without this if statement another reconnect attempt is executed, but this is not what we want to do. The problem we face is that we call the storage_connect hook only before we connect and not after it. This if statement makes it possible to stop immediately and not do an unexpected reconnect attempt.
I know it is a behavior change, so we could also use another key in the array to tell to stop immediately.
There was a problem hiding this comment.
I see, but I still don't like it. I'll consider this. BTW, there's missing $ character.
There was a problem hiding this comment.
Fixed the $ character.
I can fully understand that it does not looks not very clean. If you have any other idea...
hefee
force-pushed
the
hefee/retrylogin
branch
2 times, most recently
from
c8e41fd
to
f0246ad
Compare
hefee
force-pushed
the
hefee/retrylogin
branch
from
f0246ad
to
daa04b7
Compare
|
Now I have a question. Why would IMAP connection be unstable and not others. Shouldn't we implement the same for at least SMTP? What about managesieve? Maybe the plugin should be named just 'reconnect', so we can extend it with code for other connection types in future, if needed. |
Well I havn't seen the root of issue for quite a while, but when we debugged the issue, we didn't found out why the IMAP connection were closed by the IMAP server only that ~1% failing and retry solved the issue. I think it may happen also for SMTP and managesieve, but as the amount of connections to SMTP and managesieve are several of magnitudes lower than IMAP, it is properly harder to see the issue.
ACK. I understood you correctly, that I do not need to invenst time to add these connection types? |
In order to decide, if we want to retry, we need the result of the last attempt. This information is stored in $this->conn. If a plugin decides to stop we should do not retry another login.
The plugin that will try to reconnect to an IMAP server, if there is no explicit error code replied. If there is a know failure like wrong password, no additional attempts are triggered. This should help in cases, when the connection to the IMAP server is not 100% stable.
hefee
force-pushed
the
hefee/retrylogin
branch
from
daa04b7
to
69e6f1f
Compare
When connection fails in a recoverable way, we should retry to connect
to the server again. This should help in cases, when the connection to the IMAP server is not 100% stable.
In #7402 it was suggested to implement this retry feature as plugin. In order to create such a plugin, it needs the information about the last login attempt. The last attempt is added to the
data['conn'].We also face the issue, that we only call
storage_connectbefore the try to connect. That's why we need a way to stop instantly when no new attempt should be done.(followup of #7402, #5393)