GetMany returns empty list after connection failure

[srgclr]

NEST/Elasticsearch.Net version: 5.5.0
Elasticsearch version: 5.5.1

If I start with either elasticsearch down or simulated connection failures (using Fiddler to auto respond with 502) I'll get a PipelineException on the first GetMany. If I start with a working cluster, which succeeds on the first call, and then close the connection or kill elastic before the second call, it will not throw an exception but return empty list instead.

The low level client returns an unsuccessful response in the second case which is good but the inconsistency in throwing was causing us to miss exceptions if the access to the cluster becomes unstable.

I know exceptions are off in the connection string but returning an empty list is very misleading?

Thanks.

Steps to reproduce:

var serverLocation = "localhost:9200";
var uri = new Uri("http://" + serverLocation);
var sniffingConnectionPool = new SniffingConnectionPool(new List<Uri>() { uri });
var connectionSettings = new ConnectionSettings(sniffingConnectionPool);

connectionSettings
                .DisableDirectStreaming()
                .SniffOnConnectionFault()
                .SniffLifeSpan(TimeSpan.FromSeconds(300));

var client = new ElasticClient(connectionSettings);
var ids = new List<string> { "123" };
var postData = new Dictionary<string, IEnumerable<string>> { { "ids", ids } };

var index = "index";
var type = "type";

IEnumerable<IMultiGetHit<object>> response1 = client.GetMany<object>(ids, index, type);

// stop elastic 
Thread.Sleep(5000);

IEnumerable<IMultiGetHit<object>> response2 = client.GetMany<object>(ids, index, type);

[russcam]

What does response2.IsValid return, and response2.DebugInformation?

[srgclr]

Hi @russcam,

That's part of the problem. GetMany returns an enumerable, not a response object. I've updated the code above for clarity.

And that's why I think this should throw an exception as otherwise we cannot differentiate between no items and a failed connection.

[russcam]

I see the issue @srgclr. GetMany<T> is a helper extension but the implementation doesn't expose important details about the response from the actual API method used.

And that's why I think this should throw an exception as otherwise we cannot differentiate between no items and a failed connection.

The client generally doesn't throw exceptions* (there are exceptions to this!) unless configured to with ThrowExceptions() on ConnectionSettings. I'd suggest using

IMultiGetResponse response = client.MultiGet(s => s.GetMany<T>(ids).Index(index).Type(type));
var items = response.GetMany<T>(ids);

in this case (which is what the helper does) as response will provide IsValid and DebugInformation.

I'm thinking that we should probably remove this abstraction in 6.0.0 GA. Thoughts @Mpdreamz, @codebrain ?

[srgclr]

Cool, thanks for the suggestion @russcam. We've sorted it out eventually but were caught off guard by the inconsistency.

[Mpdreamz]

@russcam good question, i think we need to resolve this before GA.

I think a solution would be to forcefully flip ThrowExceptions() on for these kinds of extension methods. Removing would break too many people i'm afraid.

[Mpdreamz]

This will be fixed in 6.x onwards as per #2987

Thanks for raising this @srgclr 👍