Skip to content

Conversation

@philkra
Copy link
Contributor

@philkra philkra commented Dec 22, 2021

as titled.

Copy link
Member

@delvedor delvedor left a comment

Choose a reason for hiding this comment

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

@swallez can you take a look at this one? You added this change in #1034.

Copy link
Contributor

@swallez swallez left a comment

Choose a reason for hiding this comment

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

Looking at the reference material (code + docs), this change seems wrong. What are the recordings that led to it?

/** @codegen_names result, failure */
export type ResponseItem<TDocument> =
| MultiSearchItem<TDocument>
| SearchResponse<TDocument>
Copy link
Contributor

Choose a reason for hiding this comment

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

We need MultiSearchItem as multisearch response items have a status field that indicates the result of each search request. This is present in the ES source code and explained in the docs.

// This should never happen in clients, because we assume we will never send malformed request.
error: ErrorCause
status: integer
status?: integer
Copy link
Contributor

Choose a reason for hiding this comment

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

Status is always present in error responses, including in multisearch responses. See ES code and the docs "an object with error message and corresponding status code will be returned in place of the actual search response".

Keeping error and status required also plays an important role (in the Java client) to disambiguate some non-200 results where ES will return either a valid response or an error depending on the parameters (e.g. delete document that returns 404+error if the index doesn't exist but 404+valid response if the index exist but not the document).

@philkra
Copy link
Contributor Author

philkra commented Jan 25, 2022

Closing this in favour of #1320

@philkra philkra closed this Jan 25, 2022
@philkra philkra deleted the global-001 branch January 25, 2022 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment