I think if it leads to better diagnosis, it is definitely worth it, and the batching IMHO is a good thing, since you don't have to compile/edit/debug several times to reveal hidden exceptions that only occur if the loop isn't exited early. I guess I can see one common way that an onLoad could generate a recoverable exception, and that's if it performs some network I/O or depends on network I/O data which failed for transient reasons. -Ray
On Mon, Sep 14, 2009 at 2:17 PM, John LaBanca <[email protected]> wrote: > It wouldn't necessarily need a refresh, and at least the attach state would > be valid. I have two major concerns about leaving it out: > > 1. It could lead to memory leaks, because widgets are physically attached > and cannot be detached. > 2. adwords is running into this, and I can't isolate the real cause of the > problem (ie, where is onLoad or onUnload really failing). With this patch > or one like it, it should be easier to find real onLoad/onUnload failures in > awfe. > > Thanks, > John LaBanca > [email protected] > > > On Mon, Sep 14, 2009 at 5:13 PM, Ray Cromwell <[email protected]>wrote: > >> >> It seems weird from the standpoint that in the ordinary state of affairs, >> you'd only get one exception from one detach operation and the loop would >> terminate early. I guess my question is, even with this fix, is the >> application likely to be in a good state that can keep running afterwards? >> Seems to be if an onLoad fails, the application is likely to be unable to >> recover and need a refresh in many cases, would it not? >> >> -Ray >> >> On Mon, Sep 14, 2009 at 1:53 PM, John LaBanca <[email protected]>wrote: >> >>> You don't think its a little weird to batch up exceptions and then throw >>> them? It fixes the problem for everyone, but for some reason it seems weird >>> to me. >>> >>> Thanks, >>> John LaBanca >>> [email protected] >>> >>> >>> On Mon, Sep 14, 2009 at 4:51 PM, Ray Ryan <[email protected]> wrote: >>> >>>> NM, brainfart. >>>> But I'm confused just why you're tying this to UncaughtExceptionHandler. >>>> The invariant will still go straight to hell if none has been provided, >>>> right? Also, why not fix this in a single spot rather than several >>>> scattered >>>> places? Also, it's kind of weird to catch Throwable rather than Exception. >>>> >>>> But assuming you're sure about Throwable, seems like you should instead >>>> do something like this in Panel: >>>> >>>> public class PanelDetachException extends RuntimeException { >>>> PanelDetachException(Set<Throwable> causes) { ... } >>>> Set<Throwable> getCauses() {...} >>>> } >>>> >>>> protected void doDetachChildren() { >>>> Set<Throwable> caught = new HashSet<Throwable>(); >>>> >>>> // Ensure that all child widgets are detached. >>>> for (Iterator<Widget> it = iterator(); it.hasNext();) { >>>> Widget child = it.next(); >>>> try { >>>> child.onDetach(); >>>> } catch (Throwable e) { >>>> caught.add(e); >>>> } >>>> } >>>> >>>> if (!caught.isEmpty()) { >>>> throw new PanelDetachException(caught); >>>> } >>>> } >>>> >>>> >>>> On Mon, Sep 14, 2009 at 1:35 PM, John LaBanca <[email protected]>wrote: >>>> >>>>> The uncaughtexceptionhandler is in an inner try/catch block. The outer >>>>> try/finally still runs, so we always reach the finally block. >>>>> >>>>> Thanks, >>>>> John LaBanca >>>>> [email protected] >>>>> >>>>> >>>>> On Mon, Sep 14, 2009 at 4:34 PM, John LaBanca <[email protected]>wrote: >>>>> >>>>>> What do you mean? >>>>>> >>>>>> >>>>>> Thanks, >>>>>> John LaBanca >>>>>> [email protected] >>>>>> >>>>>> >>>>>> >>>>>> On Mon, Sep 14, 2009 at 4:33 PM, <[email protected]> wrote: >>>>>> >>>>>>> On 2009/09/14 20:23:31, jlabanca wrote: >>>>>>> >>>>>>> >>>>>>> So the UncaughtExceptionHandler violates finally? Isn't that a pretty >>>>>>> fundamental problem? >>>>>>> >>>>>>> >>>>>>> http://gwt-code-reviews.appspot.com/64815 >>>>>>> >>>>>> >>>>>> >>>>> >>>> >>> >>> >>> >> >> >> > > > > --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
