JDK 9 RFR : java.util.logging.Formatter#formatMessage() swallows Exceptions
alexander.fomin at oracle.com
Fri Nov 20 17:40:42 UTC 2015
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
> 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
>> 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.
>> 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 :
>> java.util.logging.Formatter#formatMessage() swallows Exceptions
>> On 20/11/15 15:47, Jason Mehrens wrote:
>>> 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
>>> 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 :
>>> java.util.logging.Formatter#formatMessage() swallows Exceptions
>>> please, review this patch to report errors in
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8137005
>>> Webrev: http://cr.openjdk.java.net/~dfuchs/alexander/8137005/webrev.00
>>> 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.
>>> require API changes. So, I'm going to file CCC when if the fix
>>> The suggested fix is to add 2 new methods in
>>> to set/get an ErrorManager, update Formatter#formatMessage() to report
>>> errors via the ErrorManager and update Handler to pass errorManager to
>>> A couple of new regression tests have been created:
>>> test/java/util/logging/Test8137005.java - real case
>>> provided by
>>> test/java/util/logging/NullErrorManagerTest.java -
>>> 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:
>>> All tests passed passed.
>>> failures in the job are known issues and not related to the fix.
More information about the core-libs-dev