Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Transform] Fix casting in ExceptionRootCauseFinder #66123

Conversation

dimitris-athanasiou
Copy link
Contributor

Method getRootCauseException is checking whether the unwrapped
exception is an instance of SearchPhaseExecutionException but then
proceeds to cast the parent exception. This would lead to a casting
error. This commit fixes this and adds a unit test to guard it.

Method `getRootCauseException` is checking whether the unwrapped
exception is an instance of `SearchPhaseExecutionException` but then
proceeds to cast the parent exception. This would lead to a casting
error. This commit fixes this and adds a unit test to guard it.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core (:ml/Transform)

Copy link
Contributor

@przemekwitek przemekwitek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Good spot!

Copy link

@hendrikmuhs hendrikmuhs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, good spot.

I did some research on the code, fortunately this case never happens, SearchPhaseExecutionException is never wrapped and never meant to be wrapped in a ElasticsearchWrapperException. Instead of checking unwrappedThrowable its also possible to check t and unwrap it, if its not a SPEE. However, it does not to hurt to do it this way.

@hendrikmuhs
Copy link

I suggest to re-label it as non-issue to avoid potential noise.

@dimitris-athanasiou dimitris-athanasiou merged commit 27ca279 into elastic:master Dec 10, 2020
@dimitris-athanasiou dimitris-athanasiou deleted the fix-casting-in-transform-root-cause-finder branch December 10, 2020 09:12
dimitris-athanasiou added a commit to dimitris-athanasiou/elasticsearch that referenced this pull request Dec 10, 2020
Method `getRootCauseException` is checking whether the unwrapped
exception is an instance of `SearchPhaseExecutionException` but then
proceeds to cast the parent exception. This would lead to a casting
error. This commit fixes this and adds a unit test to guard it.

Backport of elastic#66123
dimitris-athanasiou added a commit that referenced this pull request Dec 10, 2020
…6150)

Method `getRootCauseException` is checking whether the unwrapped
exception is an instance of `SearchPhaseExecutionException` but then
proceeds to cast the parent exception. This would lead to a casting
error. This commit fixes this and adds a unit test to guard it.

Backport of #66123
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants