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.

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

Sent data handler, or socket/tls write error handling #82

Open
stanson-ch opened this issue Feb 6, 2016 · 3 comments
Open

Sent data handler, or socket/tls write error handling #82

stanson-ch opened this issue Feb 6, 2016 · 3 comments

Comments

@stanson-ch
Copy link
Contributor

Seems that there is one thing missed in xmpp_run_once.

xmpp_send just put data into send queue and immidiately returns, so it can't return the result of actual socket/tls write.

If connection error occur while xmpp_run_once sends queued data, there are no way to find out what data from the queue was sent and what was not. Also, there are no way for queue with unsent data to survive reconnect, since connection queue uses the same queue for authorisation etc, so, you have to release connection and connect again loosing send queue without possibility to save it somehow.

There must be some notification about error with possibility to save unsent data, or, in better case allow a callback (handler) for "queued data item sent successfully" event with a pointer to sent data, just before xmpp_free(ctx, sq->data);. In latter case application can mark data as sent/unsent and try to resend something in case of reconnect.

I can try to write some patch for this, but not shure what is the right solution.

@pasis
Copy link
Member

pasis commented Feb 7, 2016

Libstrophe is low-level library and this applies some restrictions. We can't simply resend items, because they may be valid only within current session. Resending is responsibility of user (user of the library, program in most cases).
Second, any change must not break existent programs.

You're right, would be nice to have some "reliable" interface or access to unsent items. You can try to play around connection_handler. User notified with event XMPP_CONN_DISCONNECT from conn_disconnect() on errors. And at this point conn contains unsent items. Just a quick thought: you can make interface xmpp_conn_unsent_nr()/next() and user can go through the messages and save ones need to be resent. But user doesn't work with raw strings, so I think next() should return xmpp_stanza_t. Libstrophe stores complete stanzas in raw strings in the send_queue what makes this approach possible.

Also, as you said, send_queue should be cleared on a connection error. Since connection_handler may want to access the queue and call xmpp_connect_client() we can't clear queue neither before nor after handler's call in conn_disconnect(). But we can clear it at the top of connect-like functions with some static function conn_send_queue_purge().

What about handler for successfully sent items: user can mix xmpp_send() and xmpp_send_raw().

@stanson-ch
Copy link
Contributor Author

User notified with event XMPP_CONN_DISCONNECT from conn_disconnect() on errors. And at this point conn contains unsent items. Just a quick thought: you can make interface xmpp_conn_unsent_nr()/next() and user can go through the messages and save ones need to be resent.

Good point, looks like solution. Will try.

Also, as you said, send_queue should be cleared on a connection error. Since connection_handler may want to access the queue and call xmpp_connect_client() we can't clear queue neither before nor after handler's call in conn_disconnect(). But we can clear it at the top of connect-like functions with some static function conn_send_queue_purge()

Purging queue in xmpp_connect_client is a good idea. This will allow to make "short cycle" reconnects possible, without releasing connection, creating new connection and setting same credentials again. Will try to make a patch.

@pasis
Copy link
Member

pasis commented Apr 26, 2016

Fix for the send_queue is already in master and reconnect works. Access to unsent data is not in priority for the next release (0.9), but may be added in some future release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants