search

kairosdb / kairosdb-client

Java Client for KairosDB
65 stars 67 forks source link

do not process content if NO_CONTENT response was received #74

Closed sspieker closed 6 years ago

sspieker commented 6 years ago

I'm testing the new client version 3.0.0 and stumbled upon a problem when pushing metrics.

Environment

First of all, one important information: I'm not connecting directly to a K* instance. Instead there's some kind of gateway we've implemented in order to be able to do some more stuff with ingested metrics, i.e. analyze them seperately. Thus, the gateway is producing the response here.

Problem

Now, here's the thing: the client checks the response content even if the response status code is 204 (NO_CONTENT). IMHO, this doesn't make sense; furthermore, this leads to exceptions in our code because the gateway correctly sends 204 without content. Again: this problem might not orrur when directly communicating with a K* instance because it sends content. But strictly speaking it just doesn't make sense...

Proposal

With this pull request's changes, the client simply returns from the handle method if the response status code is 204, effectively skipping any content evaluation. There may be a better solution for this, but it works for me right now.

jsabin commented 6 years ago

I thought I had taken care of this case with

if (response.getInputStream() == null)

I don't understand how you can have anything in the stream if there is no content. Odd. Thanks for pull request. I will take a look at it.

jsabin commented 6 years ago

I see. I misread your comment. You are saying that the gateway is returningt 204 but has content. Hmm. That's just wrong.

jsabin commented 6 years ago

Fixed in commit 3b44aa508c6b2ca722f40565fef3316f178bf8ea.

sspieker commented 6 years ago

@jsabin First of all, thank you for merging and adding a test for this! That solves that problem right away.

There still is something left to clarify:

You are saying that the gateway is returningt 204 but has content. Hmm. That's just wrong.

In fact, it's the other way around: Our gateway is returning 204 (well, it's forwarding the response status code from K* actually), without any further investigation of the response body, thus not sending any content.

K itself is not adding any content in it's "post datapoints" method as well, if there are no errors: MetricsResource.java#L302 But I believe, K actually IS sending json content, but that's just empty (@ Produces json), and that's why your original code was working nevertheless. There really is content there, even though an empty one. But nevertheless, not "null" so to speak. ;)

To sum it all up: why should the content of a 204 (no content) response be further investigated at all? That's the gist of this code change.