Alexander,
I see your point. It is also out of spec for Handler.setFormatter to actually
call Formatter.setErrorManager. What if there are subclasses of formatter that
exist today with a setErrorManager method? Also not all handlers have one
formatter or actually call super.setFormatter since you can't unseal the super
handler (GAE).
I still lean toward letting the exceptions fly since there is already an
expectation that formatters fail. Is the concern that the handler will fail or
that the program will fail?
I've recently been tinkering with a custom filter that internally uses a
formatter and have prototyped adding get/setErrorManager method to it and it
just becomes a mess because to have start thinking about more corner cases and
adding more to the spec.
I really think that if you are going to add the 3 ErrorManager methods you
should only add them to the LogManager (JDK-6865510) as a way to unify failure
reporting in the LogManager itself and to deal with failures inside of error
manger. As in to remove that ugly System.err code in Handler.reportError. The
LogManager would work kind of like Thread.defaultUncaughtExceptionHandler and
the Handler.errorManager is kind of like the Thread.uncaughtException handler.
Then to fix this bug we could report it to via
LogManager.getLogManager().reportError.
Jason
________________________________________
From: Alexander Fomin <alexander.fo...@oracle.com>
Sent: Friday, November 20, 2015 11:27 AM
To: Jason Mehrens; core-libs-dev@openjdk.java.net; Daniel Fuchs;
mandy.ch...@oracle.com
Subject: Re: JDK 9 RFR [8137005]: java.util.logging.Formatter#formatMessage()
swallows Exceptions
Hi Jason
On 20.11.2015 17:47, Jason Mehrens wrote:
> Alexander,
>
> Why not just cache the last exception in the formatter and use getTail to
> clear it and report it?
It might be a good solution, but according to spec
Formatter#getTail() method is intended to return the tail string for a
set of formatted records. With your approach it would need to modify the
spec any way with notes about possible exception messages in the queue.
Besides that, the tail of the records should be the latest record.
I think that according to current spec it must be just that unformatted
message which had been passed to j.u.l.Formatter#formatMessage() and
failed to be formatted. Therefore the logic in Handler would be go
through all records in order to find Exceptions and report them if
found. Looks a little bit strange.
But what I really care about is possible compatibility issue if
getHead()/Tail() overridden in user's Formatter implementation. One of
the example is j.u.l.XMLFormatter. Even if Exceptions have been cached
in base Formatter, it would have not been reachable in Handler with
current XMLFormatter.getTail() implementation. Well, we can modify
XMLFormatter.getTail() since it's Java code, but I don't think user's
will happy with an idea to modify anything in their app code.
> Since formatter is in the same package as Handler you will have elevated
> access to the error manager through Handler.reportError.
Well, to which Handler? Formatter don't know anything about Handler(s)
that use(s) it. So, the only way in Formatter to get the reference on
some Handler are methods where it passed getHead()/getTail(). Taking
into account these methods may be overridden in user's implementations
the solution doesn't seem reliable.
And then, with setErrorManager()/getErrorManager() user may set his
favorite ErrorManager.
Any way, taking into account that at least some spec changes would need
in any case, it would better to extend API for flexibility and reliability.
Thanks,
Alexander
> That also makes it so you don't have to change the public API of Formatter.
>
> 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
>
>
>
