Should OS failures setting the badge reject the promise?

[mgiuca]

Raised by @marcoscaceres on #57. I could go either way on this.

The reason it's currently specified as "fire and forget" and always resolve the promise is that the API is a "best effort". Especially for the app version of the API, you could store the badge state locally, then try to set the badge in 3 places in the OS, some of which succeed and others of which fail. There's no reasonable way to communicate the partial success back to the user.

So it's currently specified as "always report success to the user, even if it failed at the OS level". We could change this.

[marcoscaceres]

So, although we probably couldn't do much about multiple windows competing to set the badge (the developer would need to centralize that in a worker), we could may limit the chance of confusion by only allowing one call to navigator.set*Badge() at a time? That is, the promise must first resolve before you can call the method again.

To be clear:

const p1 = navigator.setAppBadge(1);
// rejects, p1 already running "in parallel"
const p2 = navigator.setAppBadge(2);
// Settle... 
await p1;
// ok to call method again... 
const p3 = navigator.setAppBadge(3);

[fallaciousreasoning]

I think this that's probably overkill to be honest:

1. If multiple windows are trying to set a badge at roughly the same time, it doesn't seem ridiculous that the most recent one is taken.
2. I suspect it will be non-trivial to synchronize "A badge is being applied" across processes in Chrome.
3. Setting a Badge is (at least in my experience) fairly quick, so it seems unlikely to come up that much (and when it does, it seems like it will have been triggered by a server event, so they'll be setting the same value).

In addition, I don't think we want to encourage either of the following two patterns:

// For when I don't care if the badge actually gets set:
await navigator.setAppBadge(5).catch();

// This is probably what I want 99% of the time, so it would be nice if I didn't
// have to do the catch every time I want to set the badge, just in case
// someone else is setting the badge.

// For when I really want the badge from my window to be the One True Badge:
async function eventuallySetAppBadge(...args) {
    try {
        await navigator.setAppBadge(...args);
    } catch (err) {
        if (isBecauseSomeoneElseWasBadging(err))
            return eventuallySetAppBadge(...args);
   }
}

// instead of doing things this way, I should be going through the service worker.

[fallaciousreasoning]

With regard to Matt's original question, I don't think we should try and report failure at an OS level.

• What would we do on OS's that don't support badging? Always report failure?
• What if we have to badge multiple places and one fails? Do we reject and roll back the badge we set? What if the rollback fails?

I'm pretty happy with this just being fire and forget, but I could be convinced to resolve the promise only once we've told the OS to update. Do we have any arguments for why it would be better?

I suspect even if we make the wrong choice here we can just change this later, it seems like an implementation detail to me.

[marcoscaceres]

I'm pretty happy with this just being fire and forget, but I could be convinced to resolve the promise only once we've told the OS to update.

The point of making these methods return a promise was to give us breathing room to handle unforeseen (future/error) consequences. We will only really hit those after this has been in the wild for some time and we get more experience implementing... or possibly as we make extensions to the API in the future.

Fingers crossed we never hit any errors (and, say, Chrome can just return Promise.resolve(); and do it's IPC work - but that's an implementation detail). The spec tho, should be designed to deal with unforeseen errors, in case we need it.

Similarly with what I am proposing in #61 (comment)

We don't need to take action on that right now. We can see how things play out.

Do we have any arguments for why it would be better?

As above... I don't think we need to make a decision on this issue just yet - let's get some implementation experience and let people play with the API. Because of how we are specifying things in #57 (i.e., return the promise, run in parallel), we can update the spec to reject the returned promise if we need to at a later point.

It will be also interesting to see how app developers deal with the synchronization issue... One could use a promise queue library probably.

[fallaciousreasoning]

The point of making these methods return a promise was to give us breathing room to handle unforeseen (future/error) consequences. We will only really hit those after this has been in the wild for some time and we get more experience implementing... or possibly as we make extensions to the API in the future.

Sorry, I'm still completely on board with returning a promise, I (mis)used fire and forget to mean we shouldn't be dependent on what the OS does and whether it supports badging.

Fingers crossed we never hit any errors (and, say, Chrome can just return Promise.resolve(); and do it's IPC work - but that's an implementation detail). The spec tho, should be designed to deal with unforeseen errors, in case we need it.

Okay, I think I misunderstood the discussion here. So you're saying that we should write the spec as if the promise would only resolve once the badge had actually been set in the operating system (in case we encounter some error conditions in the process of setting a badge). However, because we have no error conditions yet, it's fine to not implement it that way? I think I can get on board.

I don't have much (any) experience on specs, so it's possible I've still got the wrong end of the stick...

Similarly with what I am proposing in #61 (comment)

So you're not proposing that we actually implement this, but illustrating the kind of thing we might need in the future?

As above... I don't think we need to make a decision on this issue just yet - let's get some implementation experience and let people play with the API. Because of how we are specifying things in #57 (i.e., return the promise, run in parallel), we can update the spec to reject the returned promise if we need to at a later point.

All right, I think we're in agreement :)

It will be also interesting to see how app developers deal with the synchronization issue... One could use a promise queue library probably.

Indeed, I'm curious to see how often developers bump into this, probably good to leave things open for fixing this.

[marcoscaceres]

So you're saying that we should write the spec as if the promise would only resolve once the badge had actually been set in the operating system (in case we encounter some error conditions in the process of setting a badge). However, because we have no error conditions yet, it's fine to not implement it that way? I think I can get on board.

Yes, exactly :)

I don't have much (any) experience on specs, so it's possible I've still got the wrong end of the stick...

Not at all. All the points/questions you raised were extremely valid. Just in my experience, we only really start hitting these things once we have multiple implementation or we writing tests - so just preemptive stuff from experience.

So you're not proposing that we actually implement this, but illustrating the kind of thing we might need in the future?

We could make an earlier call on #61 (comment) as it helps prevent flooding the IPC channel, for instance. But I don't have a super strong opinion at this point.

[fallaciousreasoning]

Awesome :)

We could make an earlier call on #61 (comment) as it helps prevent flooding the IPC channel, for instance. But I don't have a super strong opinion at this point.

To avoid saturating the IPC channel we could do something like what we discuss here: #35 (comment)

Essentially: Don't try to set a badge until you've finished trying to set the last value. When the last badge has been set, see if we have a new value. If so, set the badge to the new value. Repeat as needed.

We could go further, and throttle the rate at which we change the badge (kudos to @mgiuca for the idea), but I don't think we need to throw an exception.

[marcoscaceres]

Essentially: Don't try to set a badge until you've

Depends on who "you" is. If "you" is the web developer, then 👍 More below...

finished trying to set the last value. When the last badge has been set, see if we have a new value. If so, set the badge to the new value. Repeat as needed.

Basically, yes... if "you" is "Marcos", then: I'm trying to be lazy tho and want to avoid implementing the promise/badge-setting-queue in c++ 😜. That removes a lot complexity from the implementation and moves it to JS to deal with.

[fallaciousreasoning]

Depends on who "you" is

I meant "you" as in someone working on WebAPIs.

Basically, yes... if "you" is "Marcos", then: I'm trying to be lazy tho and want to avoid implementing the promise/badge-setting-queue in c++ stuck_out_tongue_winking_eye. That removes a lot complexity from the implementation and moves it to JS to deal with.

I think it'd be a little bit sad if we forced web developers to deal with this (it'd make the API very unpleasant to use and it would essentially force developers to go through the service worker, as it's the only place they can synchronize across tabs).

In addition, I'm not convinced it'll make the C++ work any less complicated (at least, in Chromium). Each renderer could be in a separate process in Chromium, so we'd have to do some kind of synchronization to ensure you can't call setAppBadge until a previous setAppBadge has completed (which sounds a bit scarier to me than the queuing).

I suspect this is also a case of premature optimization: We don't even know if this will be a problem and, if it becomes one, I think there's space for vendors to implement something without diverging from the spec.

[fallaciousreasoning]

Are we broadly in agreement that the OS failing to set a badge shouldn't cause the promise to reject? If so, I'll close this issue.

[marcoscaceres]

Are we broadly in agreement that the OS failing to set a badge shouldn't cause the promise to reject?

Yes.

[marcoscaceres]

I think it'd be a little bit sad if we forced web developers to deal with this (it'd make the API very unpleasant to use and it would essentially force developers to go through the service worker, as it's the only place they can synchronize across tabs).

I don't mind this so much. Like I said, it makes the implementation less complicated, which means it's easier to implement, maintain, and audit for security purposes.

In addition, I'm not convinced it'll make the C++ work any less complicated (at least, in Chromium). Each renderer could be in a separate process in Chromium, so we'd have to do some kind of synchronization to ensure you can't call setAppBadge until a previous setAppBadge has completed (which sounds a bit scarier to me than the queuing).

I think I'd defer the above to the OS - but again, it's something I need to experiment with... implementation details :)

I suspect this is also a case of premature optimization: We don't even know if this will be a problem and, if it becomes one, I think there's space for vendors to implement something without diverging from the spec.

Yes, I agree. It's bit of a preemptive measure on my part. However, I'd like to see the spec reflect that we are still experimenting here and that developers should potentially expect a rejection if this ships experimentally in a browser - the promise not rejecting is not guaranteed.

[mgiuca]

It looks like the consensus here is that we won't spec any specific errors (yet), but we should write the spec in such a way that the promise is not resolved immediately, rather, it gets resolved after the user agent attempts to set the badge, which gives us the leeway to insert errors later, and means developers can't assume the promise will resolve if all the parameters are correct.

I can update the spec, but I really want to land my mega-PR (#57) first before making further structural changes.

[marcoscaceres]

I can update the spec, but I really want to land my mega-PR (#57) first before making further structural changes.

Maybe we can split #57 one up into smaller pieces? I'd like for us to land that together with the web platform tests. I'd be happy to help with that and/or the tests if needed.

[mgiuca]

Maybe we can split #57 one up into smaller pieces? I'd like for us to land that together with the web platform tests. I'd be happy to help with that and/or the tests if needed.

Well, let's discuss that on #57. This would be a separate piece regardless.

[marcoscaceres]

I'm closing this one, as it's overly broad. However, we did find some cases in WebKit's implementation, though they can be handled in the main thread.