Expose underlying processor to blame for thrown exception within CompoundProcessor#18342
Expose underlying processor to blame for thrown exception within CompoundProcessor#18342talevy merged 1 commit intoelastic:masterfrom
Conversation
There was a problem hiding this comment.
This test originally exposed the bug, and let it pass. no more!
There was a problem hiding this comment.
actually, this needs to handle CompoundProcessorException separately as well
There was a problem hiding this comment.
I'm not sure this method name is right. Maybe I just don't know this code that well though.
There was a problem hiding this comment.
Yup. I just don't know it well enough. Disregard.
There was a problem hiding this comment.
haha, I don't blame you. I had a really awkward time trying to name all of these subtly different scenarios
|
LGTM though maybe @javanna should have a look? |
|
thanks for the review @nik9000, I'll give @javanna or @martijnvg a little bit of time to check this out as well. It was one of these problems that arose from refactoring, and this solution was kind of lost in the changes (seemed more difficult at the time). |
13aee39 to
6ad608b
Compare
|
LGTM |
There was a problem hiding this comment.
This was updated so that we do not expose CompoundProcessorException outside of the Ingest Pipeline.
…oundProcessor Fixes elastic#17823
|
@nik9000 @martijnvg thank you for the reviews. Originally, I was missing an integration test that actually caught a big flaw in my original In the latest push, I wrap all CompoundProcessor related exceptions in an ElasticsearchException which relays the processor information in its headers. let me know what you think! |
|
LGTM 👍 |
Fixes #17823