Output a consistent format when generating error json#90529
Output a consistent format when generating error json#90529thecoop merged 39 commits intoelastic:mainfrom
Conversation
Now, error fields will always have 'type' and 'reason' fields, and the information in those fields is the same regardless of whether the output is detailed or not
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
elasticsearchmachine
added
Team:Core/Infra
|
Hi @thecoop, I've created a changelog YAML for you. |
|
Note this does have some BwC concerns, as this changes the output format when not detailed, but from #89387 we consider the current behaviour a bug |
thecoop
force-pushed
the
exception-json
branch
from
237ef12 to
cec444a
Compare
server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java
Outdated
Show resolved
Hide resolved
|
Hi @thecoop, I've updated the changelog YAML for you. |
|
Hi @thecoop, I've updated the changelog YAML for you. |
|
Hi @thecoop, I've updated the changelog YAML for you. Note that since this PR is labelled |
thecoop
force-pushed
the
exception-json
branch
from
071c718 to
2a7fda6
Compare
thecoop
force-pushed
the
exception-json
branch
from
2a7fda6 to
750eb7d
Compare
|
This PR is marked as a "breaking" change. Is it really a breaking change? |
…d errors is changing in v9 (elastic#116330)" This reverts commit bd091d3.
|
Hi @thecoop, I've updated the changelog YAML for you. Note that since this PR is labelled |
ldematte
left a comment
ldematte
left a comment
There was a problem hiding this comment.
One minor nit, but it looks good to me!
server/src/test/java/org/elasticsearch/rest/RestResponseTests.java
Outdated
Show resolved
Hide resolved
|
@elasticmachine rerun elasticsearch-ci/part-1 |
|
@elasticmachine update branch |
mosche
left a comment
mosche
left a comment
There was a problem hiding this comment.
LGTM, just one optional thought / questions
|
|
||
| if (channel.detailedErrorsEnabled() == false) { | ||
| if (channel.request().getRestApiVersion() == RestApiVersion.V_8 && channel.detailedErrorsEnabled() == false) { | ||
| deprecationLogger.warn( |
There was a problem hiding this comment.
Should this change to critical now?
There was a problem hiding this comment.
I don't think so, there's (probably) nothing directly for users to do; we consider the existing behaviour a bug, as it's surprising the format changes with non-detailed errors.
Now, error fields will always have 'type' and 'reason' fields, and the information in those fields is the same regardless of whether the output is detailed or not
Now, error fields will always have 'type' and 'reason' fields, and the information in those fields is the same regardless of whether the output is detailed or not
|
Friendly reminder that this PR seems to be a breaking change for 9.0, but is missing from the 9.0 release note. We may want to add an entry to the breaking change section of 9.0 release note. |
|
See #125485 |
|
@thecoop is this PR relevant to the serverless changelog? [FYI this question is based on 9.0 breaking changes] |
|
No, this option is not relevant for serverless |
18 participants
Now, error fields will always have 'type' and 'reason' fields, and the information in those fields is the same regardless of whether the output is detailed or not. This also reverses the test changes in #116330, due to the deprecation warning not (generally) being there anymore.
This fixes #89387