-
-
Notifications
You must be signed in to change notification settings - Fork 103
Do not ignore errors when updating quota or resyncing UIDs #7010
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
0c17f7f
to
ad13477
Compare
4f6c071
to
758adb2
Compare
Another parallel PR to make it less dangerous even if the error is ignored and the stream is not dropped after the first error: |
let quota = if session.can_check_quota() { | ||
let folders = get_watched_folders(self).await?; | ||
get_unique_quota_roots_and_usage(session, folders).await | ||
Some(get_unique_quota_roots_and_usage(session, folders).await?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, does this mean that if a provider erreneously says that it has the quota capability, while it actually does not, connecting to this provider will fail?
Not sure whether this ever happens, but also, not sure whether we should include this right before the release - otoh, it hopefully also fixes a bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before, quota.modified
was updated in this case, maybe still do so and then bubble up the error? This way we won't try to update the quota again after reconnecting. This would be closer to the previous behavior
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's merge it as is after 2.x releases and see if anyone reports problems. I will try to finish #7014 instead, it should prevent stream reuse after error while still allowing to ignore errors here.
758adb2
to
1f41177
Compare
This is an attempt to debug #6477. It looks like at least once IMAP loop got stuck after updating quota timed out without logging anything.
First commit adds some logging and makes
update_recent_quota
bubble up the errors. Next commits turn errors fromupdate_recent_quota
andresync_folders
into loop failure.It is dangerous to ignore network errors during IMAP loop because if there is a timeout during reading, it does not mean that the next read will not succeed and return leftover or later received bytes. This is how tokio-io-timeout works and this is consistent with how normal TCP connections and C sockets work - timeout errors does not mean the socket will return the same error on the next call. I still don't know however why this can result into the loop getting stuck, I/O should timeout eventually, so there is likely some parser error involved anyway, but not feeding leftover bytes into it makes it more difficult to trigger at least.