[FEATURE] Raise errors for HTTP error codes in the generic client

[dblock]

Is your feature request related to a problem?

Coming from #910 (comment) I propose we raise errors on non-2xx error codes from the server for the generic client just like in the rest of the java client.

What solution would you like?

Currently:

// compare with what the low level client outputs
try (Response response = javaClient().generic().execute(...)) {
   if (response.getStatus() != 200) {
      // Request was not successful
   }
}

Proposed:

// compare with what the low level client outputs
try (Response response = javaClient().generic().execute(...)) {
   // response.getStatus() is always 2xx
} catch(GenericException e) {
  // e.response is a Response
  // e.getStatus() != 2xx
}

What alternatives have you considered?

// compare with what the low level client outputs
try (Response response = javaClient().generic().execute(...)) {
   response.checkErrors() // raises when not 2xx
}

Do you have any additional context?

Let's discuss/implement/close/reject this issue before #926

[dblock]

@reta

[reta]

@dblock I would really want to understand why we should do that, in particular what would be the reason to use generic HTTP client vs typed client - if both would do the same thing? (at least with respect to exception handling). We should not forget that, if we decided to pursue this path, the exception should include the response body + status (at least), otherwise we are missing important details.

[dblock]

1. Users have asked for a generic client because they can invoke APIs that don't have higher level client support yet, not for more control of error handling.
2. It's less code (easier) to use a client that handles errors for the users.
3. Users will be surprised by the fact that the generic client doesn't behave the same way as the non-generic client.
4. Users that want to handle errors in specific ways will be fewer than users that don't, and those who do can catch and handle errors the way they want.
5. As is one cannot easily mix code that uses a generic and a non-generic client because the generic client requires special error handling.
6. All other generic clients in opensearch-py, etc., behave the same way as the non-generic versions of those clients as far as error handling is concerned.

[dblock]

the exception should include the response body + status (at least), otherwise we are missing important details.

I think the exception should include the entire .response the same as the response currently returned. We're just discussing whether to throw a GenericClientException or similar on non-2xx error codes.

[reta]

Fair points, thanks @dblock, I think we could accommodate both favours while building the client, very roughly something like that:

javaClient().generic(Options.throwOnHttpErrors())

[dblock]

Fair points, thanks @dblock, I think we could accommodate both favours while building the client, very roughly something like that:

javaClient().generic(Options.throwOnHttpErrors())

Yes, it's certainly acceptable! That's my alternate option. It's still not my preferred, but I would be ok with it. Want to try to PR it?

[andrross]

My general expectation if using an untyped client in Java is that if the server returned a response (regardless of HTTP status code) then I will get that response without having to unwrap exceptions, and an exception would only model cases where the request can't be sent or a response is never received. I fully admit I may be in the minority here.

[reta]

I fully admit I may be in the minority here.

We are :D But it is good to provide options ....