Don't wrap exceptions in MapperParsingException
#121
Conversation
|
@tlrx could you review that please? |
|
How about using the same test as
|
|
Also, if you decide not to wrap with |
|
@apogrebnyak yeah I looked at it but actually to know if it's Serializable, you actually need to serialize it and catch the The simplest workaround is to simply send the error message and not the full error stack. |
|
About logging: yeah I agree. |
| @@ -451,7 +451,8 @@ public void parse(ParseContext context) throws IOException { | |||
|
|
|||
| // #18: we could ignore errors when Tika does not parse data | |||
| if (!ignoreErrors) { | |||
| throw new MapperParsingException("Failed to extract [" + indexedChars + "] characters of text for [" + name + "]", e); | |||
| throw new MapperParsingException("Failed to extract [" + indexedChars + "] characters of text for [" + name + "] : " | |||
| + e.getMessage()); | |||
There was a problem hiding this comment.
Add log before throwing the Exception. See #121 (comment)
|
In this case, maybe use |
|
LGTM. Adding a complete message with root causes may be fine too but I think having the logs is enough. |
Some exceptions might not be serializable. It would be safer not to wrap them in a `MapperParsingException` but just create the `MapperParsingException`. Related to elastic#113.
dadoonet
force-pushed
the
pr/mapper-parsing-exception
branch
from
ccb1ca9
to
e58878c
Compare
Some exceptions might not be serializable. It would be safer not to wrap them in a
MapperParsingExceptionbut just create theMapperParsingException.Related to #113.