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

More detailed errors when connection closed during message body #25

Merged

Conversation

njsmith
Copy link
Member

@njsmith njsmith commented Dec 3, 2016

Intended as at least a first step towards gh-23.

@njsmith
Copy link
Member Author

njsmith commented Dec 3, 2016

@Lukasa: so here's a first step towards fixing #23, with "human readable" errors only -- I guess we can merge this first and then decide whether to add special exception types as well.

The exception message phrasing currently a bit awkward IMO -- suggestions welcome.

@codecov-io
Copy link

Current coverage is 100% (diff: 100%)

Merging #25 into master will not change coverage

@@           master   #25   diff @@
===================================
  Files          20    20          
  Lines         889   894     +5   
  Methods         0     0          
  Messages        0     0          
  Branches      176   176          
===================================
+ Hits          889   894     +5   
  Misses          0     0          
  Partials        0     0          

Powered by Codecov. Last update 421d322...75e271c

@njsmith
Copy link
Member Author

njsmith commented Dec 3, 2016

Oh, and to be clear, the full behavior with this PR is:

  • connection dropped during content-length body: "peer closed connection without sending complete message body (received N bytes, expected M)"

  • connection dropped during a chunked body while we are waiting for the \r\n at the end of a chunk header, or while reading trailers: "peer unexpectedly closed connection" (generic fallback)

  • connection dropped during chunked body, at any other moment: "peer closed connection without sending complete message body (incompleted chunked read)"

Copy link
Member

@Lukasa Lukasa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So for my part I think this would be a good first step. I can investigate with urllib3 to see if this gets me close enough to what we already had to satisfy my needs. =D

@njsmith
Copy link
Member Author

njsmith commented Dec 6, 2016

Cool, let's do this then. (As usual, let me know if you need me to roll a release.)

@njsmith njsmith merged commit 1df9935 into master Dec 6, 2016
@njsmith njsmith deleted the better-errors-on-lost-connection-during-message-body branch December 6, 2016 05:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants