On Mon, Feb 8, 2010 at 7:53 AM, <[email protected]> wrote: > On 2010/02/03 20:16:51, Ray Ryan wrote: > >> Is there any reason to limit this to RuntimeException rather than >> > Exception? > > Not particularly; I did RuntimeException because #dispatch doesn't throw > any checked exceptions. Could change to Exception for clarity. > > > I think it would be better to capture all exceptions thrown during >> > event > >> dispatch and then re-throw them in an umbrella exception. This way it >> > will reach > >> the app's UncaughtExceptionHandler through the usual mechanisms >> > without the > >> developer having to think about it. >> > > We do something similar when attaching and detaching Widgets, see >> AttachDetachException. Don't mind its Command stuff, all you need to >> > imitate in, > >> say, EventDispatchException are >> > > AttachDetachException(Set<Throwable> causes) >> public Set<Throwable> getCauses() >> > > Really the thing to do is refactor out a common superclass, perhaps >> UmbrellaException >> > > Ok, I could change to that. Some questions, though: > > - Is there a worry that this changes the current behavior, such that if > anyone was relying on an early handler throwing that it would stop > subsequent handlers? >
Perhaps, but it's so much more likely that people are having mysterious failures that I'm comfortable with the change. We've made similar fixes recently in our core widgets, and the results have only been good. > > - Should HandlerManager still have an UncaughtExceptionHandler? In my > app I'd like to be able to have a HandlerManager that won't re-throw > exceptions, so that I can send Events out and still know that, > regardless of how they're handled, my original function will continue. > Won't that be the case with the flow I've described? All events will fire, and kick off whatever it is that they kick off, and then the collected exceptions will be re-thrown. In most apps, I figure this will reach the default UCE and put up a red banner or whatever. What would this break in your app? > > I could subclass HandlerManager and override its fireEvent method to > wrap it in a try/catch, but I'm worried that with that strategy the > event.kill() and event.setSource() calls at the very end wouldn't get > called if a handler threw (unless I suppose HandlerManager saved its > umbrella exception to throw after those lines are executed). Exactly, that's the intent. Protect the full event dispatch cycle, and then throw up. > > > http://gwt-code-reviews.appspot.com/136805/diff/1/2 >> File svn/user/src/com/google/gwt/event/shared/HandlerManager.java >> > (right): > > http://gwt-code-reviews.appspot.com/136805/diff/1/2#newcode64 >> Line 64: } catch (RuntimeException e) { >> Why limit to RuntimeException and not plain old Exception? >> > > > > http://gwt-code-reviews.appspot.com/136805 > -- I wish this were a Wave -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
