Handling failure to send message

When a message fails to send, how should/does this show up in the UI? Looking through the Pidgin 3 code, in pidginconversation.c a PurpleMessage gets created and PurpleMessage.send_message_async is called, but there is not a callback to handle a potential error.

In conversation.ui, it looks like there is a status label that displays errors from the PurpleConversation object, but not errors from the PurpleMessage objects. Maybe the errors could be propagated from the PurpleMessage to the enclosing PurpleConversation somehow?

Message errors are handled via the PurpleMessage:error property. The UI just fires and forgets for now because it wants to get the message visible in the UI as fast as possible.

Checking message.ui it looks like we didn’t put the error stuff in there yet. But there’s lots of other stuff missing from there too like delivery status, read receipts, retrying on failure, etc etc.

Part of what’s blocking that is we need lookup capabilities in PurpleMessages, which is relatively easy, but just hasn’t been done yet.

But the idea is that when the protocol finds out a message fails, it looks up the message if it doesn’t have it, and then just sets the properties on it. This also means you should use a temporary id for the message and then resolve it when the server confirms delivery and has the final id.

1 Like

I’ve been thinking about the API for Purple.Messages and I’m leaning towards something like the following.

void purple_messages_find_message_async(PurpleMessages *messages, const char *id, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer data);
PurpleMessage *purple_messages_find_message_finish(PurpleMessages *messages, GAsyncResult *result, GError **error);

This is async because eventually we’ll have a backend to this which will store the messages locally. That design hasn’t been completed but we should assume that any IO operation like this will need to be async.

However, the dilemma I’ve having is that most of the time we’ll be looking for recent messages so we’ll want to start at the end. So in theory this should do the same, but maybe we want to search from the start for some reason?

Although as I’m typing this I’m realizing the in memory version can just start at the end as the backend solutions should be indexed on the id and this optimization won’t be necessary.

We also need to add Purple.Messages.remove_async. I’m torn on this one taking a message or id or possibly having separate functions. But I figure we’ll probably have the id the majority of the time and not a full message instance, so it’s probably fine to just do the id method.

Anyone have any thoughts here?

Using just the ID for the find and remove functions makes sense to me, and it keeps their function signatures consistent.

1 Like

I’ve created PIDGIN-18179 and PIDGIN-18180 to get these done.

Ugh looking at this a bit deeper and apparently I didn’t finish off some stuff. Right now Conversation:messages is just a GListStore and not PurpleMessages.

Per the docs, we built PurpleMessages to:

This collection is meant to make it easy to pass around a number of related messages. For example getting messages from a server or displaying a few messages in a search result.

So it might be confusing, but we should probably create Purple.History for all of this.

Thoughts?