Skip to content
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

fix(download_msg): do not fail if the message does not exist anymore #6020

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented Oct 3, 2024

Without this fix IMAP loop may get stuck
trying to download non-existing message over and over like this:

src/imap.rs:372: Logging into IMAP server with LOGIN.
src/imap.rs:388: Successfully logged into IMAP server
src/scheduler.rs:361: Failed to download message Msg#3467: Message Msg#3467 does not exist.
src/scheduler.rs:418: Failed fetch_idle: Failed to download messages: Message Msg#3467 does not exist

The whole download operation fails
due to attempt to set the state of non-existing message to "failed". Now download of the message
will "succeed" if the message does not exist
and we don't try to set its state.

Without this fix IMAP loop may get stuck
trying to download non-existing message over and over
like this:
```
src/imap.rs:372: Logging into IMAP server with LOGIN.
src/imap.rs:388: Successfully logged into IMAP server
src/scheduler.rs:361: Failed to download message Msg#3467: Message Msg#3467 does not exist.
src/scheduler.rs:418: Failed fetch_idle: Failed to download messages: Message Msg#3467 does not exist
```

The whole download operation fails
due to attempt to set the state of non-existing message
to "failed". Now download of the message
will "succeed" if the message does not exist
and we don't try to set its state.
@link2xt link2xt added the bug Something is not working label Oct 3, 2024
@link2xt link2xt enabled auto-merge (rebase) October 3, 2024 17:10
@link2xt link2xt merged commit 22e5bf8 into main Oct 3, 2024
37 checks passed
@link2xt link2xt deleted the link2xt/fix-download-nonexistent-message branch October 3, 2024 17:13
// Probably the message expired due to `delete_device_after`
// setting or was otherwise removed from the device,
// so we don't want it to reappear anyway.
return Ok(());
Copy link
Collaborator

@iequidoo iequidoo Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be better fixed in scheduler::download_msgs(), otherwise it's easy to break again. Maybe this should return bool telling whether the message has been downloaded, which is handled download_msgs(). Otherwise the fix is unobvious when looking only here or into download_msgs()

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finally i think this fix is the best option, but there's also update_download_state() that should be changed the same way: #6032

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants