On 17.09.2011, at 05:19, Ralph Goers wrote:
>
> On Sep 16, 2011, at 5:09 PM, John Vasileff wrote:
>
>>
>> On Sep 16, 2011, at 7:41 PM, Joern Huxhorn wrote:
>>
>>>
>>> On 16.09.2011, at 19:09, John Vasileff wrote:
>>>
>>>>
>>>> On Sep 15, 2011, at 7:46 PM, Ralph Goers wrote:
>>>>
>>>>> My gut reaction is that I agree with you. I ran across this behavior a
>>>>> while ago and Joern mentioned that ParameterizedMessage was working the
>>>>> way it is supposed to, which given the behavior you are noting just
>>>>> doesn't seem correct.
>>>>>
>>>>> On the other hand, currently not every Message implementation cares about
>>>>> Throwables. I'm not sure what StructuredDataMessage should do with one,
>>>>> for example. What do you think should be done to handle that?
>>>>>
>>>>
>>>> I think it is analogous to getParameters(). It is a common feature, but
>>>> not one that all Message types need. So I think the question is how
>>>> granular the interfaces should be. Message could have _only_
>>>> getFormattedMessage(), and everything else added by sub interfaces. But
>>>> for very common things like Throwables or Parameters, I tend to think it
>>>> is easier to have them in the main Message interface and avoid instanceof
>>>> clutter in the rest of the code.
>>>>
>>>
>>> I wonder why you are so keen to extend the basic Message interface with
>>> arbitrary methods that are only relevant for certain Message
>>> implementations?
>>>
>>> The Runnable interface does also only have a run() method and not some
>>> obscure Object[] getArguments() / void setArguments(Object[]) methods.
>>>
>>
>> It is not so much that I think the Message interface should have
>> getThrowable(). It is that I think that message objects should carry the
>> throwable, if one exists.
>>
>> void info(Message msg);
>> void info(Message msg, Throwable t);
>>
>> I don't think the second method above should exist, just as we don't have:
>>
>> void info(Message msg, Object[] params);
>>
>> I would be happy to rework the patch with something like:
>>
>> interface Message { public String getFormattedMessage(); }
>> interface ThrowableMessage { public Throwable getThrowable(); }
>> interface ParameterizedMessage { public Object[] getParameters(); }
>> etc.
>>
>> We would need to think through the appropriate granularity and try to figure
>> out what the best mix of interfaces is.
>
> With the above we would still need to support
>
> void info(Message msg, Throwable t)
>
> even though the Message interface might be one that implements
> ThrowableMessage to allow support for those that don't. That would be a bit
> awkward wouldn't it? That also means filters and appenders would need to
> check to see if Message implements ThrowableMessage and then look in the
> LogEvent for the Throwable if it doesn't.
>
Adding getThrowable() would also reintroduce the issue that such a Message
could not be serialized securely since deserialization would require the
exceptions class on the classpath. In my ParameterizedMessage over at
https://github.com/huxi/slf4j/blob/slf4j-redesign/slf4j-n-api/src/main/java/org/slf4j/n/messages/ParameterizedMessage.java
the Throwable is transient so it is deliberately lost while serializing, i.e.
it must be taken care of by the framework immediately after the message is
created.
It is only contained in ParameterizedMessage at all simply because a method can
only return a single value. And the create-method is also responsible for
resolving the Throwable at the last position of arguments if it is not used up
by a corresponding {}.
Yes, it would be "cleaner" to split that into a parseresult object that
contains both the message and an additional, optional Throwable. I just didn't
implement it that way to reduce the amount of garbage (that parseresult object
would get garbage-collected immediately after evaluation).
So adding Throwable to ParameterizedMessage was just a performance optimization.
The serialization issue is btw also the reason for *not* having Object[]
getParameters() but String[] getParameters() instead. The String representation
is the serialization-safe version of the parameters.
But all of this is, in my books, an implementation detail of the Message
implementation with getFormattedMessage() being the only "public" API of
Message.
Joern.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
