jillesvangurp/kt-search

[FEAT] Ability to get errors

Closed this issue · 3 comments

Describe the enhancement

I would like the Status of the request easily available. If it is already easily available, I would like to see it in the docs. I would also like to see tests that fail intentionally and check to see if it fails.

Why is this needed?

In the previous version of the library the results returned were the objects from the ElasticSearch lib (IndexResponse etc.). These objects have the public RestStatus status() function. This allows me to check if my request was successful.

I have been staring at it for a while and I think DocumentIndexResponse.result might be the place to check.
Or maybe it is in DocumentIndexResponse.shards.successful?
Or DocumentIndexResponse.shards.failed?
Why are these Int fields?

if (response.status().equals(RestStatus.CREATED) || response.status().equals(RestStatus.OK)) {
    return Ok(uuid)
}

How do you think it should be done?

I thought maybe a try/catch, but everything is wrapped in Result types, which is great! But then the Result type is unwound and put into the DocumentIndexResponse so I can't tell if it was successful or not.

Hopefully its just some documentation. Or maybe I'll figure this out by myself with what is available. Thanks.

The old client is no longer supported because the underlying java client that came with the Elastic Java Rest High Level client has been deprecated.

The new client provides it's own Rest client abstraction which uses the RestResponse sealed class to represent the different responses that can come back with different http status codes.

Most of the client functions either return the expected 2xx style parsed response or throw an exception. So there is no need to check the response for a status code because you either get the response object or an exception is thrown. In some cases where e.g. a 404 is expected for non existing documents, the response is nullable.

In case of an exception, you can get the wrapped response (which is now exposed as a val) or the status code to check either the type or the integer rest status code.

But thanks for reporting this. I actually updated the documentation a bit and added a section on error handling. Please have a look at this: https://jillesvangurp.github.io/kt-search/manual/ClientConfiguration.html.

I am a kind of person who really needs to see examples. This is what I had:

        val uuid = UUID.randomUUID()
        val indexResponse = searchClient.indexDocument(
            target = "my-index",
            document = """{"some":"stuff"}""",
            id = uuid.toString()
        )
        if (indexResponse.shards.successful == 1) {
            return Ok(uuid)
        }
        return Err(indexResponse)

But with a try/catch, would I be doing something like this instead?

        val uuid = UUID.randomUUID()
        try {
            val indexResponse = searchClient.indexDocument(
                target = "my-index",
                document = """{"some":"stuff"}""",
                id = uuid.toString()
            )
            return Ok(uuid)
        } catch (e: Exception) {
            return Err(e)
        }

Thanks again.

You can catch a RestException instead of an Exception. But something like that yes. I would do this centrally and not all over the place.

Mostly what goes wrong with this type of logic falls into two categories:

  • validation and other bugs; which means you fix your code and the error goes away. An exception means you messed up. Basically any 3xx or 4xx status means "I messed up somewhere". I tend to treat these as bugs and log at error level when it happens. My logging system screams loudly and then I fix it.
  • networking issuess or other cluster issues beyond your control (you can retry but generally not recover in a sane way). Basically any 5xx error. Usually means there is some operational issue that needs attention.