royneary / mod_push

Other
68 stars 19 forks source link

Push notification fail after 144 minutes #17

Open robobit opened 8 years ago

robobit commented 8 years ago

The timeout is adjusted to a "big" value when a client enter the stream wait state, when this time has passed the message fails and the stream is stopped. From mod_push.erl:

-define(ADJUSTED_RESUME_TIMEOUT, 100*24*60*60).

However ejabberd interprets this number as milliseconds which works out to 144 minutes for the given number. I presume this is not intended. I worked around it by making the number bigger, it has to fit in 32 bits so doing:

-define(ADJUSTED_RESUME_TIMEOUT, 49*24*60*60*1000).

sets it to 49 days which is about the max. How about removing the timeout altogether for the purpose of mod_push and have notifications delivered forever?

weiss commented 8 years ago

How about removing the timeout altogether for the purpose of mod_push and have notifications delivered forever?

I'm currently working on changing this logic. Basically, my idea goes like this:

Basically, I'd like to implement the business rules suggested by Daniel.

What do you think?

basak commented 8 years ago

On Fri, May 27, 2016 at 02:03:29AM -0700, Holger Weiß wrote:

This assumes the client will want to go online to retrieve the actual contents as soon as it receives a notification. If this assumption would always hold, and if notification delivery could be assumed to be reliable, I wouldn't actually see a reason to generate additional notifications at all.

Please note that this assumption does not hold on Ubuntu phones. I haven't worked on my app for a while, but I got it to proof of concept stage. It isn't possible for the app to fetch the actual contents of a notification until the user manually opens the app. Ubuntu phone apps are confined this way for user security and privacy.

So in the Ubuntu case, an unlimited timeout is required. Taking this further, if the server does decide to end the session, an Ubuntu phone user will be unaware that messages will stop arriving since the app cannot run to reconnect. If ending a session is necessary, I'd prefer to see a notification at least, so the user knows to restart the app.

I'd like to see something that works for all clients without having to tune per-server configuration. IMHO, this kind of behaviour detail should thus be part of the push notification XEP. Otherwise a server claiming to support the XEP won't really mean anything if it doesn't Just Work with all clients that support the same XEP, and we may end up with a situation where it's impossible to support different clients with the same server implementation and/or configuration because of conflicting assumptions.

weiss commented 8 years ago

Ah, thanks. I wasn't aware of that.

So the push notification will usually trigger a user notification (i.e., a phone beep) directly, rather than the app? This sounds like a severe limitation to me. My idea has always been that push notifications would just wake the app, so the app can then fetch the actual payload and decide whether it's worth notifying the user. I mean, the payload may just be presence data or a ping IQ. Or a MUC message the user might prefer not to be notified about.

Ubuntu phone apps are confined this way for user security and privacy.

Out of curiosity, what are the security/privacy implications of letting an app auto-connect to the service that triggered a notification?

Taking this further, if the server does decide to end the session, an Ubuntu phone user will be unaware that messages will stop arriving since the app cannot run to reconnect.

My suggestion was to generate notifications when messages are written to MAM or offline storage after the session was closed.

I would stop generating notifications at some point (24 hours or whatever). If no notification was generated up to that point, the server could just generate one before running into the timeout (I was going to implement that anyway). If this lets the phone beep, that won't be a nice UX, of course.

I'd like to see something that works for all clients without having to tune per-server configuration.

Absolutely. But getting it right for everyone is not exactly trivial if the behavior differs so much on different platforms.

IMHO, this kind of behaviour detail should thus be part of the push notification XEP. Otherwise a server claiming to support the XEP won't really mean anything if it doesn't Just Work with all clients that support the same XEP, and we may end up with a situation where it's impossible to support different clients with the same server implementation and/or configuration because of conflicting assumptions.

I fully agree that these things are underspecified in the XEP.

basak commented 8 years ago

On Fri, May 27, 2016 at 03:58:16AM -0700, Holger Weiß wrote:

  • basak notifications@github.com [2016-05-27 02:24]:

    On Fri, May 27, 2016 at 02:03:29AM -0700, Holger Weiß wrote:

    This assumes the client will want to go online to retrieve the actual contents as soon as it receives a notification. If this assumption would always hold, and if notification delivery could be assumed to be reliable, I wouldn't actually see a reason to generate additional notifications at all.

    Please note that this assumption does not hold on Ubuntu phones. I haven't worked on my app for a while, but I got it to proof of concept stage. It isn't possible for the app to fetch the actual contents of a notification until the user manually opens the app.

Ah, thanks. I wasn't aware of that.

So the push notification will usually trigger a user notification (i.e., a phone beep) directly, rather than the app? This sounds like a severe limitation to me. My idea has always been that push notifications would

Correct.

just wake the app, so the app can then fetch the actual payload and decide whether it's worth notifying the user. I mean, the payload may just be presence data or a ping IQ. Or a MUC message the user might prefer not to be notified about.

This can be done with the app's notification helper. When a push notification arrives, an app's notification helper is allowed to process it immediately, but with no user interaction and no network access. A push message goes in to the helper and zero or one (IIRC) notifications comes out. This mechanism might suppress a notification according to local settings, for example, but can't do anything else.

So your example of presence data or a ping IQ can be suppressed, as well as a MUC message that the user has to to not be notified about inside the app. But until the user opens the app (or interacts with a notification to open the app), there can be no network communication or any other kind of interaction with the user.

Ubuntu phone apps are confined this way for user security and privacy.

Out of curiosity, what are the security/privacy implications of letting an app auto-connect to the service that triggered a notification?

I believe the idea is that apps do not have network access when the user isn't interacting with it. I don't know if this is the their rationale, but any time an app has network access its developers can track your location.

If an app could wake up when a service that triggers a notification, a malicious (or just badly written) app would be able to bypass the "no network access when in background" restriction entirely just by sending regular "poll" type notifications.

There's also a battery life consideration here too.

I didn't write the rules and am not aware of the formal rationale, but I believe this is the idea, anyway. Anything that apps do need to do in the background can be generalised into a trusted daemon, be part of the system and provide an API to apps, rather than being part of apps themselves. But the bar to getting required functionality into the system is much higher of course.

Taking this further, if the server does decide to end the session, an Ubuntu phone user will be unaware that messages will stop arriving since the app cannot run to reconnect.

My suggestion was to generate notifications when messages are written to MAM or offline storage after the session was closed.

I presume you mean after the connection was closed, but the session remains open? That's what my proof of concept does right now - it closes the TCP connection but leaves the session open. Notifications arrive and prompt the user (but cannot wake the app). When the user reopens the app (either directly or by touching a notification) then the session is resumed and the app receives the messages.

I would stop generating notifications at some point (24 hours or whatever). If no notification was generated up to that point, the server could just generate one before running into the timeout (I was going to implement that anyway). If this lets the phone beep, that won't be a nice UX, of course.

On an Ubuntu phone the app notification helper could create a notification to say that notifcations are stopping and the app needs restarting. Or it could ignore the notification. But both would we poor UX, since in the former case the user gets interrupted, and in the latter case the user silently stops receiving messages.

If necessary, we could do the former, but only after a really long time (eg. a year). Then if the user hasn't used an XMPP app in a year, he'll get a message saying that he'll stop receiving messages, but this doesn't seem too onerous. A 24 hour timeout would obviously be too short under these constraints though.

I'd like to see something that works for all clients without having to tune per-server configuration.

Absolutely. But getting it right for everyone is not exactly trivial if the behavior differs so much on different platforms.

I agree it is difficult. I hope that if we share our constraints, then we can find out what a server needs to do to work for as many client platforms as possible.

It may be that the server needs to dynamically configure some things based on the client platform. In this case, perhaps we can extend the XEP so that there are some per-session settings that a client can specify, which would define the required timeouts and behaviour allow different client platforms to work with their differing requirements?

weiss commented 8 years ago

So this would require stanza contents to be included with the push notification, which everyone else tries to avoid for privacy. This might be less of an issue for the Ubuntu Phone case, where you're not required to use central push infrastructure, IIRC.

The problem is that XEP-0357 doesn't currently allow clients to configure what payload is included with notifications. mod_push allows the XMPP server admin to configure this globally for all clients; but only for the urn:xmpp:push:summary fields suggested in XEP-0357 (which don't allow for distinguishing MUC messages from 1:1 chat messages, for example).

Another problem is that this won't enable the client to respond to IQ requests without notifying the user.

My suggestion was to generate notifications when messages are written to MAM or offline storage after the session was closed.

I presume you mean after the connection was closed, but the session remains open?

No. These are the states I have in mind:

  1. App is online and has an XEP-0198 session.

    -> Everything works as usual, no notifications are generated.

  2. App closes the connection.

    -> The XEP-0198 session is now pending. The server waits for up to (say) 24 hours for a stanza to arrive.

  3. A stanza (that's not queued by CSI) arrives.

    -> The XEP-0198 session is still pending. The server generates a notification and reduces the timeout to (say) 5 minutes. My idea was that most clients would go online within those 5 minutes and the server would return to state 1. But if this doesn't happen:

  4. The server runs into the session timeout of (say) 5 minutes.

    -> The XEP-0198 session is closed.

  5. Another stanza arrives.

    -> That stanza will go into offline storage or (if enabled) into the MAM archive, depending on whether another client is online. The server would generate a notification in either case. It would continue doing so for up to (say) 24 hours. If the client goes online during this time, the server would return to state 1. If this doesn't happen:

    1. The server runs into the push timeout of (say) 24 hours.

      -> The server sends an empty notification and gives up.

On an Ubuntu phone the app notification helper could create a notification to say that notifcations are stopping and the app needs restarting. Or it could ignore the notification. But both would we poor UX, since in the former case the user gets interrupted, and in the latter case the user silently stops receiving messages.

If necessary, we could do the former, but only after a really long time (eg. a year). Then if the user hasn't used an XMPP app in a year, he'll get a message saying that he'll stop receiving messages, but this doesn't seem too onerous. A 24 hour timeout would obviously be too short under these constraints though.

I doubt it's acceptable to all admins that stale push sessions won't be cleaned up before a year has passed. And you certainly don't want XEP-0198 sessions to run anywhere near that long.

It may be that the server needs to dynamically configure some things based on the client platform. In this case, perhaps we can extend the XEP so that there are some per-session settings that a client can specify, which would define the required timeouts and behaviour allow different client platforms to work with their differing requirements?

Yes, that might be necessary. Though you'll always want to allow admins to restrict timeouts to sane ranges.

robobit commented 8 years ago

Wouldn't it be better to separate the whole push notifications from the XEP-0198 sessions? There is no association with it in the Push Notifications XEP-0357. It seems it is just this implementation that uses the stream management to trigger the notifications. Isn't it an idea to just let the client register and enable push notifications (as happens now), and then whenever there is a message for the client always do a push notification? This seems simpler and puts the responsibility whether or not to send notifications entirely with the client.

weiss commented 8 years ago

Wouldn't it be better to separate the whole push notifications from the XEP-0198 sessions? There is no association with it in the Push Notifications XEP-0357. It seems it is just this implementation that uses the stream management to trigger the notifications.

The XEP unfortunately says nothing at all about how these things should work, that's true.

If you just notify on offline messages as other implementations do, you won't receive any notifications if another client is online. So you will at least also want to notify on MAM messages (and then only use clients with MAM support). Then you still have the problem that you won't receive notifications after loosing the connection, before the XEP-0198 session died. And you'll appear as offline. And you won't receive MUC messages.

It's more complex to implement the interaction with XEP-0198 properly, but I think it's way better to do it that way. Otherwise things will only work sometimes, and things that work sometimes are a horrible UX.

That said, if the client is fine with those limitations, the client is of course free to just close the stream properly and then only receive notifications for offline/MAM messages.

iNPUTmice commented 8 years ago

Can I suggest that anyone who disagrees with these Business rules takes the discussion to the corresponding mailing list thread. This way we have all the discussion in one central place and all server developers can benefit and more importantly adopt a similar behavior. The goal is to eventually incorporate those rules directly into the XEP. (Maybe wait for the ejabberd implementation and some real life tests)

Regarding OPs problem I believe that this is already covered by those business rules. The client just ends the stream and thus gets notified by means of scenario 4b.