Hi Daniel,
You covered my main concern regarding the notification so I'm on board with this patch. For any other package I agree with you on the frowning of System.err. However, the LogManager/Handler.reportError(ex2) seem to use it like it is the debug/failsafe option for the logging framework. For 8043306, I think designing a failsafe mode for these types of exceptions is out of scope. That is why I suggested using System.err. I still think that in this case the specification could be a little user hostile given how hard it is to get the LogManager initialization just right. I would have gone for a spec with a tone of "Don't throw uncaught exceptions. If you do you are on your own." regards, Jason ---------------------------------------- > Date: Mon, 15 Sep 2014 18:03:19 +0200 > From: daniel.fu...@oracle.com > To: jason_mehr...@hotmail.com > CC: core-libs-dev@openjdk.java.net > Subject: Re: RFR: 8043306 - Provide a replacement for the API that allowed to > listen for LogManager configuration changes > > Hi Jason, > > Logging on System.err is usually frowned upon - especially if there > are alternatives. > > May I suggest the following? > > 1. do option #1, and only fail after all listeners have been notified > as Alan indicated 'it might not be too bad'. > As Paul suggested to me privately we could use > Throwable.addSuppressed to link the suppressed exceptions, if there > are any. > > 2. Log an RFE for LogManager to use ErrorManager (or > some similar object). > > IMHO the proposal #1 above should not cripple how LogManager will > evolve in the future too much - especially if it is a rarely > used functionality. > > Here is a new webrev - with updated test case: > > http://cr.openjdk.java.net/~dfuchs/webrev_8043306/webrev.05/ > > best regards > > -- daniel > > On 9/12/14 7:21 PM, Jason Mehrens wrote: >> Daniel, >> >> >> Agreed. My preference would be to: >> >> 1. Catch and report them to 'System.err'. That type of code happens in the >> LogManager and exceptions in this case are not the normal condition. >> >> 2. Don't specify how uncaught exceptions are handled in the javadocs. >> >> 3. Outside of this issue, make an RFE to allow the LogManager to contain a >> j.u.l.ErrorManager and use it to replace all calls to System.err to use this >> new root/default error manager. Once these exception are captured in a error >> manager you can alter the behavior (rethrow, write to console, ignore, >> report to O/S event log, etc.) >> >> >> I worry that over specification of the exception handling might cripple how >> the LogManager can evolve with regard to future RFEs. >> >> >> Jason >> >> ---------------------------------------- >>> Date: Fri, 12 Sep 2014 18:54:12 +0200 >>> From: daniel.fu...@oracle.com >>> To: jason_mehr...@hotmail.com >>> CC: core-libs-dev@openjdk.java.net >>> Subject: Re: RFR: 8043306 - Provide a replacement for the API that allowed >>> to listen for LogManager configuration changes >>> >>> On 9/12/14 5:39 PM, Jason Mehrens wrote: >>>> Daniel, >>>> >>>> >>>> I suppose that the propagating uncaught exceptions to the caller was the >>>> previous behavior of the old property change methods but it seems out of >>>> place for the LogManager. The LogManager is a global resource so broken >>>> listener code in web app A could suppress notifications in web app B and >>>> C. That doesn't seem very nice. A lot of the LogManager code handles >>>> exception via catch, report or ignore, and continue when dealing with >>>> handlers etc. I would think that same strategy applies here too. >>> >>> Hi Jason, >>> >>> Yes - that was also the behavior of the old property change methods. >>> When the global LogManager calls itself readConfiguration it will >>> silently ignore exceptions that might be raised. >>> >>> I'm not too keen in specifying that exceptions raised by listeners will >>> be silently dropped. We're in the process of reinitializing the >>> configuration, >>> so trying to log the exceptions through a Logger would have its own >>> risks too. >>> Reporting exceptions raised during initialization and configuration is >>> an area where java.util.logging is clearly lacking. >>> >>> For this patch - I think that our options are limited to the alternative: >>> 1. propagate the exception >>> 2. catch and silentlty drop the exception >>> >>> What is the 'better' behavior (and I agree neither of the two are ideal) >>> probably depends on what is the typical use case for these listeners. >>> My assumption is that in most cases - there will be a single listener, in >>> which case 1. is probably better. I haven't seen any bug complaining >>> about how exceptions were handled in the previous implementation, >>> though I have seen some complaining about swallowed exceptions... >>> >>> That said - if there is consensus that 2. is better - I have no real >>> objection except those stated above. >>> >>> best regards, >>> >>> -- daniel >>> >>> >>> >>>> >>>> >>>> >>>> Jason >>>> >>>> >>>> ---------------------------------------- >>>>> Date: Fri, 12 Sep 2014 16:59:03 +0200 >>>>> From: daniel.fu...@oracle.com >>>>> To: alan.bate...@oracle.com >>>>> Subject: Re: RFR: 8043306 - Provide a replacement for the API that >>>>> allowed to listen for LogManager configuration changes >>>>> CC: core-libs-dev@openjdk.java.net >>>>> >>>>> On 9/12/14 4:42 PM, Alan Bateman wrote: >>>>>> On 12/09/2014 15:16, Daniel Fuchs wrote: >>>>>>> Thanks Alan! >>>>>>> >>>>>>> I have updated the webrev with your suggestions: >>>>>>> >>>>>>> http://cr.openjdk.java.net/~dfuchs/webrev_8043306/webrev.04/ >>>>>>> >>>>>>> -- daniel >>>>>>> >>>>>> A minor suggestion for readConfiguration is that "Any register >>>>>> configuration listeners .." might be a bit better than "The >>>>>> configuration listeners ...". >>>>>> >>>>>> In addConfigurationListener there is a <br> between the two sentences >>>>>> that talk about exceptions, I don't know if you intended that. >>>>> Done. I regenerated the webrev in place. >>>>> http://cr.openjdk.java.net/~dfuchs/webrev_8043306/webrev.04/ >>>>> >>>>> -- daniel >>>>>> Otherwise the javadoc looks good to me. >>>>>> >>>>>> -Alan. >>>>> >>> >
