ElasticClient.BulkAsync Bug - Empty POST

[peske]

NEST/Elasticsearch.Net version: 2.2.0

Elasticsearch version: The latest (but not important)

Description of the problem including expected versus actual behavior:
The problem (bug) is in behavior of ElasticClient.BulkAsync method. (Probably with sync version also - I haven't tried.) And the problem is again followed by misleading exceptions thrown by Nest. If you forget to supply an argument to mentioned method - it will try to "bulk" nothing, for example:

ElasticClient client;// initialized and works well

IBulkResponse response = await client.BulkAsync().ConfigureAwait(false);

Awaited call from the previous snippet will throw an exception:
System.Net.WebException: The remote server returned an error: (400) Bad Request.
ServerError property will contain:
400Type: parse_exception Reason: "Failed to derive xcontent"
And probably the funniest thing is that DebugInformation property, when accessed will trow its own exception: System.ArgumentNullException: Value cannot be null.

Due to "bad request" error, the first thing that crossed my mind was that some of the documents are not serialized well, so I've jumped to Fiddler, and I was surprised to see an empty request:

POST http://es.myserver.com:9200/_bulk HTTP/1.1
Accept: application/json
Content-Type: application/json
Host: es.myserver.com:9200
Content-Length: 0

And I've spent about half an hour more debugging and checking the documents I've wanted to index, until I've finally realized that BulkDescriptor instance that I've created correctly is actually never sent to BulkAsync method. Well, it is my mistake, but:

1. I'm not sure if there's a reason for having parameterless BulkAsync overload? (My logic says that it is meaningless, but I'm only few-days beginner with Nest so maybe I'm wrong.)
2. I'm sure that there's no meaningful reason to actually send an empty bulk request to the server, so it is a bug.
3. The method has to throw meaningful exception (i.e. "Nothing to send", "IBulkRequest missing", or something like that)
4. Property that is meant to provide help in troubleshooting (DebugInformation) must not throw an exception, thus adding to confusion. This is another bug.

To conclude: There are a lot of problems with Nest when it comes to troubleshooting, an they are caused (besides the lack of proper documentation and examples) by poor, misleading exceptions.

Steps to reproduce:
1.Described above

Provide DebugInformation (if relevant):

[gmarz]

@peske thank you for reporting. You're right that bulk should never be parameterless. I've opened the above PR to fix this- we now throw an ArgumentNullException if null is passed, since we can't change the signature for backwards compatibility reasons. I've also fixed the NRE in DebugInformation.

[gmarz]

Closed via #2055

[peske]

Thanks for the answer! But I admit that I'm confused. If the only reason for empty BulkAsync signature is backward compatibility - it is not reason at all. If the method will always throw ArgumentNullException how it can be backward compatible? I mean it will allow compilation but it will always throw an exception at runtime, right? It is always better to have compile-time errors than run-time ones.

[gmarz]

It is always better to have compile-time errors than run-time ones.

Agreed, but since our assemblies are strong-named, we have to maintain strict binary compatibility and not introduce any binary-level (compile time) breaks (to support things like assembly redirects).

Throwing the exception is indeed a run time error, but hopefully makes the problem much more obvious.