CrowdHailer / raxx

Interface for HTTP webservers, frameworks and clients
https://hexdocs.pm/raxx
Apache License 2.0
401 stars 29 forks source link

Dialyzer warnings when returning non-trivial responses from handle_request #135

Closed nietaki closed 5 years ago

nietaki commented 5 years ago

I'm trying to write a clean project using Ace/raxx and streaming uploads/download, including no dialyzer errors. I seem to have found a correct and working usage of Raxx that trips up dialyzer. See simplified example:

defmodule FooBar do
  use Raxx.Server

  def handle_request(request = %{path: _}, _state) do
    {[set_body(response(:ok), true)], :state}
  end

  def handle_info(_info, _state) do
      {[tail()], :state}
  end
end

(Full repro here: https://github.com/nietaki/file_beam/blob/dialyzer-repro/lib/file_beam/www/download.ex#L27 )

It produces the following dialyzer errors:

lib/file_beam/www/download.ex:3: The pattern #{'body':='true'} can never match the type {[#{'__struct__':='Elixir.Raxx.Response', 'body':=boolean() | binary(), 'headers':=[any()], 'status':=integer()},...],'state'}
lib/file_beam/www/download.ex:3: The pattern #{'body':='true'} can never match the type {[#{'__struct__':='Elixir.Raxx.Response', 'body':=boolean() | binary(), 'headers':=[any()], 'status':=integer()},...],'state'}

I double-checked the typespecs in Raxx and I'm still convinced I'm returning the right typed values. I suspect the warning comes from this line: https://github.com/CrowdHailer/raxx/blob/master/lib/raxx/not_found.ex#L52 , but I'm not sure what the best workaround would be (apart from removing the case from the code).

As a sidenote, it surprised me a little that the dispatch from handle_{head | data | tail} to the user's handle_request is done in the Raxx.NotFound module, which I didn't expect to be involved at all :thinking:

nietaki commented 5 years ago

If it is indeed the "this case clause will never match" sort of situation, it can probably be tackled in a similar way as it is in OK: https://github.com/CrowdHailer/OK/blob/master/lib/ok.ex#L419-L427

CrowdHailer commented 5 years ago

I would be interested to see if this issue still occurs once upgraded to using Raxx.SimpleServer

I think you identified the correct line in the NotFound code but that is not used anymore

nietaki commented 5 years ago

Thanks for looking into it, I'll have a look if I can still repro it this weekend.

nietaki commented 5 years ago

After updating Ace to 0.18 and changing handle_request() to handle_head() the dialyzer now reports clean.

I don't think you can do download streaming using Raxx.SimpleServer (as per https://hexdocs.pm/raxx/readme.html#http-streaming), I think this covers the case and can be closed :+1: