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

HtWire hangs until connection is closed remotely #62

Closed
llorllale opened this issue Jun 14, 2018 · 19 comments
Closed

HtWire hangs until connection is closed remotely #62

llorllale opened this issue Jun 14, 2018 · 19 comments
Assignees
Labels
bug Something isn't working

Comments

@llorllale
Copy link
Collaborator

llorllale commented Jun 14, 2018

Test case:

new TextOf(
  new HtResponse(
    new HtWire("somehost"),
    "GET /some/resource HTTP/1.1\r\n" +
    "Host: somehost\r\n\r\n"
  )
).asString()

The code above will hang until the remote service sends a TCP packet with the FIN flag.

The fundamental question here is: how do we tell that we have already received the entire HTTP response and thus avoid having to wait until the remote service ends the connection themselves?

We can start with section 3.3.3 of RFC7230.

Edit: can be avoided by sending the Connection: close header (remote service needs to behave well; usually not a problem though)

@llorllale llorllale added the bug Something isn't working label Jun 14, 2018
@llorllale
Copy link
Collaborator Author

@0crat in

@0crat 0crat added the scope label Jun 14, 2018
@0crat
Copy link
Collaborator

0crat commented Jun 14, 2018

Job #62 is now in scope, role is DEV

@0crat
Copy link
Collaborator

0crat commented Jun 14, 2018

@0crat in (here)

@llorllale Job #62 is already in scope

@0crat
Copy link
Collaborator

0crat commented Jun 14, 2018

Bug was reported, see §29: +15 point(s) just awarded to @llorllale/z

@llorllale llorllale self-assigned this Jun 28, 2018
@0crat
Copy link
Collaborator

0crat commented Jun 28, 2018

@llorllale/z not enough funds available in the project, can't set budget of job #62, see §21; @llorllale/z will get no money on completion; in order to fix that, add funds to the project and assign the job again

@0crat
Copy link
Collaborator

0crat commented Jun 28, 2018

Manual assignment of issues is discouraged, see §19: -5 point(s) just awarded to @llorllale/z

@0crat
Copy link
Collaborator

0crat commented Jun 28, 2018

It is strongly discouraged to assign jobs to their creators, see §19: -15 point(s) just awarded to @llorllale/z

@llorllale
Copy link
Collaborator Author

@llorllale

Keep in mind that our goals [1] do not include implementing every feature offered by other HTTP libs.

With that said:

Work to be done

  • Need to refactor HtWire so that the socket is kept alive and its associated inputstream is not read at all. This opens the possibility for us to support persistent connections in the future (although would need to keep a reference to the socket if we intend to write to it again) and makes it the responsibility of *Response objects to properly handle network resources.
  • Need to provide decorators to HtResponse that automatically close the inputstream, and thus the socket, on some criteria:
    • Based on Content-Length
    • Based on Transfer-Encoding: chunked

Alternatives

As noted in the description, a workaround would be for the sender to manually include the Connection: close header in the request:

  new HtResponse(
    new HtWire("somehost"),
    new JoinedText(
        "\r\n",
        "GET /some/resource HTTP/1.1",
        "Host: somehost",
        "Connection: close\r\n"
    ).asString()
  )

The problems I see with this are:

  1. The user has to manually type that string. Querying network resources is typically a one-off thing in the majority of use cases outside of browsers.
  2. Leaves control of the lifecycle of network resources entirely in the remote service' hands. This is unacceptable, as we must also be able to close the socket when we are finished using it.

[1] Goals not written anywhere. There is only a vague statement in the README: We are well aware of competitors (most likely they are more powerful than our client, but less object-oriented)

llorllale added a commit that referenced this issue Jun 28, 2018
is not closed when returning from #send(), and for implementing
HtResponse decorators that will enable us to decide when to close
connection on our end.
@0pdd
Copy link
Collaborator

0pdd commented Jun 28, 2018

@llorllale 2 puzzles #63, #64 are still not solved.

@llorllale
Copy link
Collaborator Author

llorllale commented Jun 28, 2018

@llorllale left puzzles in 795e57c

@0crat 0crat removed the scope label Jun 28, 2018
@0crat
Copy link
Collaborator

0crat commented Jun 28, 2018

Order was finished: +30 point(s) just awarded to @llorllale/z

@0crat
Copy link
Collaborator

0crat commented Jun 28, 2018

The job #62 is now out of scope

@llorllale llorllale mentioned this issue Jul 3, 2018
@0pdd
Copy link
Collaborator

0pdd commented Mar 30, 2019

@llorllale 2 puzzles #64, #78 are still not solved; solved: #63.

@0pdd
Copy link
Collaborator

0pdd commented Apr 21, 2019

@llorllale 3 puzzles #78, #80, #81 are still not solved; solved: #63, #64.

@0pdd
Copy link
Collaborator

0pdd commented Jun 1, 2019

@llorllale 3 puzzles #80, #81, #87 are still not solved; solved: #63, #64, #78.

@0pdd
Copy link
Collaborator

0pdd commented Jun 18, 2019

@llorllale 2 puzzles #81, #87 are still not solved; solved: #63, #64, #78, #80.

@0pdd
Copy link
Collaborator

0pdd commented Oct 21, 2019

@llorllale 3 puzzles #81, #98, #99 are still not solved; solved: #63, #64, #78, #80, #87.

@0pdd
Copy link
Collaborator

0pdd commented Nov 8, 2019

@llorllale 2 puzzles #81, #98 are still not solved; solved: #63, #64, #78, #80, #87, #99.

@0pdd
Copy link
Collaborator

0pdd commented Nov 12, 2019

@llorllale the puzzle #81 is still not solved; solved: #63, #64, #78, #80, #87, #98, #99.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants