FramedRead only calls decode_eof when there is buffered data

[carllerche]

This issue is related to the discussion in tokio-rs/tokio-core#178

Given the tokio-proto pattern of representing a body stream as Body(Option<Chunk>), an HTTP 1.1 Decoder would require decode_eof to be called even if there is no more data.

In the case of streaming bodies w/ Connection: close and no content length, the body stream is all the data until the socket is closed. This means that an HTTP 1.1 decoder would need to emit a Body(None) frame on EOF when there is no buffered data.

[alexcrichton]

Yeah this was sort of the ... "easy" mode? Do we want it to support everything? Do you have an idea how to fix? (I don't)

[carllerche]

It could be unconditionally called, but then it would require changing the return signature of decode_eof -> Result<Option<Item>> or something like that.

[alexcrichton]

Perhaps yeah, but I'd be wary of how the meaning of None is then very different between decode and decode_eof.

[carllerche]

I'm open to alternatives :) Given that `decode_eof` has a default impl, I think it is OK to expand on it a bit and still have framed be "easy mode"

…

On Thu, Mar 2, 2017 at 1:17 PM Alex Crichton ***@***.***> wrote: Perhaps yeah, but I'd be wary of how the meaning of None is then very different between decode and decode_eof. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#7 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAYJLyJ-A_tLG85-iGwiph8dm7sOJKnks5rhzHcgaJpZM4MQhGk> .

[alexcrichton]

It's true yeah being a default method could help quite a bit here. That means most will never implement the method itself.

Perhaps we could try itout and see how it goes? (I can't think of any other alternatives myself)

[Ralith]

It could be unconditionally called, but then it would require changing the return signature of decode_eof -> Result<Option> or something like that.

This was the approach that I introduced in #1, motivated purely by it seeming much more obvious to me. It was reverted in cddbdb3.

[carllerche]

As mentioned in the original PR. The change to generic errors was not rejected, but the API change should be made & discussed in isolation in a PR. That makes it easier to evaluate + leaves a discussion that can be linked to in the future when people ask questions. It was my fault for including the change in the PR. I mostly just adapted #1 without looking at master.

…

On Thu, Mar 2, 2017 at 5:32 PM Benjamin Saunders ***@***.***> wrote: It could be unconditionally called, but then it would require changing the return signature of decode_eof -> Result<Option> or something like that. This was the approach that I introduced in #1 <#1>, motivated purely by it seeming much more obvious to me. It was reverted in cddbdb3 <cddbdb3> . — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#7 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAYJFP-HZEmkG7h1ejFHunBn3qs0p7iks5rh22ngaJpZM4MQhGk> .

[Ralith]

To be clear, I was referring to the EOF handling, not generic errors (which are offtopic in this PR).