WICG / background-sync

A design and spec for ServiceWorker-based background synchronization
https://wicg.github.io/background-sync/spec/
Apache License 2.0
639 stars 83 forks source link

How to handle multiple syncs registered for the same tag #104

Closed jakearchibald closed 8 years ago

jakearchibald commented 8 years ago

In the current spec:

  1. User online
  2. User sends chat message, saved in IDB
  3. Sync with tag "chat" registered
  4. In SW: Sync event with tag "chat" fired
  5. In SW: Chat message retrieved from IDB
  6. In SW: Chat message starts to send to the server using fetch
  7. User sends chat message, saved in IDB
  8. Sync with tag "chat" registered. Deduped, existing registration returned
  9. In SW: Fetch completes, sync is complete, unregisters

In the above, there's a chat left in the outbox.

jakearchibald commented 8 years ago

Workaround:

If there's a firing sync with the same tag, register() waits until it's done. If the firing sync unregisters, either due to terminal failure or completion, register() resolves with a new sync registration. If the firing sync fails but will be retries, register() resolves with the existing sync registration.

This "waiting" must survive browser/tab closure.

jkarlin commented 8 years ago

If there's a firing sync with the same tag at registration we could set the firing registration's state to unregisteredWhileFiring so that it gives up if it fails. Then return a new SyncRegistration in a new state, blocked, that changes to pending as soon as the firing registration hits a final state.

jakearchibald commented 8 years ago

In that model, when would .register resolve?

jkarlin commented 8 years ago

Immediately with the new registration.

jakearchibald commented 8 years ago

We'd have to change or even drop getRegistration, since tag is no longer a primary key, but I'm ok with that.

jkarlin commented 8 years ago

Ah, I missed a step above, sorry. The firing registration's state changes to unregisteredWhileFiring and it's also removed from the list of sync registrations for the given service worker registration. Now it's still a primary key.

jakearchibald commented 8 years ago

Ohh, that sounds ideal!

mkruisselbrink commented 8 years ago

I agree that @jkarlin's suggestion makes the most sense. The only thing I'm a bit concerned about is what happens with the done promise of the earlier registration in this situation. Without special handling it could get rejected even though without the new registration the user agent might have retried and ultimately succeeded with the original sync. We could fix this oddity by always having register() replace any existing registration with the same tag. Then at least the done promise of the old registration will always fulfill (most likely by failing), and there will always be a new registration to takes its place.

jkarlin commented 8 years ago

Good point @mkruisselbrink. If we always replace then we should return an enum in the done promise so that the dev can differentiate between failure and being overwritten.

jakearchibald commented 8 years ago

I wonder if we're over complicating this from the developer's perspective. If register() is called during firing, could that registration just be given a flag to indicate it must refire, regardless of the result of the current firing?

jkarlin commented 8 years ago

Say the first registrations completes successfully but was reregistered while firing and the second time it fails. Would done then reject? That seems only partially correct.

jakearchibald commented 8 years ago

I'm really split on this. Perhaps my steps in the OP are wrong.

Is it safe to say things that use "tag" are operating some kind of "outbox" model? Eg:

From the page:

  1. Add message to outbox in idb
  2. Register sync

In the sync event:

  1. Fetch items from outbox idb
  2. If no items found, return success
  3. Post items
  4. GOTO 1

Does the above work in the current spec? Is it possible to add a message to the outbox, and not have sync attempt to send it? I guess the new item could be added between steps 1 & 2 of the sync event.

jakearchibald commented 8 years ago

Updating user profile: https://gist.github.com/jakearchibald/f61577b2feddd267726f Sending chat messages: https://gist.github.com/jakearchibald/a8826691ce3f2936d854 Fetching a page in the background: https://gist.github.com/jakearchibald/a88db50b629677c00c1f

(will post analysis shortly)

jakearchibald commented 8 years ago

Updating the user profile & sending chat messages are surprisingly similar. For the user profile example, I thought I'd end up with a single "new profile settings" entry in idb, but that quickly becomes messy if the profile is altered while a sync is in progress. An "outbox" queue works much better, where deduping happens on entry collection.

I think this outbox model will be boilerplate for any multi-message sync.

Fetching the page in the background is much simpler, as it's a sync per thing to fetch & there are no ordering issues.

Anyway, in terms of this ticket

Profile update / sending chat

If I register a sync while a sync is active, I want another sync event after the one currently processing.

I want the .done for sync 1 to resolve once the first sync succeeds/fails, I don't care about the second, as my UI reaction is in relation to what the user is doing in that tab, or relates to specific messages. If sync 1 succeeds and sync 2 fails, I expect sync 1's done to appear successful.

Fetching a page in the background

If I register a sync while a sync is active, I don't want another.

So I guess the question is, which case do we design for?

Stick with the current spec, require the user to defend against in-progress syncs

I don't think this is possible. I thought this would work:

navigator.serviceWorker.ready.then(reg => {
  reg.sync.getRegistration('foo').then(r => r && r.done).then(_ => {
    return reg.sync.register({ tag: 'foo' });
  });
});

…but this would fail to schedule a sync if the tab closes before r.done resolves.

"unregister" the current sync on another sync register

I can easily defend against this in the page-fetching example.

navigator.serviceWorker.ready.then(reg => {
  reg.sync.getRegistration('foo').then(r => r || reg.sync.register({ tag: 'foo' }));
});
jakearchibald commented 8 years ago

Ok, I'm convinced on the unregister thing. Sorry it took me so long to come round to it. Some remaining problems:

onChatSend(message => {
  showMessageAsSending(message);
  storeMessage(message).then(_ => navigator.serviceWorker.ready).then(reg => {
    return reg.sync.register({ tag: 'send-chat' }).finish;
  }).then(_ => {
    showMessageAsSent(message);
  }).catch(_ => {
    showMessageAsFailed(message);
  });
});

onChatSend(message1);
// then later, while message1's sync is firing…
onChatSend(message2);

Between the registering of message2's sync, but before message1's sync completes, the user loses connectivity, what happens to message1's .finished?

Option 1: Reject .finished, but somehow indicate that there's another registration with the same tag.

I think this is a bit of a footgun, as .finished.catch feels like perma-failure. In chat-message terms it would be "Failed to send - [retry]". This behaviour changes that, it may not have completely failed, in fact, it may be in progress.

Option 2: Resolve .finished with the .finished of the new registration. This means message1's .finished would resolve at the same time and with the same value as message2's.

This would make the code above work as intended. But is it weird having .finish on one registration depend on another? Although the two registrations are already linked, as message2's sync will not fire until message1 has finished firing.

Option 3: Drop .finished from the spec.

The means the user would be back to relying on postMessage to communicate success/failure.

I'm in favour of option 2. The more I think about it, the less I like option 1, it makes .finished really tough to use.

mkruisselbrink commented 8 years ago

So if I'm understanding it correctly, you're proposing that registering for an existing tag:

I think that makes a lot of sense, and is probably the least unexpected behavior, so sgtm.

jakearchibald commented 8 years ago

I was thinking of only doing the unregister thing if the other sync was mid-firing, but now you mention it, they way you describe it is functionally identical and less special-casey.

Is it less efficient though? Having to create a new registration when you could return the current one (provided it hasn't started firing).

On Thu, 22 Oct 2015, 19:21 Marijn Kruisselbrink notifications@github.com wrote:

So if I'm understanding it correctly, you're proposing that registering for an existing tag:

  • always returns a new SyncRegistration/unregisters the existing registration
  • if the original registration succeeds, its done promise resolves as success
  • if the original registration failed, its done promise resolves/rejects based on the new registration

I think that makes a lot of sense, and is probably the least unexpected behavior, so sgtm.

— Reply to this email directly or view it on GitHub https://github.com/slightlyoff/BackgroundSync/issues/104#issuecomment-150311980 .

jakearchibald commented 8 years ago

Just to confirm, your bullet point summary is what I was thinking. If message1's sync succeeds it can resolve straight away, no dependency on message2.

jakearchibald commented 8 years ago

Thought for the day: .data would have only been useful for the "Load page in the background" example, and even then I'd likely have the url in the tag name, so still kinda redundant.

jkarlin commented 8 years ago

always returns a new SyncRegistration/unregisters the existing registration if the original registration succeeds, its done promise resolves as success if the original registration failed, its done promise resolves/rejects based on the new registration

I find the split on success/failure to be strange and confusing. How about we just unregister the original registration and pass the finished observers to the new registration regardless.

jkarlin commented 8 years ago

In fact, I think Jake's comment way above of:

I wonder if we're over complicating this from the developer's perspective. If register() is called during firing, could that registration just be given a flag to indicate it must refire, regardless of the result of the current firing?

Is probably the right approach.

mkruisselbrink commented 8 years ago

I don't know... Always having the finished promise of the old registration refer to the result of the new registration seems counter-intuitive if the original registration fails... My preference would be either what jake suggests, or alternatively always unregister and leave the finished promise of the original registration alone.

Separately I wonder if it makes sense to distinguish between the three different final states (unregistered, success, failure) in the finished promise. Currently the spec reports success by resolving the promise, and both other states are treated by rejecting the promise with an AbortError, but it might make more sense to treat unregistered and failed differently. Not sure if unregistered should reject or resolve, and if reject which error would be most appropriate for either case...

mkruisselbrink commented 8 years ago

But yeah, maybe the simple solution would indeed be best... If not getting the success result from the original registration if it succeeded is okay then that's definitely the easiest thing to do.

martinthomson commented 8 years ago

I think that if you are collapsing two sync requests based on tag and that can happen before an event is delivered to the SW, then having the .finished resolve when both are done makes perfect sense. However, if you are forced to deliver two events into the SW, what is inherently wrong with resolving the first after the first sync event completes? and... Are you overconstraining the problem by insisting on having a .finished attribute?

I think that you have to assume that the first sync event handler is only able to "sync" whatever state has been propagated from the page to the SW ahead of the sync event. Anything that might have triggered the second sync event might not be required to have propagated to the SW by the time that the first sync event fires.

By the same token, the actions that the first sync request triggers will be fulfilled when that event completes, so awaiting the completion of the second sync event seems like it isn't really needed.

If you are awaiting processing of a sync event in the SW, would you delay delivery of further sync events for that tag? I think that that would be a valuable constraint on this.

Have you considered that perhaps registration of a SyncManager and the triggering of a sync event might be worth splitting? A Promise<void> sync() method on SyncManager would allow you to identify different attempts to sync in the case that you are forced to enqueue a new sync event.

In the event that requests can be collapsed into a single sync event successfully, then you can resolve all the promises together. If they have to be staggered (and you never need more than two if you serialize delivery of events for a given tag), then you resolve each as its corresponding sync event completes.

jakearchibald commented 8 years ago

@jkarlin

In fact, I think Jake's comment … is the right approach

I think you showed a good reason why my proposal wasn't good enough https://github.com/slightlyoff/BackgroundSync/issues/104#issuecomment-149545617

If message1's sync succeeds, but message2's sync fails, it would be weird for message1's .finished to be reject (or even be delayed).

Using the code for the profile update, or the chat messages, the UI will show messages failed when they actually sent (if the follow-on sync fails), or will be delayed in showing that messages sent (if the follow-on sync is delayed).

jakearchibald commented 8 years ago

@martinthomson

Are you overconstraining the problem by insisting on having a .finished attribute?

This is my "Option 3" (https://github.com/slightlyoff/BackgroundSync/issues/104#issuecomment-150308031). The chat and profile-update sketch could be done using postMessage or an idb observer, when we have those. .finished is a really neat way of doing it, but if it comes with gotchas it might not be worth it.

I'm happy with "Option 2", which is resolve-on-success, but resolve failures with the .finished of the new sync. But it's complexity makes me wonder if there are gotcha cases we haven't thought about.

If you are awaiting processing of a sync event in the SW, would you delay delivery of further sync events for that tag?

Yep, this is a must. It's really confusing otherwise. Thankfully @jkarlin covered this off earlier in the thread https://github.com/slightlyoff/BackgroundSync/issues/104#issuecomment-147435981. Syncs of differing tags are fine to overlap though.

Have you considered that perhaps registration of a SyncManager and the triggering of a sync event might be worth splitting?

I can't find the issue, but this was considered. Since syncs can be delayed because of connectivity, it's a little confusing what .sync() would do. Would it trigger the event even without connectivity? Would it never fire if it isn't called? What if .sync() is called multiple times while other syncs are ongoing? From this, we decided a sync should fire automatically, but .finished can track progress.

If .finished becomes a real sticking point, we could move it out of MVP & reintroduce it based on observed usage.

martinthomson commented 8 years ago

@jakearchibald

My point about .finished was more that it was an attribute, not that it existed. I think that the idea of being able to directly get an response from the SyncManager is very appealing, but the attribute is a big constraint due to the disconnect between the action of asking for sync and the requesting of the promise by accessing .finished.

Would it trigger the event even without connectivity? Would it never fire if it isn't called? What if .sync() is called multiple times while other syncs are ongoing?

I think that .sync() would just request that sync be initiated at the next available time. That means the usual prerequisites would need to be met, nothing more.

  1. If there is another sync planned (but not started), then it will effectively do nothing, just return a promise that is bound to the existing sync.
  2. If there is another sync, but it is running, then it enqueues another sync to run after the the running sync completes.
  3. If there is already a sync awaiting the completion of a running sync, then it joins to that.
  4. If there is no other sync and one can be started, then it starts one.
  5. If there is no other sync, but the sync can't start just yet, it waits until a sync can be started.

I don't know what you mean about never firing here. In the design I suggest you have to call .sync() to get a sync event delivered.

jakearchibald commented 8 years ago

So you have to register and call sync? Is there a reason I would want to register a sync but not start it? Do I have to manually unregister the sync when it's done? Are there any retries in this model? (sorry for all the questions, just trying to get my head around it)

martinthomson commented 8 years ago

Well, I didn't want to overwhelm you with options, but the more that I think on this, the more that I think that an instantaneous sync is best served by a simple .sync(DOMString tag) method, with no registrations at all.

Al the registration machinery serves little actual purpose here. You have getters and unregister methods, but all that does is establish a bunch of state in the browser. The only state you actually need lives between the time when the need for sync is identified and when it can be fulfilled.

(Caveat: I haven't slept in a long time, and I'm new to this API, I might have missed something.)

jakearchibald commented 8 years ago

Of the three use-case sketches I did (https://github.com/slightlyoff/BackgroundSync/issues/104#issuecomment-150184998) the "fetching page in background" one would benefit from getting a registration. If there's a sync in-flight I want to get its .finished rather than queuing another.

martinthomson commented 8 years ago

@jakearchibald, I'm not sure that I see why this is necessary. If there is no new information to exchange, the SW can identify that and quickly resolve the promise as a noop.

jkarlin commented 8 years ago

How about this:

.finished informs the page that the browser will no longer attempt to sync tag. Upon attempt to register an existing tag that is mid-fire, the browser should return the existing registration and change the registration's state to pending after it completes firing the sync event, regardless of outcome. The browser shouldn't call .finished until the registration is removed.

.finished does not convey the result of a sync event very well. If it resolves, it's unclear which outbox messages were sent as more could have been added before the event fired. If it rejects, which (if any) messages from the outbox successfully sent before it failed? This makes me think that .finished should always resolve, leaving the page to determine what happened via better means such as storage, postMessage, or we could add a data parameter to .finished's promise.

In terms of whether or not we should have unregister and getRegistration functions, keep in mind that a noop is an expensive operation in the world of sync (you potentially have to start the browser, then the service worker, just to do nothing). That said, I'm all for removing unnecessary API.

Of the three use-case sketches I did (#104 (comment)) the "fetching page in background" one would benefit from getting a registration. If there's a sync in-flight I want to get its .finished rather than queuing another.

Given my sketch of how .finished works above, you could simply register the same tag to get a pointer to the original registration and use its .finished.

martinthomson commented 8 years ago

@jkarlin,

[...] you potentially have to start the browser, then the service worker, just to do nothing [...]

The sync event is only enqueued in this fashion if there is another one running. So the noop would occur immediately after processing the last, while the service worker is still running. Even if the last sync caused the service worker to be replaced, the browser is still actively working.

.finished does not convey the result of a sync event very well. If it resolves, it's unclear which outbox messages were sent as more could have been added before the event fired. If it rejects, which (if any) messages from the outbox successfully sent before it failed? This makes me think that .finished should always resolve, leaving the page to determine what happened via better means such as storage, postMessage, or we could add a data parameter to .finished's promise.

I'm less sure about this. I see no real problem with returning the result of the promise passed to .waitUntil. It's convenient, though I'm not sure that it's entirely possible to propagate everything (non-Transferable objects, for certain).

I agree that if sync is incomplete, then you do have a problem. But that only happens if either a) the code is buggy, b) the data that required sync was not properly in place before the sync started, which a subsequent sync call should rectify, or c) when the sync fails. None of those states leaps out as an actual problem with the API, just the implementation.

The problem with .finished is that it is bound to an object - the registration - that has a sort of semi-permanence. But that permanence isn't really justified by the use cases.

I think that the following API is sufficient:

interface SyncManager {
  Promise<void> sync(optional DOMString tag);
  Promise<void> cancelSync(DOMString tag);
};

This isn't as capable in the sense that an anonymous sync request cannot be cancelled, but it's not clear that the current API is capable of supporting that properly anyhow. If cancellation is important, then perhaps requiring the tag argument to sync() is the right answer. If the sync truly is independent, then the app can provide a UUID or otherwise unique tag to each sync call.

jkarlin commented 8 years ago

The problem with .finished is that it is bound to an object - the registration - that has a sort of semi-permanence. But that permanence isn't really justified by the use cases.

Agree. I'd be interested in removing SyncRegistration

I think that the following API is sufficient:

interface SyncManager {
  Promise<void> sync(optional DOMString tag);
  Promise<void> cancelSync(DOMString tag);
};

This isn't as capable in the sense that an anonymous sync request cannot be cancelled, but it's not clear that the current API is capable of supporting that properly anyhow.

I'm not sure what the use of anonymous syncs would be. Setting a default tag of "" seems sufficient. A dictionary for options on sync would be useful in case we want to add more options down the road. We could sneak the functionality of .finished in here by having sync's promise resolve once the sync is finished instead of once it has registered.

martinthomson commented 8 years ago

@jkarlin, I was imagining that sync would resolve after the waitUntil() promise resolved. Regarding anonymous, I'm happy to default to ""; that's certainly the simplest option. Adding options would be trivial as an extra argument, but I think that tag is fundamental enough to justify a bare argument.

jkarlin commented 8 years ago

I think getSyncTags (a getRegistrations equivalent) is still necessary for debugging if nothing else.

martinthomson commented 8 years ago

Sure, noting that you will only be able to see outstanding sync requests.

jakearchibald commented 8 years ago

@martinthomson

The sync event is only enqueued in this fashion if there is another one running. So the noop would occur immediately after processing the last, while the service worker is still running.

Unless connectivity has dropped, then the browser will spin up some time in the future. It's an edge case. Developers will also have to check to see if the task has already completed before attempting. Eg this example doesn't https://gist.github.com/jakearchibald/a88db50b629677c00c1f

I see no real problem with returning the result of the promise passed to .waitUntil

I thought about this, but what would you do if the clone operation fails? Rejecting in this case seems really misleading. Also, on rejection, you couldn't pass the error through as errors aren't clonable, so it's a little asymmetrical.

I'll rewrite the example using the proposed API…

jakearchibald commented 8 years ago

…the sync/cancel API seems to work (not much difference). Although we'd need to do better than swReg.sync.sync('hello').

swReg.sync.sync('hello'); // sync1
// then later…
swReg.sync.sync('hello'); // sync2

To sum up:

…I'm pretty happy with that, but the remaining bit is what happens with sync1's promise in the last example.

If a sync can complete partially (eg, an outbox that's sent message by message), then postMessage can be used to communicate progress.

jakearchibald commented 8 years ago

@martinthomson

you will only be able to see outstanding sync requests [from getSyncTags()]

I assume that currently firing syncs will be in there.

jakearchibald commented 8 years ago

Note: Highly unlikely that promise will ever reject. If there's a page open, there's little reason to terminally fail.

jkarlin commented 8 years ago

@martinthomson Let's move the reduced API discussion to a new issue.

@jakearchibald The outbox approach will have to deal with figuring out what successfully sent on .finished rejection, regardless of the semantics of registering mid-fire.

I think we should keep .finished simple and consider registration of a mid-fire sync as an extension of the tag's lifetime and not resolve/reject .finished until the browser gives up on the tag. @martinthomson 's reduced API makes it easier to think in terms of tag lifetime instead of registration lifetime.

mkruisselbrink commented 8 years ago

@jkarlin what we realized today when discussing this more in person is that with the reduced API (where the only way to get the finished promise is to register a sync), there really is no reason for the promise to ever reject. So swReg.sync.sync('tag') returns a promise, that promise resolve as soon as the associated sync registration succeeds. If the sync registration fails the promise just never resolved. This works because as @jakearchibald points out if there still is a page open to observe the promise rejecting there is no reason to not retry the sync more, so it own't actually fail.

So the simplified API, and the returned promise never rejecting pretty much addresses all issues.

jkarlin commented 8 years ago

No reason to let the sync fail while the page is open? Perhaps. What if the resource that the sync is trying to fetch is constantly 404ing? It seems like a limited number of retries even when the page is open is a good idea.

mkruisselbrink commented 8 years ago

Okay, presumably if a page is open for a really long time, we might eventually stop retrying. I'm still not convinced that this kind of failure needs to be reflected in the returned promise though. In the simplified API (which maybe indeed needs a separate issue # for name bikeshedding etc) navigator.sync.sync() returns a promise; that promise is 1) rejected if registering for the sync somehow fails (no sync attempts will ever be made), or 2) resolved if the sync succeeds. I'm not sure also rejecting for the third case, where registration succeeded, but sync eventually fails is really the best thing to do. I don't feel particularly strongly about this though, so if that's the desired behavior, I'd be happy to keep it.

jakearchibald commented 8 years ago

@jkarlin

The outbox approach will have to deal with figuring out what successfully sent on .finished rejection, regardless of the semantics of registering mid-fire.

In the chat case, .finished is fine as it's a bulk send that either works or doesn't. But if it had message-by-message granularity, .finished isn't sufficient, and postMessage would be needed (or consult IDB for outbox status on .finished).

I think we should keep .finished simple and consider registration of a mid-fire sync as an extension of the tag's lifetime and not resolve/reject .finished until the browser gives up on the tag.

You convinced me otherwise back in https://github.com/slightlyoff/BackgroundSync/issues/104#issuecomment-149545617

The simpler solution means .finished resolves later than it should, or rejects when it shouldn't.

I don't think what's being proposed here is too complicated. Conceptually, it's:

sync.finished = waitForSyncOfSameTagToFinishFiring()
  .then(_ => addSelfToPendingQueue())
  .then(function doSync() {
    return waitForOnline().then(_ => {
      // this returns the result of waitUntil()
      return callSyncHandlers();
    }).then(_ => {
      removeSelfFromPendingQueue();
    }).catch(_ => {
      if (newSyncOfSameTagWaiting) {
        removeSelfFromPendingQueue();
        return syncOfSameTag.finished;
      }

      if (shouldTerminallyFail) {
        removeSelfFromPendingQueue();
        throw Error("Game over pal");
      } 

      return waitSomeTime().then(doSync);
    });
  });
jakearchibald commented 8 years ago

Thinking about the use-cases:

The above makes we doubt the compact API. True, a terminal sync failure is unlikely with a page open, but this api is available in things that aren't pages. There isn't much of a use-case outside of pages, granted, but it doesn't feel safe assuming that forever.

A beacon is a good example of this. As is a preference set. Although there may be many preference changes, it might be dangerous to apply them non-atomically.

A chat send can be an example of this. The messages are sent in bulk in one request, but the response may indicate one of the messages was invalid for some reason. This is a successful sync. The server has indicated terminal failure of one of the items, so there's no point in a retry. This should just work, as long as I don't throw on discovery of the "some stuff wasn't accepted" message.

When .finished settles I'll consult the outbox for status and update the UI.

This could be some kind of mass messaging system that delivers to multiple servers. It should be a unique sync per server, then .finished is useful for reporting individual success.

This could be a chat system that delivers each chat as a separate request. I could wait until .finished to update the UI, but that would mean I'm showing things as unsent that are sent.

jakearchibald commented 8 years ago

Considering the use-cases above, if a mid-fire sync simply extends the lifetime of the sync, it's effectively turning both "sync can only succeed or fail" and "sync has a single request, but can result in a partial success", into "sync involves multiple requests that can be delivered in a single order & partial success is interesting".

Not only is it unclear that this switch happens, it makes things more complicated. Now I can have a sync that is pending or even failed, but has actually partially succeeded. .finished is at best uninteresting, and at worst misleading.

I think we either need to go for the more complex model (https://github.com/slightlyoff/BackgroundSync/issues/104#issuecomment-152048959), or the simpler model but drop .finished.

jkarlin commented 8 years ago

Let's just drop .finished. It's a nice-to-have and we can add it back in a future version if it's needed and we've figured out an elegant solution to this.

jkarlin commented 8 years ago

With .finished out of the way, what happens on registration while mid-fire becomes simpler. We just need to reset the registration's retry counter and resolve with the existing registration.