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

Add parsing method for ElasticsearchException.generateFailureXContent() #22815

Merged

Conversation

Projects
None yet
3 participants
@tlrx
Copy link
Member

commented Jan 26, 2017

This commit adds a ElasticsearchException.failureFromXContent() that can be used to parse the result of ElasticsearchException.generateFailureXContent().

Add parsing method for ElasticsearchException.generateFailureXContent()
This commit adds a ElasticsearchException.failureFromXContent() that can be used to parse the result of ElasticsearchException.generateFailureXContent().
@javanna
Copy link
Member

left a comment

left a few comments

@@ -527,7 +519,8 @@ public static void generateThrowableXContent(XContentBuilder builder, Params par
* exception is rendered. When it's true all detail are provided including guesses root causes, cause and potentially stack
* trace.
*
* This method is usually used when the {@link Exception} is rendered as a full XContent object.
* This method is usually used when the {@link Exception} is rendered as a full XContent object., and its output can be parsed

This comment has been minimized.

Copy link
@javanna

javanna Jan 26, 2017

Member

leave , and remove . ?

ensureFieldName(parser, token, ERROR);

token = parser.nextToken();
if (token.isValue() || token == XContentParser.Token.VALUE_NULL) {

This comment has been minimized.

Copy link
@javanna

javanna Jan 26, 2017

Member

when can the error be null?

This comment has been minimized.

Copy link
@tlrx

tlrx Jan 26, 2017

Author Member

Never, I'll remove that

return new ElasticsearchException(buildMessage("exception", parser.textOrNull(), null));
}

ensureExpectedToken(token, XContentParser.Token.START_OBJECT, parser::getTokenLocation);

This comment has been minimized.

Copy link
@javanna

javanna Jan 26, 2017

Member

how about the root cause? I remember we discussed that it is a problem when we get more than one, but can we add a TODO for it somewhere or do at least something when we have a single one? I don't see where we parse them but I may be missing something. Maybe they end up skipped where we parse metadata arrays in innerFromXContent

This comment has been minimized.

Copy link
@tlrx

tlrx Jan 26, 2017

Author Member

You're right, root causes are ignored because innerFromXContent ignores arrays of objects for now. I should have added a comment to make this clear, sorry.

I added a TODO for parsing root causes. I'd like to address this later in another issue because it might be tricky to parse them depending of the order of fields. Most of the time root_causes will come first but we must not rely on that and so innerFromXContent has to be adapted a bit.

@javanna
Copy link
Member

left a comment

LGTM

@tlrx tlrx merged commit ea7077f into elastic:master Jan 27, 2017

2 checks passed

CLA Commit author is a member of Elasticsearch
Details
elasticsearch-ci Build finished.
Details
@tlrx

This comment has been minimized.

Copy link
Member Author

commented Jan 27, 2017

Thanks @javanna !

@tlrx tlrx deleted the tlrx:add-parsing-methods-for-generate-failure-to-xcontent branch Jan 27, 2017

tlrx added a commit that referenced this pull request Jan 27, 2017

Add parsing method for ElasticsearchException.generateFailureXContent…
…() (#22815)

This commit adds a ElasticsearchException.failureFromXContent() that can be used to parse the result of ElasticsearchException.generateFailureXContent().
@tlrx

This comment has been minimized.

Copy link
Member Author

commented Jan 27, 2017

5.x: 45c4973

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.