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.

Sign up for GitHub

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

Jump to bottom

feat: Don't add "Failed to send message to ..." info messages to group chats #6318

Merged
merged 1 commit into from
Dec 9, 2024
Merged

feat: Don't add "Failed to send message to ..." info messages to group chats #6318

merged 1 commit into from
Dec 9, 2024

Conversation

iequidoo
Copy link
Collaborator

@iequidoo iequidoo commented Dec 6, 2024

No description provided.

@iequidoo iequidoo marked this pull request as ready for review December 6, 2024 03:59
@iequidoo iequidoo requested review from Hocuri and link2xt December 6, 2024 03:59
@link2xt
Copy link
Collaborator

link2xt commented Dec 6, 2024 •
edited
Loading

For context: these info messages seem to happen very rarely even if NDNs are a normal thing (someone is over quota in a group etc.), info message duplicates the info already seen in the error message behind exclamation mark, info messages do not point to the message that is failed to be sent, and there are no tests for it that fail by removing it.

The reason it rarely works is probably because mimeparser.rs parses recipients from x-failed-recipients and not the standard(?) Final-Recipient or Original-Recipient, so it likely only works for Gmail. Postfix does not add this X-Failed-Recipients header.

I suggest to also remove X-Failed-Recipients parsing and unused failed_recipient field from DeliveryReport. It's a few lines of process_delivery_status that are not a robust way to find out failed recipients anyway.

@iequidoo
feat: Don't add "Failed to send message to ..." info messages to grou…
97f9e95 
…p chats

A NDN may arrive days after the message is sent when it's already impossible to tell which message
wasn't delivered looking at the "Failed to send" info message, so it only clutters the chat and
makes the user think they tried to send some message recently which isn't true. Moreover, the info
message duplicates the info already displayed in the error message behind the exclamation mark and
info messages do not point to the message that is failed to be sent.

Moreover it works rarely because `mimeparser.rs` only parses recipients from `x-failed-recipients`,
so it likely only works for Gmail. Postfix does not add this `X-Failed-Recipients` header. Let's
remove this parsing too. Thanks to @link2xt for pointing this out.
@iequidoo iequidoo force-pushed the iequidoo/no-ndn-info-msg-in-group branch from d9a1fab to 97f9e95 Compare December 6, 2024 15:25
@iequidoo
Copy link
Collaborator Author

iequidoo commented Dec 6, 2024

I suggest to also remove X-Failed-Recipients parsing and unused failed_recipient field from DeliveryReport. It's a few lines of process_delivery_status that are not a robust way to find out failed recipients anyway.

Done.

@iequidoo iequidoo merged commit 0501917 into main Dec 9, 2024
37 checks passed
@iequidoo iequidoo deleted the iequidoo/no-ndn-info-msg-in-group branch December 9, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@link2xt link2xt link2xt approved these changes

@Hocuri Hocuri Awaiting requested review from Hocuri

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@iequidoo @link2xt