search

CrowdHailer / raxx

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

On rare occasions we see this error: `Elixir.RuntimeError: Headers should not be duplicated` #98

Closed samsondav closed 6 years ago

samsondav commented 6 years ago

Stacktrace is here:

Elixir.RuntimeError: Headers should not be duplicated
  File "lib/raxx.ex", line 303, in Raxx.set_header/3
  File "lib/enum.ex", line 1899, in Enum."-reduce/3-lists^foldl/2-0-"/3
  File "lib/ace/http1/parser.ex", line 128, in Ace.HTTP1.Parser.pop_part/1
  File "lib/ace/http1/parser.ex", line 49, in Ace.HTTP1.Parser.parse/3
  File "lib/ace/http1/endpoint.ex", line 27, in Ace.HTTP1.Endpoint.handle_info/2
  File "gen_server.erl", line 616, in :gen_server.try_dispatch/4
  File "gen_server.erl", line 686, in :gen_server.handle_msg/6
  File "gen_server.erl", line 636, in :gen_server.try_handle_call/4
  Module "Elixir.Ace.HTTP.Server", in Ace.HTTP.Server.init/1

Have seen this two or three times. Doesn't appear to be correlated with anything.

I am completely sure we aren't setting headers more than once in our code. Is there some logging or tracing we can add that might help debug this if it happens again?

CrowdHailer commented 6 years ago

Looking at the stacktrace I think the fix for this will probably end up in Ace.

I still thinking testing what happens with duplicate headers would be a good idea to rule it out in cases where anyone else sees it.

On the logging, sure. Ace could log at debug level the packets that arrive for HTTP/1 (The HTTP/2 implementation already logs each frame for debug)

CrowdHailer commented 6 years ago

I think because the vast majority of headers are submitted singly the best thing would be to concatenate the headers that are duplicated. for all headers I think this means join by , except cookie that are joined by ;

https://tools.ietf.org/html/rfc7540#section-8.1.2.5

varnerac commented 6 years ago

Can't we just return them as proplists and let users decide how to handle it? Looking back at how query was parsed, I think we just use the existing key/value lists. https://github.com/CrowdHailer/raxx/blob/master/lib/raxx.ex#L27-L35

CrowdHailer commented 6 years ago

I think that would work just as well. There were plans to put the headers in a map so they were easier to match on but I think matching on headers is not something i'm worried about loosing

varnerac commented 6 years ago

https://github.com/CrowdHailer/raxx/pull/113

CrowdHailer commented 6 years ago

closed by https://github.com/CrowdHailer/Ace/pull/107