sending message from mobile not replicated (spectrum2 & Gajim)

[ticktostick]

using spectrum2 latest build from source
getting this error on purple backend log
The message from mobile phone is not replicated on purple but all other parts works ok (messages sending from others and messages send by me from other than main mobile phone)
ERROR backend: g_log purple_conversation_get_im_data: assertion 'conv != NULL' failed
DEBUG backend: conv_write_im(): msg='?', flags=66561

don't have this problem on pidgin (without Spectrum2 gateway)

[hoehermann]

don't have this problem on pidgin

I only try with Pidgin, that is why I did not catch this. Thank you for the report.

Does this affect direct messages or group messages?

[ticktostick]

I only checked the direct message will try The group and report

[hoehermann]

There was some weirdness in the code for direct messages, I fixed it in the dev branch. Even before that, it read like this:

PurpleIMConversation *imconv = purple_conversations_find_im_with_account(username, account);
if (imconv == NULL) {
    imconv = purple_im_conversation_new(account, username);
}
PurpleConversation *conv = PURPLE_CONVERSATION(imconv);
…

I do not see how conv would end up being NULL unless purple_im_conversation_new somehow failed.

[ticktostick]

checked the dev branch
the error has gone I have only
DEBUG backend: conv_write_im(): msg='Test3', flags=66561

but the problem persist then checked the spectrum log found this

Conversation: Carbon to @.*/gajim.JUJJGR6U skipped (originated entity)

I think it must me spectrum2 problem here

[hoehermann]

Indeed, a change in spectrum disallows receiving one's own messages: SpectrumIM/spectrum2@7d7680c

[hoehermann]

Hi @vitalyster

I am receiving a number of reports regarding this issue. It seems quite a lot of users send messages to themselves for notes, reminders, etc. to be synced to all their devices. It would be nice to have this working again in Spectrum.

With XMPP, you send a message and get an immediate response from the server. The request either succeeded or failed – which is why send_im(…) allows positive and negative results as well as 0 for a special case.

WhatsApp works a little bit different than traditional protocols since it has been designed for mobile phones with an unreliable intermittent connection. In the given implementation, you do not actually send a message but rather append it to an opaque output buffer. The messages are then transferred in the background. If the server received a message, it sends a receipt. This entire process is working asynchronously. In case of connectivity issues, the server might answer late or never answer at all.

Since there might be no answer and I really dislike arbitrarily chosen time-outs, I cannot wait for the server to answer. This leaves me with three interpretations when sending a message:

1. Appending to the buffer is a success (return 1 immediately). I think this is dangerous since users think the message has been sent when in fact it is not.
2. Hide the message (return 0), wait for the server receipt and then write it back into the conversation. This seems to be the most reasonable choice hence I picked this one as the default. Since it is possible to receive messages sent by oneself, I just use the same function (messages written on other devices and message written on this device). It works really nice in Pidgin.
3. Hide the message (return 0) and never write it back to the conversation. Allegedly, some people prefer it this way. Apparently, some clients do not consider 0 as a special case and always perform a local echo.

I offer all these interpretations in the setting echo-sent-messages, so it really is a user-choice here. Maybe I should pick a different default setting when the plug-in is executed in Spectrum. If so, which one is good for Spectrum?

[vitalyster]

I'm pretty sure Spectrum should work the same way as Pidgin and not display these messages without any user settings.
Why Pidgin does not display it? Does it contain some specific flag? I can just make Spectrum work the same way and ignore message with that flag or something

[hoehermann]

Instead of a flag libpurple uses the return value. The documentation to send_im says "If the message should not be echoed to the conversation window, return 0." That is what the plug-in is using by default.

[vitalyster]

Well, I have an idea. To distinct local messages and messages from other clients we send messages with PURPLE_MESSAGE_SPECTRUM2_ORIGINATED (0x80000000) flag, and when message is came back in write_im or write_chat callback it have the same flag there. Maybe gowhatsapp erase that flag on message echoing? That may be the root issue of the problem.
Additionally you should add PURPLE_MESSAGE_REMOTE_SEND and PURPLE_MESSAGE_DELAYED to messages from other clients.

[hoehermann]

I am already doing something similar, but I am using all three flags at the same time. Maybe that is where I am using it wrong. Should I be using PURPLE_MESSAGE_SEND for "successfully sent from this connection" and PURPLE_MESSAGE_REMOTE_SEND for "sent by myself but from other device"? With that distinction, it feels like PURPLE_MESSAGE_SPECTRUM2_ORIGINATED would not be needed, does it?

[vitalyster]

Right now PURPLE_MESSAGE_SEND alone is treated by Spectrum as "sent from other devices" and PURPLE_MESSAGE_SEND | PURPLE_MESSAGE_SPECTRUM2_ORIGINATED is "sent from this connection". Actually I can remove that outdated behavior and check for PURPLE_MESSAGE_REMOTE_SEND but in this case we need to review compatibility with other plugins which are not updated to this new flag.

[vitalyster]

The main issue is still remains - why our flags are disappeared? :) I think you should keep flags from the original message

[hoehermann]

I just noticed some interesting code in purple_conv_chat_write. For chats, the flags are manipulated based on the local user's name compared to the chat participants. Since the user information are obtained by the QR code, the local user's name might not actually be their WhatsApp identifier, so this can set the wrong flags. There might be more manipulations like these in other purple functions. Thank you for pointing that out. I need to look into this more thoroughly and then write to you again.

[EionRobb]

1. Appending to the buffer is a success (return 1 immediately). I think this is dangerous since users think the message has been sent when in fact it is not.
2. Hide the message (return 0), wait for the server receipt and then write it back into the conversation. This seems to be the most reasonable choice hence I picked this one as the default. Since it is possible to receive messages sent by oneself, I just use the same function (messages written on other devices and message written on this device). It works really nice in Pidgin.
3. Hide the message (return 0) and never write it back to the conversation. Allegedly, some people prefer it this way. Apparently, some clients do not consider 0 as a special case and always perform a local echo.

Just to throw another idea into the ring: what about displaying the sent message immediately, but then showing a "this message could not be sent" error message in the IM later?

[hoehermann]

I also thought it was a good idea. That was actually the way I implemented it in the beginning. It turned out to be quite confusing.

Especially if you rapidly send a couple of messages and one of them fails, then you need to check which one it was.

Also one might send a message, it pops up in the conversation immediately, then they close Pidgin and miss a delayed error message.

[EionRobb]

Is there an opportunity to auto-retry on a failed send? Can the error message have a portion of the message text in the "could not send message 'xyzabc...'" ?

[hoehermann]

Yes, there is. Though I cannot really see the advantages over the current approach. The current aproach feels responsive, intuitive and the code is relatively simple.

I want to move towards a more established style. That includes enforcing correct usernames, setting the own nick in group chats, using serv_got_chat_in instead of writing to the conversation directly,… and hoping this will also align everything with Spectrum. :)

On a side-note: Blocking is definitely not an option. Latency is way too big to make chatting a comfortable experience.

[hoehermann]

Dear @vitalyster, I have now reworked the way messages are sent and displayed. I am using the standard purple_serv_got_chat_in and purple_serv_got_im in nearly all places. Exception: When displaying a message that has been sent, purple_conv_im_write is used. The flags are set to PURPLE_MESSAGE_RECV for all incoming messages and PURPLE_MESSAGE_SEND for all outgoing messages (regardless if sent by the local connection or phone). If a message has been sent via the main phone (or browser), PURPLE_MESSAGE_REMOTE_SEND is set additionally. In Spectrum, only replicating messages which have that flag set seems like a sensible mechanism.

Even more sensible: I made it possible to use blocking function calls. This plug-in now behaves more normal. I have made this the default setting and hope GUI users won't complain too much. I hope this works out in our favor. :)

[hoehermann]

Complaints are already coming in. It turns out that the internal echo of locally sent messages only is implemented for direct messages, but not for group chats: https://keep.imfreedom.org/pidgin/pidgin/file/v2.14.12/libpurple/conversation.c#l191

I had to make the Pidgin-friendly setting "on-success" the default again, unfortunately.
Spectrum users are advised to use "internal" or "never".

[vitalyster]

@hoehermann I receive complains that it does not work at all :) So maybe the problem is not in method but there is some implementation bug. I will check it later.

[hoehermann]

I added an explicit echo for group chats. Flags are forwarded as generated by the caller. Now the echo mode "internal" is working as intended on Pidgin. I created a bugfix-release tag v1.11.1. I do not think it will change anything for Spectrum.

I would really like to test against Spectrum myself. Unfortunately, I still fail to build libSwiften on amd64 Debian 11 "bullseye". scons completely disregards libSwiften's internal dependencies. It says "3rdParty/LibNATPMP is up to date" but actually it is never built. 🤷

On my preferred Ubuntu 22.04 machine, scons refuses to build libSwiften since qt5 is not available (only qt6). For swift (the GUI application), I would expect that. Why libSwiften would need qt5, too, I have no idea.

I am giving up on this for now.

[vitalyster]

We have Swiften in packages.spectrum.im repository ;)