Right. I know attachments get trashed sometimes so hopefully this hacked up
code in-lines correctly:
=====================================
private Exception formatThrown;
private volatile Handler lastHandler;
private void reportError(Handler h, Exception ex) {
if (h == null) {
h = this.lastHandler;
}
if (h != null) {
this.lastHandler = h;
if (ex == null) {
ex = set(null);
}
if (ex != null) {
h.reportError(null, ex, ErrorManager.FORMAT_FAILURE);
}
} else {
if (ex != null) {
set(ex);
}
}
}
private synchronized Exception set(Exception e) {
Exception l = this.formatThrown;
this.formatThrown = e;
return l;
}
==========================================
Then in the formatter.getHead/getTail call reportError(h, null) and in
formatMessage call reportError(null, ex). This example can be improved by
clearing the handler/exception in getTail and avoid some redundant volatile
stores but you get the idea.
Jason
________________________________________
From: Daniel Fuchs <daniel.fu...@oracle.com>
Sent: Friday, November 20, 2015 11:04 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 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?
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.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
>>
>>
>>
>
