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

Retain exception fields in deserialization #49278

Closed

Conversation

Tim-Brooks
Copy link
Contributor

In ElasticsearchException type, reason, and stacktrace are
serialized to xcontent. However, when the xcontent is deserialized
these fields are lost. This PR adds a method that will optionally retain
these fields for usage when the exception is serialized a second time.

@Tim-Brooks Tim-Brooks added >non-issue v8.0.0 :Distributed/Reindex Issues relating to reindex that are not caused by issues further down v7.6.0 labels Nov 18, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Reindex)

@Tim-Brooks
Copy link
Contributor Author

@ywelsch @henningandersen This is a POC of how we could retain the information between deserializing a exception from reindexing and deserializing it for http response.

@Tim-Brooks Tim-Brooks added the WIP label Nov 18, 2019
Copy link
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

I think this looks like the right direction. I left a few comments to consider/discuss.

@@ -1182,4 +1226,18 @@ private static String toUnderscoreCase(String value) {
return sb.toString();
}

private static class XContentRetainingException extends ElasticsearchException {

private final String fromXContentExceptionType;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to also return this type from the static getExceptionName method.

@polyfractal polyfractal added v7.7.0 and removed v7.6.0 labels Jan 15, 2020
henningandersen added a commit to henningandersen/elasticsearch that referenced this pull request Feb 25, 2020
Added xcontent serialization tests for ReindexTaskStateDoc

Related to elastic#42612
Depends on elastic#49278 (todo)
@bpintea bpintea added v7.8.0 and removed v7.7.0 labels Mar 25, 2020
@rjernst rjernst added the Team:Distributed Meta label for distributed team label May 4, 2020
@ywelsch ywelsch removed their request for review July 23, 2020 07:25
@andreidan andreidan removed the v7.10.0 label Oct 7, 2020
@Tim-Brooks Tim-Brooks closed this Sep 8, 2021
henningandersen added a commit that referenced this pull request Sep 1, 2022
Added xcontent serialization tests for ReindexTaskStateDoc

Related to #42612
Depends on #49278 (todo)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Reindex Issues relating to reindex that are not caused by issues further down >non-issue Team:Distributed Meta label for distributed team v7.16.0 v8.0.0-alpha2 WIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.