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.
