IndexMany() does not throw exception even though Index() does

[nphmuller]

NEST/Elasticsearch.Net version: 5.0.0-rc1

Elasticsearch version: 5.0.0

Description of the problem including expected versus actual behavior:
When configuring ThrowExceptions as true on the ElasticClient's connection settings, it's expected that the client throws an exception if a call fails. When running IndexMany(), this however doesn't happen. What does happen is that a BulkResponse instance is returned with the Errors property as True and IsValid as False. The OriginalException and the ServerError properties are both null, however.

When running Index() with the first object of the IndexMany input array the exception is thrown properly.

Steps to reproduce:

// Setup elastic
const string indexName = "crashdummy";
var client = new ElasticClient(new ConnectionSettings().ThrowExceptions().DefaultIndex(indexName));
if (client.IndexExists(indexName).Exists) client.DeleteIndex(indexName);
client.CreateIndex(indexName, index => index.Settings(s => s.Setting("index.mapper.dynamic", false)));

// Setup entity
dynamic entity = new System.Dynamic.ExpandoObject();
entity.Id = 1;
entity.Name = "Test";

// Relevant calls
var bulkResult = client.IndexMany(new[] { entity }); // Doesn't throw an exception.
var singleResult = client.Index(entity); // Throws an exception.

Details of BulkResponse instance returned by IndexMany():
ApiCall.HttpStatusCode: 200

DebugInformation:

Invalid NEST response built from a successful low level call on POST: /_bulk?pretty=true
# Invalid Bulk items:
  operation[0]: index returned 400 _index: [my_type]: id: 2 _version: 0 error: Type: mapper_parsing_exception Reason: "failed to parse [XXX]" CausedBy: "Nest.CausedBy"
  operation[1]: index returned 400 _index: [my_type]: id: 20 _version: 0 error: Type: mapper_parsing_exception Reason: "failed to parse [XXX]" CausedBy: "Nest.CausedBy"
[...]
# Audit trail of this API call:
 - [1] HealthyResponse: Node: http://127.0.0.1:9200/ Took: 00:00:00.8225922
# Request:
<Request stream not captured or already read to completion by serializer. Set DisableDirectStreaming() on ConnectionSettings to force it to be set on the response.>
# Response:
<Response stream not captured or already read to completion by serializer. Set DisableDirectStreaming() on ConnectionSettings to force it to be set on the response.>

[russcam]

@nphmuller this is the intentional/correct behaviour; the bulk call response is 200 so the call itself has not failed or resulted in an exception, however some of the items may have errors, indicated by the .IsValid property and the errors collection.

[nphmuller]

Yeah, I kind of thought that it would be.

My own use case for the ThrowsExceptions option is to catch everything that did not go as expected, without any manual checks. So no exception would mean for me that everything succeeded. And while the bulk request itself did succeed, it did not succeed as expected because one or more items gave a, for example, 400 result.

Entity Framework for example solves this with an exception that contains an array of all the errors. Nest's version could be an exception with an array of items that failed, with their errors.

Just my two cents. In the end it's your call of course! :)

I've made an extension method for the bulk methods, as a workaround. It checks for errors and throws an exception if any items failed.

[gmarz]

We've discussed this in the past and decided to keep the existing behavior. See #2153 for details.

I've made an extension method for the bulk methods, as a workaround. It checks for errors and throws an exception if any items failed.

👍