Add metadata to ShardOperationFailedException

[DaveCTurner]

... (the implementations thereof) rather than implementing ToXContent directly. This should reduce the number of ways that errors are reported to clients.

This was suggested as part of #27672 but worthy of another discussion next week as this particular design choice is very well-established and may not be worth changing.

[DaveCTurner]

We discussed this in FixItFriday and the main question was whether this'd have any effect on clients of the API or whether the effect would just be on our code. Could @cbuescher and/or @tlrx investigate the impact of this and form a view about whether we want to do it or not?

[tlrx]

Sorry to come back so late!

I'm not sure to understand the investigation you're asking for: ShardOperationFailedException are different from the exceptions listed in #27672 as it is not an exception but instead it represents a shard failure with a potential exception as the reason/cause of the failure. To me ShardOperationFailure would be a better name. I agree they are rendered at the REST level in a similar manner than the exceptions, but they are containers of exceptions, not the underlying exception that is rendered using ElasticsearchException.generateThrowableXContent() in failures.

[jaymode]

@tlrx @DaveCTurner do you think we should add metadata and remove the ToXContent implementation within these exceptions? If not, can we close this issue?

[DaveCTurner]

I can believe it would be better for clients for all operations that fail on a shard to report themselves with a consistent XContent structure (i.e. shard/index/node in standard fields). However I don't really think we need to keep this open independently of #27672 so I am closing this.