Adjust exception wrapping logic when converting to ES exceptions

[benwtrent]

This is a broad brush to address #131548

It seemed weird to only address strange wrapping in only one place (the bitset logic). However, I admit, this is a pretty huge change.

There are many unknowns, so maybe this just isn't worth it. But it does seem weird that a Java Exception will have an ElasticsearchException as a cause and we would WANT to rather return the java exception rather than the inner ElasticsearchException...

//cc @javanna what do you think? Maybe we add a new "convertOrUnwrapToElastic" method and switch over good paths to that?

[javanna]

Thanks for proposing this fix @benwtrent !

The change isn't that huge after all, reviewing the usages of ExceptionsHelper#convertToElastic.

I wonder if there's any case where the wrapping with a non ElasticsearchException is useful to be honest, which I guess is what led you to opening this PR.

A more contained way to address this would be to have specific unwrapping for ExecutionException, where I am 100% sure the wrapping is never useful. Seeing tests green gives me confidence though about your approach.

A couple more doubts, beyond your PR, that may or may not affect your approach or confidence on it:

• I wonder why we need this wrapping in e.g. field data loading code and very specific places to be honest. There may be additional cleanups possible in that area to even reduce the surface of the calls made to convertToElastic? The pattern there is to catch a generic Exception, where no checked exceptions are even potentially thrown.

• in the few places where we catch a IOException, it seems a bit lazy to call convertToElastic perhaps? We could directly wrap it?

All in all I am starting to question even the need for this method somehow...

[benwtrent]

closing for now

[javanna]

Do you have a different plan @benwtrent ? I think it was an ok direction or at least some initial step to discuss.