Beahivour of HTTPClient.apply if a session already exists

[sacovo]

When calling HTTPClient.apply for the second time using a new HandleFactory, the behaviour isn't quite clear.

I'm trying to work with HTTPClient and promises, but the second promise is not used, only the first one.

use "net"
use "promises"
use "http"


actor Main
  new create(env: Env) =>
    let client = NetworkClient(TCPConnectAuth(env.root), env.out)

    try
      let url = URL.build("https://echo.sacovo.ch")?

      let p1 = Promise[Payload val]
      let p2 = Promise[Payload val]

      p1.next[Array[ByteSeq] val](ToBody)
        .next[None](PrintBody(env.out))

      p2.next[Array[ByteSeq] val](ToBody)
        .next[None](PrintBody(env.out))

      client.get(url, p1)
      client.get(url, p2)
    end

class ToBody
  fun apply(p: Payload val): Array[ByteSeq] val? =>
    recover val p.body()?.clone() end


class PrintBody
  let out: OutStream

  new create(out': OutStream) =>
    out = out'

  fun apply(a: Array[ByteSeq] val) =>
    for line in a.values() do
      out.print(line)
    end


class MyHandlerFactory is HandlerFactory
  let promise: Promise[Payload val]
  let out: OutStream

  new create(promise': Promise[Payload val], out': OutStream) =>
    promise = promise'
    out = out'

  fun box apply(session: HTTPSession tag): HTTPHandler ref^ =>
    out.print("Received session")
    MyHandler(promise, out, session)


class MyHandler is HTTPHandler
  let promise: Promise[Payload val]
  let out: OutStream
  let session: HTTPSession

  new create(promise': Promise[Payload val], out': OutStream, session': HTTPSession) =>
    promise = promise'
    out = out'
    session = session'

  fun ref apply(payload: Payload val): None tag =>
    out.print("Received payload, status: " + payload.status.string())
    promise(payload)
    out.print("Called promise")


actor NetworkClient
  let client: HTTPClient
  let out: OutStream

  new create(auth: TCPConnectAuth, out': OutStream) =>
    client = HTTPClient.create(auth)
    out = out'

  be get(url: URL, promise: Promise[Payload val]) =>
    out.print("Sending get...")
    try
      let payload = Payload.request("GET", url)
      payload.update("My-Header", "This is a test header!")
      client.apply(consume payload, MyHandlerFactory(promise, out))?
      out.print("Sent request")
    else
      promise.reject()
    end

As far as I understand from this conversation, this is because a session already exists and the HandlerFactory that is provided in the second call is not used because of that.

[SeanTAllen]

So, I went over this and #9 with @jemc and then did more looking into this and came up with what I think is an acceptable plan for maintaining as much compatability with the existing client as is.

Change handler factory creation

• change HTTPClient.apply from taking a payload and handler factory to ONLY taking a payload.
• change the HTTPClient constructor to take a handler factory

While the existing interface is non-surprising if the scheme, post, and port hasn't been seen before, it is very surprising for cases like the code in the body of this issue where the session is shared but, then handlers are expected to be different in some way.

This change will make folks write a handler for all requests for a given host using the same handler.

Change HTTPClient and associated documentation

We should make it clear that while HTTPClient manages 1 or more connections, it does it with a single handler so if different handling strategies are needed, different HTTPClients should be used.

---

I believe this is the least invasive way to remove this issue and #9. Doing this would then allow for more thorough rethinking of an HTTPClient for Pony.

[SeanTAllen]

The other options for "mostly the same interface" would be:

Connection per host/handler

Make the handler part of the session lookup so that we start a new connection for each different handler factory to a given host. This would result in a lot of extra connections if the handler factory is a lambda.

This is not a breaking change.

Store handler with request

In _client connection, as part of "sent/unsent", we could store the app handler for a given apply call and call the appropriate handler per response. We would need to check and see how this works out with "send body". I think it should be fine, but I'm not positive.

This is not a breaking change.

[SeanTAllen]

We discussed during sync. We are going to go with #102 (comment)

And we are very open to a redesign of the HTTP library to do a new version that has gone through a redesign.