Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error handling to be revised #18

Open
ClaesNilsson opened this issue May 13, 2013 · 10 comments

Comments

Projects
None yet
4 participants
@ClaesNilsson
Copy link
Contributor

commented May 13, 2013

Generally the following rules apply:

  • Use exceptions for synchronous detectable errors in the application code.
  • Use asynchronous errro events for asynchronous dynamic errors.

Go through the specification and assure that this approach is fulfilled.

Specifcally for the UDP and TCPServerSocket constructors the following change should be done:

  • For UDPSocket and TCPServerSocket add an "open" event stating that socket has successfully been set up
  • Use readyState "connecting" during the process of setting up the UDPSocket and TCPServerSocket but state in the description that this means just setting up the socket for UDPSocket and TCPServerSocket. Or maybe it is better to have separate readyState enums for UDP, TCP and TCPServer?
  • Change the error handling for "localAddress/localPort already in use" from exception to using the "error" event.
@ClaesNilsson

This comment has been minimized.

Copy link
Contributor Author

commented Jul 5, 2013

Bakground to error handling modifications in Pull Request #34:

The background to theses updates on error handling is a mail dialogue between me and Jonas:
http://lists.w3.org/Archives/Public/public-sysapps/2013Jul/0008.html
http://lists.w3.org/Archives/Public/public-sysapps/2013Jul/0017.html
http://lists.w3.org/Archives/Public/public-sysapps/2013Jul/0020.html

So, the basic problem with error handling is that with the standard DOMError interface the granularity of machine readable error names are too limited. An example is when the attempt to create a TCP socket fails. The reason can for example be "no network contact"," peer does not respond" and "local address/port pair is already in use". The available DOM error names does not make it possible to separate these error reasons and the only sensible error name for all these situations is "NetworkError".

In the previous version of the specification I tried to solve this problem by distinguishing between these error reasons with the DOMError message attribute. However, the message attribute contains an implementation dependent message to be displayed to the user. Using this attribute does not make machine readable handling of errors possible, i.e. the application can not be coded to handle these dirrent error reasons in different manners.

The http://darobin.github.io/api-design-cookbook/ does not give much guidence here. So after discussion with Jonas I took the approach of subclassing DOMError and added the "type" attribute that provides a more detailed error definition than the "name" attribute.

This problem with error handling is probably applicable for more W3C specifications and it would be good if the authors of the DOM-specification stated a recommended approach on how to define a more granular error handling.

@ClaesNilsson

This comment has been minimized.

Copy link
Contributor Author

commented Aug 4, 2013

The approach I have taken in the Raw Socket API is a subclass of DOMError called SocketError that adds an attribute "type" that details the error in addition to the error names defined for DOMError.
WDYT? Should we contact the WebApps WG to hear their option on this approach? If so, do you (or anyone else) know if www-dom@w3.org is the correct address for a question on this?

@marcoscaceres

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2013

On Sunday, August 4, 2013, Claes Nilsson wrote:

The approach I have taken in the Raw Socket API is a subclass of DOMError
called SocketError that adds an attribute "type" that details the error in
addition to the error names defined for DOMError.
WDYT? Should we contact the WebApps WG to hear their option on this
approach? If so, do you (or anyone else) know if www-dom@w3.org<javascript:_e({}, 'cvml', 'www-dom@w3.org');>is the correct address for a question on this?

Yep, that's the right place to ask.


Reply to this email directly or view it on GitHubhttps://github.com//issues/18#issuecomment-22075697
.

Marcos Caceres
http://datadriven.com.au

@ClaesNilsson

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2013

Reply from Anne van Kesteren:

"DOMError is going away. We only need DOMException. Allen (editor of
ECMAScript) did suggest we do something like what you suggest. Have DOMException.prototype.subname which gives a more detailed name and ideally have DOMException.name be "DOMException" but we can probably no longer do that. Still need to work out the details here unfortunately as apparently last time we thought we figured error handling out we didn't :/"

So, I guess that I should keep the current solution in the Raw Socket API until the DOMException solution has been defined in the DOM specification.

@asutherland

This comment has been minimized.

Copy link

commented Aug 28, 2013

I'm not sure if this is the perfect issue for this, but in mozTCPSocket in Gecko for the Firefox OS e-mail app, we tried to add a number of specific error types and names related to network and security problems. The plan was to do what is proposed here, have a more generic type attribute and then some more specific error code information. Here is the webidl for what was desired but not implemented: https://bug867872.bugzilla.mozilla.org/attachment.cgi?id=744436

The implementing bug that added our error codes was https://bugzilla.mozilla.org/show_bug.cgi?id=861196 and resulted in the logic found here: https://github.com/mozilla/mozilla-central/blob/ba7e562edcd537b00aba393e065d964103f17348/dom/network/src/TCPSocket.js#L607

The specific error codes in use were the result of taking existing NSS error codes and weeding out the ones that Brian Smith indicated were going away imminently. This shouldn't be viewed as a specific proposal.

In order to avoid adding a ton of UI strings late in the release process which would be mean to localizers, we didn't map all of the error codes to distinct strings. But in general our e-mail app would like to be able at the very least to generate machine error codes for QA/support purposes even if we use a smaller subset of strings to explain that there is a security problem.

In general our error goal is to report these 3 categories: a security problem the user might be able to do something about (check the system clock), something that looks a lot like an attack against the user and therefore maybe they should try a different network, or something that just looks like a badly configured server. If we end up supporting adding certificate exceptions, we might end up wanting more information.

@ClaesNilsson

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2013

Thanks a lot for this information Andrew. I am in the process of executing a set of updates to the specification. Then I will go through the error situations again and your input will be very helpful here. So I might come back with questions.

@nickdesaulniers

This comment has been minimized.

Copy link

commented Oct 29, 2013

In example 3, I found it ambiguous that the assignment of the onerror member of a TCPSocket instance would itself be wrapped in a try ... catch block. In general, it leaves the developer more flexibility to return them the error object and let them decide if they want to throw it, rather than just always throw. Of course, developers are not always keen on checking return variables though.

@ClaesNilsson

This comment has been minimized.

Copy link
Contributor Author

commented Oct 30, 2013

The idea was to:

  • use exceptions for synchronous detectable errors in the application code.
  • use asynchronous errror events for asynchronous dynamic errors.

So for example an exception is thrown if the remoteAddress argument to the constructor is not a valid IPv4/6 address (a bug in the application code). On the other hand, if the attempt to establish a new TCP connection with the peer fails the error event is fired.

However, Marcos rewrote Examples 1 and 2 to not let the application catch the exeptions. This might be a better style and my plan is to rewrite examples 3 and 4 as well.

WDYT?

@ClaesNilsson

This comment has been minimized.

Copy link
Contributor Author

commented Mar 19, 2014

When using promises rejection reasons should always be instances of the ECMAScript Error type such as DOMException or the built in ECMAScript error types. See https://github.com/w3ctag/promises-guide#rejection-reasons-should-be-errors.

In the TCP and UDP Socket API specification we need a better granularity of machine readable errors and therefore the SocketError interface (http://raw-sockets.sysapps.org/#interface-socketerror) was added. This interface extends DOMError with a machine readble error type providing a better granularity of error reporting. However, the SocketError interface can not be used by promises so I have temporily just stated a DOMException for promises rejection reasons in the specification. This issue must be further explored and aligned with ongoing discussions in W3C, WHAT WG and ECMA. For example see http://esdiscuss.org/topic/error-objects-in-w3c-apis.

@ClaesNilsson

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2014

See the discussion in #77. The conclusion was that the specification should be walked through regarding error handling to see if any additional error types should be submitted to https://github.com/heycam/webidl/.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.