Siskin doesn't register push on login if none is setup (#450)

Unknown opened 4 years ago

Details (please complete the following information):

Siskin Version: 6.0 maybe 6.1
iOS version 13 or latest
iPhone model Recent one, maybe X

I went back in time, and on Sep 23 the subscription was there in the DB.

But, on Sep 22, we talked in the support MUC about how this contact has 2 push subscriptions, for the newely installed Siskin and the old one for Monal. This was giving strange log results so I've looked up on how to delete the old one.

ejabberd has "delete_old_push_sessions" and on Sep 24 I've run ejabberdctl delete_old_push_sessions 0 early in the morning.

@weiss:

Deleting subscriptions shouldn't do real harm as clients re-register for push on the next login.

In theory...

In practice though, Monal (old and new) and Conversations re-registered just fine, but Siskin logged in and out several times since then, yet a push subscription was never recreated.

I'll ask the contact to disable/enable Push in settings, but as usual it might take a while.

Any thoughts?

Unknown commented 4 years ago

Ah, sorry, I was assuming all clients re-register when opening a new session. Doing so allows servers to map push subscriptions to XMPP sessions in order to suppress notifications for connected clients.

Unknown commented 4 years ago

Since version 6.0 SiskinIM checks if there is anything "requiring" reenabling push notifications (ie. chat notifications settings has changed) and if not then it will not try to reenable push notifications.

This change was intentional to reduce "load" on the XMPP server (additional requests every time XMPP-iOS clients logs in - so almost every time the app is opened).

Just to be clear - this behaviour is not a bug. Specification do not mention any requirement of "reenabling" push notifications on each login.

Unknown commented 4 years ago

Does Tigase generate push notifications for a connected client? If it doesn't, how does it map the new session to a push subscription registered via a previous session?

Unknown commented 4 years ago

Tigase sends push notifications if no client is connected.

Unknown commented 4 years ago

So I thought I'd just put back the entry in the DB and call it a day (it should just work, right?), but the Tigase entry looks like this: user mydomain.tld unixtimestamp push.tigase.im 01#longhexstring

While all the other: user mydomain.tld unixtimestamp push.chatsecure/monal/conversations xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx <x type='submit' xmlns='jabber:x:data'><field var='FORM_TYPE'><value>http://jabber.org/protocol/pubsub#publish-options</value></field><field var='secret'><value>sekret</value></field></x>

The form is not needed?

Unknown commented 4 years ago

No, our push service works without a form, so your entry looks ok.

Unknown commented 4 years ago

I've INSTERTed it back in the DB, did not restart the server yet, but I've written one message just in case and later on the contact answered. Lucky coincidence? Dunno 👍

Anyway will keep an eye on it.

@hantu85

This change was intentional to reduce "load" on the XMPP server (additional requests every time XMPP-iOS clients logs in - so almost every time the app is opened).

Sounds good, but this means the server is always perfect, a worthy though, not a reality. :)

Unknown commented 4 years ago

this means the server is always perfect

It also means the XMPP server must choose one of two options to deal with multi-device scenarios:

If at least one client is connected (e.g., a desktop client), don't notify any of your clients.
Always notify all push-registered clients regardless of whether they're currently connected.

Tigase seems to prefer option (1). With (at least) ejabberd and Prosody, (2) will happen (which might lead to superfluous notifications on the phone, I guess?). (What ejabberd and Prosody try to implement is the third option of notifying only disconnected push clients, regardless of the connection status of other devices.)

Unknown commented 4 years ago

Yes, you are correct. However, I'm considering switching to second option of always sending push notifications as it would simplify logic on the client side (should I present a notification from the XMPP stream? or was it already shown as there was a push notification already delivered?)

	Unknown opened 4 years ago Details (please complete the following information): Siskin Version: 6.0 maybe 6.1 iOS version 13 or latest iPhone model Recent one, maybe X I went back in time, and on Sep 23 the subscription was there in the DB. But, on Sep 22, we talked in the support MUC about how this contact has 2 push subscriptions, for the newely installed Siskin and the old one for Monal. This was giving strange log results so I've looked up on how to delete the old one. ejabberd has "delete_old_push_sessions" and on Sep 24 I've run `ejabberdctl delete_old_push_sessions 0` early in the morning. @weiss: Deleting subscriptions shouldn't do real harm as clients re-register for push on the next login. In theory... In practice though, Monal (old and new) and Conversations re-registered just fine, but Siskin logged in and out several times since then, yet a push subscription was never recreated. I'll ask the contact to disable/enable Push in settings, but as usual it might take a while. Any thoughts?
	Unknown commented 4 years ago Ah, sorry, I was assuming all clients re-register when opening a new session. Doing so allows servers to map push subscriptions to XMPP sessions in order to suppress notifications for connected clients.
	Unknown commented 4 years ago Since version 6.0 SiskinIM checks if there is anything "requiring" reenabling push notifications (ie. chat notifications settings has changed) and if not then it will not try to reenable push notifications. This change was intentional to reduce "load" on the XMPP server (additional requests every time XMPP-iOS clients logs in - so almost every time the app is opened). Just to be clear - this behaviour is not a bug. Specification do not mention any requirement of "reenabling" push notifications on each login.
	Unknown commented 4 years ago Does Tigase generate push notifications for a connected client? If it doesn't, how does it map the new session to a push subscription registered via a previous session?
	Unknown commented 4 years ago Tigase sends push notifications if no client is connected.
	Unknown commented 4 years ago So I thought I'd just put back the entry in the DB and call it a day (it should just work, right?), but the Tigase entry looks like this: `user mydomain.tld unixtimestamp push.tigase.im 01#longhexstring` While all the other: `user mydomain.tld unixtimestamp push.chatsecure/monal/conversations xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx <x type='submit' xmlns='jabber:x:data'><field var='FORM_TYPE'><value>http://jabber.org/protocol/pubsub#publish-options</value></field><field var='secret'><value>sekret</value></field></x>` The form is not needed?
	Unknown commented 4 years ago No, our push service works without a form, so your entry looks ok.
	Unknown commented 4 years ago I've `INSTERT`ed it back in the DB, did not restart the server yet, but I've written one message just in case and later on the contact answered. Lucky coincidence? Dunno 👍 Anyway will keep an eye on it. @hantu85 This change was intentional to reduce "load" on the XMPP server (additional requests every time XMPP-iOS clients logs in - so almost every time the app is opened). Sounds good, but this means the server is always perfect, a worthy though, not a reality. :)
	Unknown commented 4 years ago this means the server is always perfect It also means the XMPP server must choose one of two options to deal with multi-device scenarios: If at least one client is connected (e.g., a desktop client), don't notify any of your clients. Always notify all push-registered clients regardless of whether they're currently connected. Tigase seems to prefer option (1). With (at least) ejabberd and Prosody, (2) will happen (which might lead to superfluous notifications on the phone, I guess?). (What ejabberd and Prosody try to implement is the third option of notifying only disconnected push clients, regardless of the connection status of other devices.)
	Unknown commented 4 years ago Yes, you are correct. However, I'm considering switching to second option of always sending push notifications as it would simplify logic on the client side (should I present a notification from the XMPP stream? or was it already shown as there was a push notification already delivered?)