Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print complete response body along with response code for BadResponse… #894

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

amitavmohanty01
Copy link

Print complete response body along with response code for BadResponseCodeError.

When a BadResponseCodeError happens, the response body tells the user more about the issue and can indicate how to fix it. For example, when a template is applied from Logstash to Elasticsearch, a BadResponseCodeError does not explain much but the body can tell where the template has an issue.

A more elaborate approach would be to print this only for a certain log level but it is not a log line. The caller is calling message method so there is a need of passing parameters to indicate required verbosity.

Thanks for contributing to Logstash! If you haven't already signed our CLA, here's a handy link: https://www.elastic.co/contributor-agreement/

@amitavmohanty01
Copy link
Author

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself. is this related to my code ?

@amitavmohanty01
Copy link
Author

@jsvd please take a look and suggest whether the PR will be useful.

@amitavmohanty01
Copy link
Author

Failures:

  1) LogStash::Outputs::ElasticSearch::HttpClient Host/URL Parsing an ipv6 host should properly transform a host:port string to a URL
     Failure/Error: let(:hostname_port_uri) { ::LogStash::Util::SafeURI.new("//#{hostname_port}") }
     
     URI::InvalidURIError:
       bad URI(is not URI?): //[::1]:9202
     Shared Example Group: "proper host handling" called from ./spec/unit/outputs/elasticsearch/http_client_spec.rb:123
     # ./spec/unit/outputs/elasticsearch/http_client_spec.rb:29:in `block in hostname_port_uri'
     # ./spec/unit/outputs/elasticsearch/http_client_spec.rb:36:in `block in <main>'

Finished in 27.63 seconds (files took 4.53 seconds to load)
74 examples, 1 failure

Failed examples:

rspec ./spec/unit/outputs/elasticsearch/http_client_spec.rb[1:1:3:1] # LogStash::Outputs::ElasticSearch::HttpClient Host/URL Parsing an ipv6 host should properly transform a host:port string to a URL

It seems I am getting this error on my branch as well as on master.

@LuckyWindsck
Copy link

I think that this is necessary for me. I get the following message:

"Got response code '400' contacting Elasticsearch at URL 'http://elasticsearch:9200/_template/my_template'"

I was not familiar with the status code 400, and tried to debug this for a long time. Maybe it's easier for user to debug by having a more complete message.

@LuckyWindsck
Copy link

@amitavmohanty01

I found that currently (v10.5.1), we have four instance variables, @response_code, @url, @request_body, @response_body.

Do you think that users need @request_body for easier debugging?

@amitavmohanty01
Copy link
Author

@LuckyWindsck my only worry is request body might become too big and spam log files.

@amitavmohanty01
Copy link
Author

@karenzone please take a look. Let me know if you have any suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants