SharedServiceWorker needs an error event/interface

[slightlyoff]

We need a way to communicate to pages that service workers may have failed to start on subsequent boot.

[jakearchibald]

onserviceworkererror good enough?

Feels like we need a namespace on navigator, would simplify the method/event names too.

[alecf]

This came out of a discussion with @michael-nordman about error handling. I was also thinking navigator.serviceWorker.onerror would work.

The only trick with that is it requires a serviceWorker to be present.

I think that's ok though: the only error case that @michael-nordman and I could come up with right now was the case mentioned above (i.e. the case where the service worker has in fact been registered, but fails to start) in which case navigator.serviceWorker would exist.

[jakearchibald]

Feels weird having the events in different places, but…

Is navigator.serviceWorker a performance problem, being sync n'all?

It's not always decided on page load, a page could have no serviceWorker, then one is registered, if it replaces, navigator.serviceWorker needs to change from null to object.

If this is a problem I'll draft up another api where navigator.serviceWorker is always present, move all the events and methods in there, and add an async method to get status.

[alecf]

As it stands today, navigator.serviceWorker looks sync, but under the covers, it doesn't need to be sync at all:

1. When a page loads, it knows whether or not it is "controlled" so it knows if serviceWorker is present, and what state it is in
2. The state of the service worker does not change for the lifetime of the document
3. The only methods on service worker are async, such as navigator.serviceWorker.postMessage()

This means that the frontend only has to have a local object that is initialized at pageload time, that really only exists so as to be visible to the DOM. It doesn't need to reach into any storage or anything during access to the DOM object 'navigator.serviceWorker'

The only time this will really be a problem is if we add synchronous attributes to this object, that can't be inferred during pageload time. I don't think we have any plans to do that.

[alecf]

oh and since onerror is asynchronous (being an event) then it is ok too. The frontend implementation of

navigator.serviceWorker.onerror = function(...) { ...}

merely has to store a reference to that function in that frontend DOM object, it doesn't require being stored anywhere.

[annevk]

The service worker can decide to take over during the lifetime of the document. Or did we kill that feature?

[alecf]

@annevk As I understand it there has never been a way to take over in the middle of a document's existence - the document is either controlled from the start, or not at all.

[annevk]

@alecf https://github.com/slightlyoff/ServiceWorker/blob/master/service_worker.ts#L98 (InstallEvent.prototype.replace)

[michael-nordman]

@annevk,the replace method can advance the version for already "controlled" document during the lifetime of a document. The navigator.serviceWorker is non null before and after that transition.

[jakearchibald]

Really? So what happens if you call replace() oninstall of the first worker, where there isn't another present? I thought it took over.

[alecf]

Maybe we need to be clear that the 'onerror' is happening in the document. replace() is something that happens in the service worker.

Looking at replace, I'm not really sure how to interpret the comment.

// Ensures that the worker is used in place of existing workers for
// the currently controlled set of window instances.

if a page is not currently controlled, then replace() isn't affecting that window, right?

But I think we're veering off the original topic here - this bug is about an error event in a document when the service worker fails to start.

consider a service worker:

if (Math.random() > .9) {
   foo.bar = baz;
}

9 times out of 10 this will start fine, so it will probably install just fine. 1 time out of ten, this script will fail to run. Imagine it's successfully installed but fails to run 5 minutes later. the page needs to be able to capture that if it cares:

<head>
<script>
if (navigator.serviceWorker) {
  navigator.serviceWorker.onerror = function(e) {
    alert("service worker failed to start. good luck with that!");
  }
</script>
</head>

or possibly just navigator.onserviceworkererror

[michael-nordman]

There are some other events in the works too, surfacing this one as 'navigator.onserviceworkererror' looks most in keeping with the other event handlers i've seen mentioned in the .ts file.

[jakearchibald]

@michael-nordman agreed. I'll open another issue for the replace() thing. If navigator.serviceWorker becomes never-null, we'll move all the events in there, which is great because we can drop 'serviceworker' from all their names.

[annevk]

So with the new API this is the ServiceWorker object. It has onerror. Is that what we need?

[jakearchibald]

Dedicated workers currently throw errors like this:

1. If uncaught, call onerror in the worker scope (which behaves like window.onerror)
2. If the handler does not return truthy, fire error event on the worker object
3. If the event is not default-prevented, act as if the error was thrown in the global scope of the worker object (eg, the window, which would call window.onerror)

We can do 1 easily, and should. However, this doesn't catch syntax errors.

We can also do 2, but that won't catch syntax errors either.

We could fire an error event at navigator.serviceWorker at this point (this is in the spec, but without detail). That would catch some syntax errors, but only if there's an active tab with a listener.

We could also bubble up to window.onerror.

[jakearchibald]

Thinking about the use-cases (following from #198)…

navigator.serviceWorker.onerror or navigator.serviceWorker.pending.onerror (whichever it becomes) are not useful for logging errors back to the server, as errors can happen outside of the life of any page. onerror inside the worker itself is best for that.

.pending.onerror is useful if you're updating the UI in response to an update. So maybe it's better as a statechange, although you'd need somewhere to put the error message.

That leaves errors that happen before the SW instance is created. AppCache has an error event that covers network-related update failures, and also parse failures. However, once again we'd lose any errors that happened outside the life of a page.

[jyasskin]

It may be important to tell a server that its new SW script has a parse error. Could we fire the error event on the next page load? Or maybe do something like CSP's report-uri?

[jakearchibald]

…or navigator.serviceWorker.updateError which is either null or an error for the last update.

However, update errors are part of the norm. If you're offline or in a captive portal, those are update failures. I guess parse errors could be treated differently.

[KenjiBaheux]

These feels like enhancements, let me know if something really ought to be offered as part of a v1/minimal viable product.

[jakearchibald]

Happy to treat as enhancements. We should probably have a call to sort this out.

[KenjiBaheux]

It has been pointed that we need a common understanding of how to use labels and milestones. I'll work on a draft to suggest how we should use the labels and milestones.

Let me make some adjustments based on how I believe we should use these:

enhancement: anything that was assessed as not having any impact on milestone 1 decisions and can therefore be safely discussed, rejected or prioritized later.

milestone: to mark items we agreed to get done in principle by a given revision. I would argue that we should only focus on the current milestone and leave the rest without specific milestones.