JDK 9 RFR [8137005]: java.util.logging.Formatter#formatMessage() swallows Exceptions

[Alexander Fomin]

On 20.11.2015 20:04, Daniel Fuchs wrote:
> On 20/11/15 17:55, Jason Mehrens wrote:
>> 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).
>
> oh - I see. I hadn't thought of that. That's actually a very good 
> suggestion. So the Formatter could access the Handler's ErrorManager
> and if that's not null it could call handler.errorManager.error(...)
>
> That's what you are suggesting - right?

What about MemoryHandler and SocketHandler? Should we call 
getHead()/getTail() somewhere in it just to get a Formatter a chance to 
report errors?

Thanks,
Alexander


>
> best regards,
>
> -- daniel
>
>> 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.fuchs at oracle.com>
>> Sent: Friday, November 20, 2015 9:32 AM
>> To: Jason Mehrens; Alexander Fomin; core-libs-dev at openjdk.java.net; 
>> mandy.chung at 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-bounces at openjdk.java.net> on 
>>> behalf of Alexander Fomin <alexander.fomin at oracle.com>
>>> Sent: Friday, November 20, 2015 7:48 AM
>>> To: core-libs-dev at openjdk.java.net; Daniel Fuchs; 
>>> mandy.chung at 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
>>>
>>>
>>>
>>
>