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

Revise message deletion from SMTP queue #4579

Closed
3 tasks done
link2xt opened this issue Jul 25, 2023 · 2 comments
Closed
3 tasks done

Revise message deletion from SMTP queue #4579

link2xt opened this issue Jul 25, 2023 · 2 comments
Assignees
Labels
bug Something is not working good first issue Good for newcomers

Comments

@link2xt
Copy link
Collaborator

link2xt commented Jul 25, 2023

Currently message sending is implicitly cancelled by removing the message from the database or trashing it:

// If there is a msg-id and it does not exist in the db, cancel sending. this happens if
// delete_msgs() was called before the generated mime was sent out.
if !message::exists(context, msg_id)

This logic is difficult to test with Rust tests as you need to run actual SMTP sending code for that and does not allow to delete a message without cancelling sending as reported in #2363

This issue is a replacement for #2363 and #4387. #3685 is related.

To close this issue, we need to:

  • Remove the code from SMTP sending code that checks if the message is in the msgs table.
  • Explicitly delete the message from SMTP queue when the user calls delete_msgs and tests this with a Rust test (fix: Delete messages from SMTP queue only on user demand (#4579) #4615)
  • Test with a Rust test in ephemeral.rs that when outgoing message expires, it is deleted from the chat but can still be extracted from SMTP queue on Alice side, received on Bob side and result in a displayed message.
@link2xt link2xt added bug Something is not working good first issue Good for newcomers labels Jul 25, 2023
@iequidoo iequidoo self-assigned this Aug 8, 2023
iequidoo added a commit that referenced this issue Aug 9, 2023
I.e. from delete_msgs(). Otherwise messages must not be deleted from there, e.g. if a message is
ephemeral, but a network outage lasts longer than the ephemeral message timer, the message still
must be sent upon a successful reconnection.
iequidoo added a commit that referenced this issue Aug 10, 2023
I.e. from delete_msgs(). Otherwise messages must not be deleted from there, e.g. if a message is
ephemeral, but a network outage lasts longer than the ephemeral message timer, the message still
must be sent upon a successful reconnection.
iequidoo added a commit that referenced this issue Aug 12, 2023
I.e. from delete_msgs(). Otherwise messages must not be deleted from there, e.g. if a message is
ephemeral, but a network outage lasts longer than the ephemeral message timer, the message still
must be sent upon a successful reconnection.
iequidoo added a commit that referenced this issue Aug 15, 2023
I.e. from delete_msgs(). Otherwise messages must not be deleted from there, e.g. if a message is
ephemeral, but a network outage lasts longer than the ephemeral message timer, the message still
must be sent upon a successful reconnection.
iequidoo added a commit that referenced this issue Aug 17, 2023
I.e. from delete_msgs(). Otherwise messages must not be deleted from there, e.g. if a message is
ephemeral, but a network outage lasts longer than the ephemeral message timer, the message still
must be sent upon a successful reconnection.
iequidoo added a commit that referenced this issue Aug 24, 2023
I.e. from delete_msgs(). Otherwise messages must not be deleted from there, e.g. if a message is
ephemeral, but a network outage lasts longer than the ephemeral message timer, the message still
must be sent upon a successful reconnection.
@hpk42
Copy link
Contributor

hpk42 commented Aug 24, 2023

Just for the record, i just got another accident: two messages sent and deleted while.offline were sent when I came online 10 minutes later. Manual deletion basically doesn't work. This is 1.40.0 android. I reported this issue early may already btw. As #4387

iequidoo added a commit that referenced this issue Aug 24, 2023
I.e. from delete_msgs(). Otherwise messages must not be deleted from there, e.g. if a message is
ephemeral, but a network outage lasts longer than the ephemeral message timer, the message still
must be sent upon a successful reconnection.
link2xt pushed a commit that referenced this issue Aug 24, 2023
I.e. from delete_msgs(). Otherwise messages must not be deleted from there, e.g. if a message is
ephemeral, but a network outage lasts longer than the ephemeral message timer, the message still
must be sent upon a successful reconnection.
@link2xt
Copy link
Collaborator Author

link2xt commented Aug 24, 2023

The fix #4615 is merged into master and I also cherry-picked it into stable as ba2c365

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

No branches or pull requests

3 participants