Bug 16608 - Algorithms should rethrow the exception
Algorithms should rethrow the exception
Product: WebAppsWG
Classification: Unclassified
Component: DOM Parsing and Serialization
PC All
: P2 normal
: ---
Assigned To: Ms2ger
Depends on:
  Show dependency treegraph
Reported: 2012-04-03 10:34 UTC by Simon Pieters
Modified: 2012-04-03 14:35 UTC (History)
4 users (show)

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description Simon Pieters 2012-04-03 10:34:52 UTC
"If this threw an exception, then terminate these steps."

Both the innerHTML setter algorithm and the fragment parsing algorithm have this step.

This means that setting innerHTML will silently do nothing if the XML fragment parsing algorithm throws. The fragment parsing algorithm should rethrow, and the innerHTML setter should rethrow.

(The produce an XML serialization algorithm says "If this threw an exception, allow that exception to propagate out from this algorithm" which seems fine.)
Comment 1 Ms2ger 2012-04-03 11:19:49 UTC
Changed to propagating exceptions by default:


Comments on how I phrased it definitely welcome.
Comment 2 Glenn Maynard 2012-04-03 13:08:37 UTC
I strongly disagree with this approach.

Specifications should explicitly catch and rethrow exceptions, so it's clear where the possible exit points in an algorithm are, what exceptions they may throw, and so overlooked exceptions at any point are strictly a bug.  If you can't keep track of what exceptions might be thrown underneath your algorithm, the people calling your algorithm won't be able to, either.
Comment 3 Ms2ger 2012-04-03 13:20:02 UTC
Well, that's unfortunate for you, then.
Comment 4 Ms2ger 2012-04-03 13:47:39 UTC
Apologies. I would like to politely disagree with your comment. I think having to propagate exceptions every time you call into an algorithm is rather error-prone (as proved by the bug) and unnecessarily verbose for the default case. Also, it seems rather confusing to silently eat exceptions in this spec, while other specs like ES5, WebIDL and DOM4 do the opposite. (I'm not sure what HTML does, but I would be rather surprised if it mentioned every place where an exception could be thrown.) This way of throwing exceptions also matches how exceptions work in programming.
Comment 5 Marcos Caceres 2012-04-03 14:14:37 UTC
FWIW, I strongly agree with Ms2ger. Exceptional behavior is usually defined underneath the algorithm that is being invoked (except if for some reason it needs to throw its own exception). So, apart from noting that it might happen (as in "If this threw an exception"), there is no need to redefine it because otherwise you might get conflicts in behavior (specially if the underlying spec changes beneath you). 

I'll note that this caused a lot of issues in, for illustrative/historical purposes, WAC's Device API's error handling behavior: because it defined throwing exceptions that were then contradicted by WebIDL's behavior... it basically meant that you could not implement WebIDL underneath an API because the API had defined it's own error handling that seemed to be overriding WebIDL... and hence it all became a huge non-conforming redundant confusing ugly ugly ugly untestable mess.
Comment 6 Glenn Maynard 2012-04-03 14:35:53 UTC
> Also, it seems rather confusing to silently eat exceptions in this spec

I'm confused--currently the spec is silently handling exceptions, and I'm saying it should be explicit, not silent.

HTML does explicitly catch and re-throw exceptions, eg. http://www.whatwg.org/specs/web-apps/current-work/multipage/history.html:

"Let cloned data be a structured clone of the specified data. If this throws an exception, then rethrow that exception and abort these steps."

I don't know if it does this consistently, but it should.

> This way of throwing exceptions also matches how exceptions work in programming.

I'm not a fan of Java's checked exceptions for most development (and I hate making the analogy at all), but when you need to be sure that everyone is properly handling exceptions in every possible case, as specs require, they're what you want.

Finally, you should be able to look at any algorithm in isolation and be able to immediately understand its logical flow and exit points.

> there is no need to redefine it because otherwise you might get conflicts in behavior (specially if the underlying spec changes beneath you). 

This is exactly why exceptions need to be handled explicitly.  If you're calling an algorithm with the expectation that it doesn't fail, then that spec *can't* be changed to throw an exception, unless you change your spec to deal with that.

For example, see http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-canvas-getcontext.  This calls the "Return a new object for contextId" algorithms in other specs.  This algorithm logically fails if that algorithm throws an exception, because it would incorrect leave the "primary context" set, and step 5 wouldn't make sense because it assumes that step 6 always completes.

If you want to define a "return a new object for" algorithm that defines an exception, you'd first need to have this algorithm updated, so it's written in a way that expects it.  Currently, you can't write an algorithm that does that; it's logically invalid and would be a spec bug.  If Canvas said "all exceptions are simply propagated upwards", all that does is implicitly allow other specs to throw exceptions in unexpected and broken ways.

I can't imagine how much would break if "decode a byte string as UTF-8, with error handling" or "Converting a string to ASCII uppercase" suddenly started throwing exceptions.