state of secure join error handling

[r10s]

there are some recent discussion about error handling in secure-join.

this thread is to sum up things a bit and order the partly complicated flows.

some definitions

• Alice is the inviter who shows the qr-code.
  the ui stays modeless and does not go to a special state on Alice's device, secure-join runs in background from the view of Alice

• Bob is the joiner who scans the qr-code from Alice

  • in case of group-join, the ui shows a modal progress-dialog until the process is finished or aborted by Bob ("Cancel" hit)
  • in case of one-to-one-join, the ui stays modeless [EDITED, initially i wrote Bob is always modal]

current state

current state is that errors on progress on modeless ui show up as "toasts".

for the modal dialog, errors are captured and shown in an alert-dialog on failure (this capturing takes place on android/ios, not sure about desktop).

success is indicated by an info-message in the Alice-Bob chat on both sides.

discussion

my 2 cents:

technically, the capturing and later alert- is error-prone and a bit hard to handle. we recently removed this need-to-capture from dc_configure(), ui-wise, however, this would not make a difference.

on Alice's side, having concrete errors as toasts are easily overseen, esp. as the progress may take longer under bad circumstances. there was the suggestion to put these errors to the device-chat, however, i would prefer putting them to the Alice-Bob chat.

[flub]

Thanks for writing this down!

You say Bob is always modal, but dc_join_securejoin() is only fully blocking when joining a group and returns immediately otherwise. There is an ongoing process allocated though. Should the UIs always perform the Bob operation as blocking?

You say that errors are shown as toasts for Alice. To my understanding currently errors are shown to both Bob and Alice as toast. But there is also the class of protocol failures (and success) which is being sent to the 1:1 chat, again for both sides I think.

I'm wondering if we should never show a toast from the dc_join_securejoin() call and rather return the error, leaving it to the UI to handle that error. The FFI can not carry much error information but perhaps just a we failed message is enough and the user can retry.

For messages in handle_securejoin_message (the protocol handling) the situation is trickier, as there is no-where to report errors. I've been wondering if it would be possible to set and (ephemeral, in-memory only) error state to the 1:1 flag for these. E.g. sending a device message to the 1:1 chat could result in errors, so what do you do? Being able to set an error flag with a message on the 1:1 chat would be interesting. Or maybe have it as a small queue/list even. If the app is killed this would be lost (so there is no DB involved and we can implement this with less code that could error itself. Ideally we can do this in a way the rust compiler can guarantee us no errors.

[r10s]

you're totally right, i've forgotten about this change. yes, bob is also modeless in case of one-to-one-join, i've adapted the initial post to that fact.

Should the UIs always perform the Bob operation as blocking?

no, the change is intended and very useful as allows writing and using the chat immediately and also works when one of Alice or Bob is offline, see

You say that errors are shown as toasts for Alice. To my understanding currently errors are shown to both Bob and Alice as toast.

when Bob joins a group and therefore a modal dialog is shown, in case of dc_join_securejoin() returning 0, an alert is shown with the last error string captured by DC_EVENT_ERROR (at least on android and ios)

But there is also the class of protocol failures (and success) which is being sent to the 1:1 chat, again for both sides I think.

protocol-success is added to the 1:1 chat - why not just add errors there as well?

For messages in handle_securejoin_message (the protocol handling) the situation is trickier, as there is no-where to report errors. I've been wondering if it would be possible [...]

i assume you are talking about handle_securejoin_handshake() - i think, on errors, the progress is aborted. why can't these errors go to the one-to-one chat as well? (to contact_chat_id then) that would feel pretty natural to me. even when doing a group-join.

if we already have the group_chat_id, we could prefer this id.

maybe we should avoid duplicate errors reported there, but i think this is what you meant by:

Being able to set an error flag [...]

otoh, maybe this case is rare enough to ignore it, typically the other side will abort the progress as well.

maybe, no really sure, we can also stop returning 0 from dc_join_securejoin() in case of protocol errors and return the chat_id the error was reported to. the ui will open that chat then and the message gets visible in the right context. just an idea.

[flub]

Entirely agree with reporting errors in the 1:1 chat where possible, and some of the errors are already shown there.

One reasonable thing to do could be to reserve a numberic value in https://github.com/deltachat/deltachat-core-rust/blob/0c03024b97568034bed594bac60ed7647cda98a2/deltachat-ffi/deltachat.h#L4727-L4739 for failures where this is not possible. E.g. 0 could be the protocol failed and we could emit it always, even if we also post a device message in the 1:1 chat

[r10s]

One reasonable thing to do could be to reserve a numberic value [...]

that would be similar to DC_EVENT_CONFIGURE progress where 0 also indicates an error - nice!

however, i would not emit the same code for "error added to 1-to-1 chat" and "no message could be added to 1-to-1 chat" as otherwise the ui might show duplicate error messages which is annoying. so, maybe better only repot 0 in case of the state could not be written to the one-to-one-chat. that would be in-line with dc_join_securejoin() that could also return the 1-to-1-chat-id in case an error-message is added there (dc_join_securejoin() would also only return 0 if this is not possible and something "really" went wrong.

[link2xt]

Closing this issue as currently errors are already added into the chat, could_not_establish_secure_connection adds info messages to the chat.

This was changed in 3b7b8ea (PR #2508).