fix: add info message if user tries to create a QR code for deprecated channel #7399
Conversation
src/securejoin.rs
Outdated
| true, | ||
| ) | ||
| .await; | ||
| if let Err(e) = res { |
There was a problem hiding this comment.
This part can now be reverted, solution for dealing with exceptional receive_imf errors is at #7401
There was a problem hiding this comment.
True, it'd good to have more informative messages like this (and in the corresponding chats, not in the device chat), but we can't add such a code for all failing paths
Hocuri
force-pushed
the
hoc/fix-broadcast-infinite-loop
branch
from
9e5f21a to
ebd4da5
Compare
src/securejoin.rs
Outdated
| if chat.typ == Chattype::OutBroadcast { | ||
| // If the user created the broadcast before updating Delta Chat, | ||
| // then the secret will be missing, and the user needs to recreate the broadcast: | ||
| // let secret = load_broadcast_secret(context, chat.id).await?; |
There was a problem hiding this comment.
Is this commented out code somehow related to the comment above?
There was a problem hiding this comment.
No, it was an old version of the code, forgot to remove
|
Infinite loop is already fixed on main, so this change is something else, more like "add info message if user tries to create a QR code for deprecated channel" |
Hocuri
changed the title
Before this PR, when a QR code couldn't be loaded because of chatmail/core#7399, Delta Chat crashed. Now, the button does nothing.
| if load_broadcast_secret(context, chat.id).await?.is_none() { | ||
| warn!(context, "Not creating securejoin QR for old broadcast"); | ||
| let text = BROADCAST_INCOMPATIBILITY_MSG; | ||
| add_info_msg(context, chat.id, text, time()).await?; |
There was a problem hiding this comment.
Why showing an error toast isn't enough? Because the message is large? The info message will persist, this doesn't look useful
There was a problem hiding this comment.
Yes.
The info message is supposed to persist forever; this broadcast channel can't be used anymore and has to be recreated, because the secret is missing.
There was a problem hiding this comment.
warn! could still be replaced with an error! so there is some short toast in addition to info message, but the info message is for having a guide on how to migrate.
There was a problem hiding this comment.
Ok, then makes sense to make it an error and also log chat.id
| // If the user created the broadcast before updating Delta Chat, | ||
| // then the secret will be missing, and the user needs to recreate the broadcast: | ||
| if load_broadcast_secret(context, chat.id).await?.is_none() { | ||
| warn!(context, "Not creating securejoin QR for old broadcast"); |
There was a problem hiding this comment.
| warn!(context, "Not creating securejoin QR for old broadcast"); | |
| warn!(context, "Not creating securejoin QR for old broadcast {}.", chat.id); |
Before this PR, when a QR code couldn't be loaded because of chatmail/core#7399, Delta Chat crashed. Now, the button does nothing. It's not perfect, but easy to do, and only about an edge case where the user tried out the experimental broadcast channels in the past, and now tries to add a new member to a broadcast channel created with an old version of DC. The hollistic alternative would be to switch to RPC for getSecurejoinQrSvg, so that the UI can get a proper error message. --------- Co-authored-by: adb <adb@merlinux.eu>
Fix #7397:
If adding a member to a channel fails, add an info message with the error rather than landing in an infinite imap loopI think about how we could add tests yet, and I didn't think yet about whether this is the best solution.