Hi Daniel, Well I'm sure the authors of the unit tests wrote code that never leaks the handlers they have created right? :)
If urgency or frequency of the reporting is required then capture the handler in getHead as formatter state. The write code to report the exception under all possible states: 1. if exception present and getHead is called then report it to non null Handler and clear (JDK-6351685). 2. if exception happens during format and we have a handler captured from getHead then report otherwise store it. 3. if exception is present during getTail then report it to a non null Handler and clear the exception and hander. That means you'll have to add super.getHead and super.getTail calls in XMLFormatter too. I'm really leaning towards removing this try/catch (https://blogs.oracle.com/darcy/entry/kinds_of_compatibility). Handlers already expect that Formatter.format->formattMessage will fail. After all if they didn't fail we wouldn't have specified ErrorManager.FORMAT_FAILURE. Take a look at StreamHandler.publish. Or the handler implementation goes in the opposite direction and makes the publish method exception hostile which is already a violation of the spec. It pains me to say it but, as long as you don't break the SLF4J bridge handler then you have covered most of the JUL users. Jason ________________________________________ From: Daniel Fuchs <daniel.fu...@oracle.com> Sent: Friday, November 20, 2015 9:32 AM To: Jason Mehrens; Alexander Fomin; core-libs-dev@openjdk.java.net; mandy.ch...@oracle.com Subject: Re: JDK 9 RFR [8137005]: java.util.logging.Formatter#formatMessage() swallows Exceptions On 20/11/15 15:47, Jason Mehrens wrote: > Alexander, > > Why not just cache the last exception in the formatter and use getTail to > clear it and report it? Since formatter is in the same package as Handler > you will have elevated access to the error manager through > Handler.reportError. That also makes it so you don't have to change the > public API of Formatter. Hi Jason, That would mean that you won't see the exception until the handler is closed. Not sure whether that matters much. ErrorManager looks already bizarre to me. But at least with ErrorManager it looks as if someone who cares could set his/her own ErrorManager on the formatter (with hopefully a more sensible implementation). I have no specific opinion on the subject I'm in favor of taking the solution that is the least likely to cause compatibility issues :-) best regards, -- daniel > > Jason > > ________________________________________ > From: core-libs-dev <core-libs-dev-boun...@openjdk.java.net> on behalf of > Alexander Fomin <alexander.fo...@oracle.com> > Sent: Friday, November 20, 2015 7:48 AM > To: core-libs-dev@openjdk.java.net; Daniel Fuchs; mandy.ch...@oracle.com > Subject: JDK 9 RFR [8137005]: java.util.logging.Formatter#formatMessage() > swallows Exceptions > > Hi, > please, review this patch to report errors in > java.util.logging.Formatter#formatMessage(). > > Bug: https://bugs.openjdk.java.net/browse/JDK-8137005 > Webrev: http://cr.openjdk.java.net/~dfuchs/alexander/8137005/webrev.00 > > Summary: > j.u.logging.Formatter#formatMessage() swallows exceptions that > happening during formatting of a message. In the result the exceptions > are lost and users don't know about reasons why the message hasn't been > formatted as expected. We would avoid to throw any exceptions in > Formatter#formatMessage() from compatibility stand point. To report an > error in consistent way we have to pass ErrorManager in Formatter. It's > require API changes. So, I'm going to file CCC when if the fix approved. > The suggested fix is to add 2 new methods in j.u.logging.Formatter > to set/get an ErrorManager, update Formatter#formatMessage() to report > errors via the ErrorManager and update Handler to pass errorManager to > Formatter. > > Testing: > A couple of new regression tests have been created: > test/java/util/logging/Test8137005.java - real case provided by > users > test/java/util/logging/NullErrorManagerTest.java - additional > check to make sure no NPE showed if ErrorManager isn't set. Beside of > this touched new API methods. > > Logging regression tests have been run: > jdk/test/java/util/logging > jdk/test/closed/java/util/logging > jdk/test/sun/util/logging > All tests passed passed. > > JPRT: > http://sthjprt.uk.oracle.com/archives/2015/11/2015-11-19-143523.gtee.dev/ > failures in the job are known issues and not related to the fix. > > Thanks, > Alexander > > >
