benoitc / hackney

simple HTTP client in Erlang
Other
1.34k stars 427 forks source link

hparser error in state on_trailers #638

Closed xadhoom closed 3 years ago

xadhoom commented 4 years ago

Updating from hackney 1.15.2 to 1.16.0 http connections against a microhttpd-powered process randomly* fails with error (elixir syntax here)

  {error: {:more,
   {:hparser, :response, 4096, 10, 0, :on_trailers, "", {1, 1}, "", [],
    :undefined, "chunked", "keep-alive", "application/json", "", :done}}}

Bisecting seems that the error appeared with commit https://github.com/benoitc/hackney/commit/78ce833a88715b195a045c6c949dca6ea1dc7d7a .

As additional info, if we force to not use keep-alive (by adding the header Connection: close on hackney) everything works ok.

Any idea on where to look?

* randomly means that we need to run several requests to get the error, sometimes 100 rqs are ok in a row, sometime we get 1 error in those 100, other times 6 errors and so on. Is reproducible but does not have a fixed pattern. Right now we're not been able to reproduce it against external services (google, our own apache/nginx servers, etc)

lukaszsamson commented 4 years ago

I'm seeing a similar error all the time since upgrade from 1.15.2 to 1.16.0 (otp 22.3.4) I'm using Connection: keep-alive

{:error, %HTTPoison.Error{id: nil, reason: {:more, {:hparser, :response, 4096, 10, 0, :on_trailers, "\r", {1, 1}, "", [], :undefined, "chunked", "", "application/json", "", :done}}}}
sebastian-schuth commented 4 years ago

Same for us - the error pops up seemingly randomly. The error message is exactly matching the one @lukaszsamson has posted. We call services written in Ruby on Rails, and the problem pops up with multiple of them. I assume (not verified) that they are all using Connection: keep-alive, too.

I am not sure if I am hijacking another issue, as the error does not match 100% the original problem described in this issue.

We are on hackney 1.16, too.

rozap commented 4 years ago

I think i've fixed this. https://github.com/benoitc/hackney/pull/641

cjbottaro commented 4 years ago

Can this be merged in and a release cut? I can 100% repo the issue with this response:

# curl -i "http://ia-web/api/v0/swccd/build_automatch_query" -XPOST -d '{"opportunity_id": 3093}'
HTTP/1.1 200 OK
X-Frame-Options: SAMEORIGIN
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
Content-Type: application/json; charset=utf-8
ETag: W/"86371c7746d599e12538d25a503f219a"
Cache-Control: max-age=0, private, must-revalidate
X-Request-Id: 4624fc82-bf1e-4799-9282-7352f5929555
X-Runtime: 0.066514
Transfer-Encoding: chunked

{"bool":{"must":{"term":{"_type":{"value":"Application"}}},"should":[{"bool":{"filter":[{"terms":{"opportunity_id":[3032]}},{"term":{"vam_effective_end_at":"20180306"}},{"term":{"vam_archive_at":"20180814"}},{"range":{"created_at":{"lt":"2018-03-07T07:59:59.999Z"}}},{"bool":{"minimum_should_match":1,"should":[{"bool":{"filter":[{"range":{"vam_form_field_30":{"gte":"2.8"}}},{"range":{"vam_form_field_271:submitted":{"gte":1}}},{"range":{"vam_form_field_33":{"gte":"12"}}},{"range":{"vam_form_field_34":{"gte":"9"}}},{"term":{"vam_form_field_224":"nursing - a.s."}},{"term":{"form_field_55":"yes"}}]}},{"bool":{"filter":[{"term":{"form_field_55":"yes"}},{"term":{"vam_form_field_397":"nursing - a.s."}},{"range":{"vam_form_field_33":{"gte":"12"}}},{"range":{"vam_form_field_271:submitted":{"gte":1}}},{"range":{"vam_form_field_30":{"gte":"2.8"}}},{"range":{"vam_form_field_34":{"gte":"9"}}}]}}]}}],"should":[],"must":[{"bool":{"filter":{"match_all":{}},"minimum_should_match":0,"should":[{"constant_score":{"filter":{"bool":{"filter":[{"range":{"form_field_30":{"gte":"2.8"}}},{"range":{"form_field_271:submitted":{"gte":1}}},{"range":{"form_field_33":{"gte":"12"}}},{"range":{"form_field_34":{"gte":"9"}}},{"term":{"form_field_224":"nursing - a.s."}},{"term":{"form_field_55":"yes"}}],"_name":"qg_1005"}}}},{"constant_score":{"filter":{"bool":{"filter":[{"term":{"form_field_55":"yes"}},{"term":{"form_field_397":"nursing - a.s."}},{"range":{"form_field_33":{"gte":"12"}}},{"range":{"form_field_271:submitted":{"gte":1}}},{"range":{"form_field_30":{"gte":"2.8"}}},{"range":{"form_field_34":{"gte":"9"}}}],"_name":"qg_1143"}}}}]}}],"must_not":[{"term":{"actualized_by":3093}},{"term":{"vam_category_state":"drafted"}}]}},{"bool":{"filter":[{"term":{"opportunity_id":3093}}],"must":[{"bool":{"filter":{"match_all":{}},"minimum_should_match":0,"should":[{"constant_score":{"filter":{"bool":{"filter":[{"range":{"form_field_30":{"gte":"2.8"}}},{"range":{"form_field_271:submitted":{"gte":1}}},{"range":{"form_field_33":{"gte":"12"}}},{"range":{"form_field_34":{"gte":"9"}}},{"term":{"form_field_224":"nursing - a.s."}},{"term":{"form_field_55":"yes"}}],"_name":"qg_1005"}}}},{"constant_score":{"filter":{"bool":{"filter":[{"term":{"form_field_55":"yes"}},{"term":{"form_field_397":"nursing - a.s."}},{"range":{"form_field_33":{"gte":"12"}}},{"range":{"form_field_271:submitted":{"gte":1}}},{"range":{"form_field_30":{"gte":"2.8"}}},{"range":{"form_field_34":{"gte":"9"}}}],"_name":"qg_1143"}}}}]}}],"should":[],"must_not":[]}}],"minimum_should_match":1}}

Thanks!

vasumur commented 4 years ago

Appreciate any update on when this fix will be bundled into 1.16.x version. Thanks

benoitc commented 4 years ago

there is a release planned sometimes this week that should include it.

LukasKnuth commented 4 years ago

@benoitc Any update on the release? Downgrading to 1.15.2 is hard for us because of the OTP 23 incompatibility, so this is somewhat critical.

chaitradixit-15 commented 4 years ago

Hi @benoitc ,

We are facing the same issue in our project as well where randomly we are getting this error.

%HTTPoison.Error{

id: nil,

reason: {:more,

{:hparser, :response, 4096, 10, 0, :on_trailers, "", {1, 1}, "", [],

 :undefined, "chunked", "", "application/json;charset=utf-8", "", :done}}

}

As suggested we have downgraded to 1.15.2 and it seems to be working fine. Any update on when we will get the release with the fix?

vasumur commented 3 years ago

@benoitc

Appreciate if you could provide an update on this issue and the planned release to address this.

We are still using 1.15.2.

benoitc commented 3 years ago

@vasumur count it either today or tomorrow. I have been sidetracked last we, I couldn't finish the release.

vasumur commented 3 years ago

Appreciate it @benoitc.

neslinesli93 commented 3 years ago

@benoitc any news? thanks!