PushRegisterMessage => PushError

[efullea]

PushRegisterMessage => PushError with 'reason' attribute (enumeration). Now it covers at least 2 type of errors:
1- new registration is needed (e.g. due to fatal error at server),
2- "out of sync" , informational and does not imply registration has ended

[mvano]

Can we phrase this so that it inherits from DOMException? The reason afaik should be a string. The type could be RegistrationMissingError or OutOfSyncError. I'm not sure what the formal pattern looks like exactly.

See: http://www.w3.org/TR/WebIDL/#idl-exceptions

[mvano]

I think the enum approach would look as follows in WebIDL:

[Constructor(DOMString type, optional PushErrorEventInit eventInitDict), Exposed=ServiceWorker]
interface PushErrorEvent : Event {
  readonly attribute PushErrorReason reason;
};

dictionary PushErrorEventInit : EventInit {
  PushErrorReason reason;
};

enum PushErrorReason {
  "registration-missing",
  "out-of-sync"
};

So the javascript would then look like:

this.onerror = function(event) {
  switch(event.reason) {
    case "registration-missing":
     ...
    break;
    case "out-of-sync":
     ...
    break;
  }
};

[mvano]

The enum approach has a drawback: it is basically reinventing DOMException by replacing name or code with reason. I think we can reuse DOMException by using an instance of it for PushErrorEvent:

[Constructor(DOMString type, optional PushErrorEventInit eventInitDict), Exposed=ServiceWorker]
interface PushErrorEvent : Event {
  readonly attribute DOMException error;
};

dictionary PushErrorEventInit : EventInit {
  DOMException error;
};

We can then add PushOutOfSync and PushRegistrationLost to the table of error names - http://www.w3.org/TR/dom/#error-names-0

So the javascript would then look like:

this.onerror = function(event) {
  switch(event.error.name) {
    case "PushRegistrationLost":
     ...
    break;
    case "PushOutOfSync":
     ...
    break;
  }
};

I think the DOMException approach has two drawbacks of its own:

• The event.error.name syntax is verbose and a bit weird
• It requires adding very Push-specific error names

[annevk]

Maybe this can reuse http://www.whatwg.org/specs/web-apps/current-work/#the-errorevent-interface in some way?

[annevk]

Also adding @domenic and @heycam. They might be able to help out API-wise.

[mvano]

Isn't ErrorEvent for parse and runtime errors? Other than that I see two problems:

• There's a bunch of fields not relevant to Push e.g. for "PushRegistrationLost" we cannot populate line and column numbers in a meaningful way
• The switch block would have to look at the message property, which is meant for developers, not for basing logic on

[mounirlamouri]

I think it would be nice to have a DOMErrorEvent (or DOMExceptionEvent) because PushErrorEvent looks generic enough.

[annevk]

@mvano I would expect ErrorEvent.error to be a DOMException. We could introduce yet another interface that means the same thing, but I'm not sure what the point would be. Having some fields initialized to their initial values seems fine.

[mvano]

@annevk So the switch would be on event.error.name? I guess that's ok because we'd not be reinventing the wheel too much. Just some new error names.

[mounirlamouri]

@annevk, ErrorEvent seems to be for js runtime errors with the line/column numbers. Why re-use that. Though, we could make this inherit from a base class.

[domenic]

ErrorEvent seems fine if the intent here is to wrap up an exception. Most exceptions generated by the DOM (including most DOMExceptions) do not have a line/column number. I imagine I can put together a quick test case to show that.

Is the goal to wrap up an exception, though? E.g. does this exception show up elsewhere as a promise rejection or thrown exception?

[mvano]

No this is not an exception that might be thrown or used to reject a promise. This is an event indicating that e.g. the client is out of sync with the server, or the registration has been invalidated and must be renewed.

Still it looks exactly like an exception: it needs a type or a name, and it would be nice to provide a message field for the author as well.

If I was to invent a clean new generic type it would look like this:

interface ErrorEvent : Event {
  readonly attribute DOMString name;
  readonly attribute DOMString message;
};

[mounirlamouri]

Why not using DOMExceptions instead of having name and message?

[mvano]

@mounirlamouri, because we're firing an event, not throwing an error that can be caught or is used to reject a promise. The event.error syntax seems verbose and it also seems weird to receive an error object that way.

Anyway, that's my ideal, the cleanest, most generic form of an event that signals some kind of error happened. I can use less pretty syntax if it avoids reinventing multiple wheels.

[annevk]

@mvano if you actually expect people to branch based on the error, why not have distinct events?

[annevk]

@mvano that seems much more natural for an event based system. You could even dispatch the specific event first and then a generic simple event named error. Various APIs have such a setup.

[mvano]

@annevk That's an interesting idea, thanks. Just firing distinct simple events of type PushRegistrationLost and PushOutOfSync might be sufficient. We'd not have the message field to provide additional details, but I think we can try it.

[mounirlamouri]

👍

[annevk]

Please keep the names all lowercase, but yes.