HTTP binary client

[quinchs]

This PR implements an HTTP binary client

API surface changes

• Configuration for client type now accepts HTTP:

  var client = new EdgeDBClient(EdgeDBClientConfig.builder()
      .withClientType(ClientType.HTTP)
      .build()
  )

• Added getClientType method to EdgeDBClient

  /**
   * Gets the underlying client type for this client pool.
   * @return The underlying client type, usually based on transport.
   * @see ClientType
   */
  public ClientType getClientType() {
      return config.getClientType();
  }

Fixes

• Fixed an issue where an exception during pooled execution isn't propagated to the callee correctly.
• Fixed an issue during parsing where ReadyForCommand wouldn't be consumed in the failed parse case.

Overview

A new duplexer called HttpDuplexer was introduced which provides the duplex implementation for HTTP via a promise queue. On top of this, a new client class, EdgeDBHttpClient, was added which extends the EdgeDBBinary client to support the binary protocol over HTTP by proxying the duplexer to the protocol code.

One implementation note: Java being java doesn't auto implement DNS name resolving for matching URI hosts to the certificates (see this stack overflow post which is related) so using 127.0.0.1 as a hostname for HTTP clients will not work. To fix this, the default hostname in EdgeDBConnection was changed from 127.0.0.1 to localhost to match certificates, this should not effect other behavior of the client.

[quinchs]

cc @nsidnev & @scotttrinh

[scotttrinh]

Everything looks sensible to me, but I mostly skimmed the implementation itself, so feel free to point out anything you think might want some closer inspection and I'll take another pass.

[scotttrinh]

Not sure how this works in the HTTP client we're using here, but would a 3XX response also be considered a success? Or does redirection happen implicitly by the client and you end up with 2XX/4XX/5XX?

[quinchs]

I believe redirection happens implicitly, although should we even support/handle redirects though in bindings @1st1?

[1st1]

withClientType

should it instead be a separate type, like EdgeDBHTTPClient (I think this is what we have with JS, no?)

[quinchs]

withClientType

should it instead be a separate type, like EdgeDBHTTPClient (I think this is what we have with JS, no?)

The way this binding is written internally mimics the abstraction of the .NET binding, essentially EdgeDBClient acts as the root pool of x type of client, the protocol abstraction is implemented internally and controlled by the configuration option.

I'm not opposed to changing it to a new class, it would just require writing more abstractions for pooling

[scotttrinh]

I'm not opposed to changing it to a new class, it would just require writing more abstractions for pooling

Yeah, it seems like that's the main difference between the two clients in the JS driver: https://github.com/edgedb/edgedb-js/blob/master/packages/driver/src/nodeClient.ts

I defer to what feels idiomatic. If they both seem fine, then yeah maybe alignment across the clients is something we should strive for.

[quinchs]

I think the main difference with the approaches is for .NET and java we don't use global methods (createClient()) for the client, instead we use constructors (new EdgeDBClient()), so there is already already some differences between the two.

In js, adding a new method for creating a http client makes sense because you're returning the same client type in the function, but with the constructor approach this would most likely make integration in previous code (and future code for client-agnostic APIs) a bit more difficult. I think this was the consensus reached with .NET and why it acts similar to this.