Ragmaanir/soegen

Nil trace in request.body attribute

ferblape opened this issue · 3 comments

Hi,

I'm quite newbie in Crystal, so first of all apologies if the issue I'm going to describe is not very accurate. I'm using Soegen together with Kemal to create an small API to a ElasticSearch and I'm getting the following error when compiling my program:

in ./libs/soegen/soegen/server.cr:83: instantiating 'log_request(Soegen::CompletedRequest, Soegen::Timing)'
      log_request(response, timing)
      ^~~~~~~~~~~

in ./libs/soegen/soegen/server.cr:98: instantiating 'to_curl(HTTP::Request)'
        to_curl(response.request)
        ^~~~~~~

in ./libs/soegen/soegen/server.cr:114: undefined method 'empty?' for Nil (compile-time type is String?)
      body = if !request.body.empty?
                              ^~~~~~

================================================================================

Nil trace:
  macro getter (in /usr/local/Cellar/crystal-lang/0.9.1/src/object.cr:208):4
          def body
              ^~~~
  macro getter (in /usr/local/Cellar/crystal-lang/0.9.1/src/object.cr:208):5
            @body
            ^~~~~
  /usr/local/Cellar/crystal-lang/0.9.1/src/http/request.cr:11
      def initialize(@method : String, @resource, @headers = Headers.new : Headers, @body = nil, @version = "HTTP/1.1")
                                                                                    ^
  /usr/local/Cellar/crystal-lang/0.9.1/src/http/request.cr:11
      def initialize(@method : String, @resource, @headers = Headers.new : Headers, @body = nil, @version = "HTTP/1.1")
                                                                                    ^~~~
  /usr/local/Cellar/crystal-lang/0.9.1/src/http/request.cr:11
      def initialize(@method : String, @resource, @headers = Headers.new : Headers, @body = nil, @version = "HTTP/1.1")

It looks like request.body is not nil by default and on server.cr:114 the code is not dealing with Nil strings. I made a quick patch that works, to convert ti to empty string:

request_body = request.body || ""
body = if !request_body.empty?
  "-d '#{request_body}'"
else
  ""
end

If you agree about the problem I can try to send a PR for you to review it.

Hi,

sorry for my late response, i didn't see the notification. Thank you for your report. Could you show me the line in your code that triggers this compilation error? I am not sure yet how this error is triggered. Are we using the same crystal version?
Mine is: Crystal 0.9.1 [b3b1223](Fri Oct 30 03:31:36 UTC 2015)

Hi again, I have created this repo with a demo program.

There are two source files:

  • src/query_engine.cr which is just a class to talk to Elastic Search
  • src/server.cr which is a HTTP endpoint that talks to Elastic Search and returns the the hits as a JSON

If you run $ crystal build -o bin/test src/server.cr you'll see the error I was mentioning above. The error happens in https://github.com/ferblape/soegen-nil-trace/blob/master/src/query_engine.cr#L31

In https://github.com/ferblape/soegen-nil-trace/blob/master/src/query_engine.cr#L1-L8 I have included some lines to create the index I'm using for the demo.

Thanks for your time.

Thank you for your demo program. The problem is indeed as you mentioned that the body of the HTTP::Request can be nil. My test suite did not catch this error because it never instantiated HTTP::Request with the body being nil. But kemal does. The combination of requiring both soegen and kemal therefore leads to this compilation error. Fix will be pushed in a second.