w3c / push-api

Push API
https://w3c.github.io/push-api/
Other
145 stars 40 forks source link

Make PushSubscription.expirationTime mandatory #302

Open collimarco opened 5 years ago

collimarco commented 5 years ago

Push subscriptions can last many years without expiring and there is a negative correlation between the age of a push subscription and its expected delivery rate, as described by this study (that I have published):

https://blog.pushpad.xyz/2018/09/web-push-subscription-age-affects-delivery-rates/

Enforcing the browser to set an expirationTime for all the push subscription generated, would be helpful. Basically that will ensure that an unused endpoint will always be removed after some time... otherwise the number of old, unused endpoints will increase steadily over time (and the average delivery rate will tend toward zero).

martinthomson commented 5 years ago

This is less a question of API design and more one of policy. I'd suggest taking this to the browsers. For Firefox, all I can say is that we don't have the resources to make any changes to the push code right now.

marcoscaceres commented 5 years ago

@martinthomson as I'm new to the Push work, I don't know who the key implementation contacts are outside of Mozilla. Could you kindly suggest a couple of folks that we could cc here, just so they are aware?

@collimarco, I've filed a bug for you on the Firefox side: https://bugzilla.mozilla.org/show_bug.cgi?id=1503779

Please cc yourself on the bug above - and we may have some follow up. But as Martin stated, we are focused on other things right now - so please be patient while we get to it (it could take up to a year).

collimarco commented 5 years ago

This is less a question of API design and more one of policy. I'd suggest taking this to the browsers.

@martinthomson The point is that until the standard says that the expiration is optional, browser manufacturer are free to ignore this and they don't set any expiration... indeed no current browser sets the expirationTime.

This issue is really a pain... valid subscriptions (that return successful status codes) have such low delivery rates due to this problem and it is difficult to explain that to normal people (still we don't want to do like other services that simply show the sending rate as if it was the delivery rate...). Also it doesn't really make sense that the number of inactive subscriptions (that belong to abandoned devices and browsers) grows constantly over time. Fixing this is also relevant for privacy, because it would allow us to delete unused subscriptions together with all the associated data.

marcoscaceres commented 5 years ago

@collimarco, please understand that even if we were to put something into the spec today, without browsers first agreeing to follow the spec, it’s just empty words.

Specs don’t magically make implementations happen: we, more often than not, fix implementations first and then update the spec to match.

That’s why Martin suggests going through browsers first. Getting things implemented first will help us understand exactly what we should put in the spec.

aliams commented 5 years ago

@collimarco Microsoft Edge sets expirationTime on the subscription (30 days from creation) and implements the onpushsubscriptionchange event that allows sites and apps to handle the case when Microsoft Edge refreshes a subscription that is 5 days from expiration. I anticipate that other browsers would be interested in setting expirationTime and implementing onpushsubscriptionchange as well for the reasons that you mentioned. @beverloo, are there plans for Chrome to set expirationTime to a value other than null?

beverloo commented 5 years ago

Yeah, Chrome plans to match Edge in this regard. I don't have a timeline, but would expect this to pop up somewhere next year.

I don't have strong feelings about whether we should mandate this in the spec or not. I'd like to have a better understanding on whether it's possible to implement this behaviour natively on various operating systems with various push services. Push services might also employ alternative mechanisms for keeping subscriptions current, invaliding subscriptions after a very short period of time without connections for example, in which case enforcing browser-side behaviour might be unnecessary.

collimarco commented 5 years ago

I agree that there are two strategies here:

  1. Making the PushSubscription.expirationTime mandatory allows the application server to safely delete old endpoints in case the push service forgets to do it
  2. The browser push service should detect inactive devices and mark the endpoint as expired (i.e. return 410 Gone to the application server)

However none of them is enforced by the standard at the moment. I think that at least one should be enforced by the standard in order to avoid future problems. Also, the two strategies can coexist: if a browser decides to implement both that would be great.

aliams commented 5 years ago

That would indeed be ideal. Both are currently implemented by Microsoft Edge and the Windows Notification Service.

borisavz commented 4 years ago

I've asked this on Stackoverflow too, has any progress been made? Link to my question.

How can apps with authentification limit the time a subscription is valid? So far, I think only browser can limit time, but field expirationTime is still not implemented properly.

novaknole commented 4 years ago

Any progress about this?

how do I know which ones are active/inactive? still the only way to detect this is if request from my app server to push server returns 410?

collimarco commented 4 years ago

still the only way to detect this is if request from my app server to push server returns 410?

Unfortunately yes, that is the main method and requires a push to the subscription. Or you can also listen for pushsubscriptionchange and remove the old subscription (but this can be applied only to some situations).

novaknole commented 4 years ago

@collimarco do you use pushsubscriptionchange on your web page? I mean is it worth using it?

1) The idea is that subscription still might change depending on who knows!. but this means that wherever I have the subscription code in my app (not in service worker), it will return the new subscription if it got changed. So what's the deal of pushsubscriptionchange ?

collimarco commented 4 years ago

@novaknole Absolutely, you need to handle the pushsubscriptionchange otherwise you constantly lose subscribers for no reason... browser can replace a subscription at any time (and actually they do that, based on our server logs).

If you need inspiration for handling pushsubscriptionchange you can look at our service worker here: https://pushpad.xyz/service-worker.js

Then on your server you simply replace the old subscription with the new one.

but this means that wherever I have the subscription code in my app (not in service worker), it will return the new subscription if it got changed.

Yes, it will. But in the meantime, if you don't handle the subscription change, the user will stop receiving the notifications (until he visits your website again and subscribes again).

novaknole commented 4 years ago

@collimarco Thanks for your explanation. A couple of things I want to add here .

1) The final idea is that we don't know when browser decides that subscription is valid or not. So we send a request to endpoint from app server and if it returns any code other than 201, we decide to say that this subscription is NOT valid and we remove it from our database. Right?

2) because of the fact that I only let users subscribe to my app when they are logged in, it means that i assign user_id to endpoint. Turns out in service worker, i can't grab the user_id . So only way to send the subscription is without user_id. but i don't think that's gonna be a problem, since my api would take the old endpoint and find the record with it. If it exists, it replaces it with the new one . Right?

3) Do I still need to have the code which gets called each time user refreshes the page? the code which gets subscription and sends it to the server. Right now, what I have is i only send the subscription if user agreed to that permission and if the subscription couldn't be saved in the database. This makes sure that i only execute sending subscription to my server only one successful time. If I leave the logic this way, the risk is that when pushsubscriptionchange gets called, fetch might result in an error. pushsubscriptionchange won't get called again and I lose the new subscription. What do you think?

collimarco commented 4 years ago

@novaknole

  1. Yes, right. However you need to handle each status code: check the push service docs for the meaning of each status code they can return and handle appropriately. For Pushpad for example we handle temporary errors and permanent errors differently. Also we use a trash (and not delete directly) to prevent temporary errors from deleting all the data.
  2. Yes, exactly: you can just replace the subscription and keep the old data, since you know the old endpoint.
  3. You need to check frequently with your server if the subscription is actually present in your server, otherwise you are right: you can lose the subscription (since pushsubscriptionchange and fetch are not 100% safe). You need to use some sort of caching mechanism (e.g. localStorage or sessionStorage) in the browser to avoid too many requests to the server (e.g. on each page is too much), but you also need to send checks to the server sometimes(to make sure that the subscription is actually in your database).
novaknole commented 4 years ago

@collimarco

Thanks.

1) by using trash, i think you mean soft delete. What possible scenario can you think of which tells you that it's a good idea to use soft delete instead of delete from db?

2) why would you keep the old data if you realized that old endpoint was found in db and can be replaced with the new endpoint ?

3) how would you go with this? I mean you would use some kind of timestamp which lets you send the subscription request every 1 hour?

collimarco commented 4 years ago

@novaknole

  1. Yes, I mean soft delete. For example a temporary maintenance to a push service that cause them to return a 404... and you have lost everything.
  2. I mean, if you want to keep extra data that your application has associated to the user.
  3. That depends on your application design.
novaknole commented 4 years ago

@collimarco

Question 1)

I was thinking about checking subscription for each specific time so that server always has the new one. regarding my question 3)

What if in my service worker's pushsubscriptionchange, I call fetch api call and if it fails, in a catch block, I put newSubscriptionTriggered:true in the indexeddb.

Way 1) what we could do is use setInterval in service worker and check indexeddb and if it has newSubscriptionTriggered:true in there, call the fetch api again . If it fails, we do nothing as another interval would kick in. If it succeeds, we remove newSubscriptionTriggered:true from the indexeddb. The following setInterval call wouldn't call fetch as there's no newSubscriptionTriggered:true in there.

Way 2) What we could do is instead of using setinterval in service worker, we could do that check in our app code. and do the same. The downside of it is that what if user doesn't go to our website for some time and pushsubscriptionchange 's fetch failed. It means that new subscription won't be saved to db until user checks our website again where I check indexeddb's newSubscriptionTriggered:true and call fetch again.

Way 3) don't use indexeddb at all and in app code, we check somehow when to run the code which grabs subscription again and send it to the server. But this also has a downside. I think Way 1) should be the optimal solution.

Question 2)

https://stackoverflow.com/questions/61559039/how-to-get-jwt-token-in-service-worker

What do you think, Marco ?

novaknole commented 4 years ago

@collimarco could you take a look at my latest questions if you got some free time... I'm struggling a little bit to find opinions from experienced people from this..

gbhrdt commented 4 years ago

@novaknole Could you please discuss this in another channel? I'm subscribed to this thread because I'm interested in this issue.

marcoscaceres commented 2 years ago

Moving comment from #340

The purpose of PublicationSubscription's expirationTime attribute is a bit unclear in the spec. Elsewhere, @martinthomson wrote:

the spec needs is a description of what it means to set this value, specifically that the browser decides what the value is and that it indicates to sites whether the subscription will automatically end and - if it ends - when.