bigtestjs / server

All BigTest development has moved to https://github.com/thefrontside/bigtest
https://github.com/thefrontside/bigtest
2 stars 1 forks source link

Query subscriptions #58

Closed cowboyd closed 4 years ago

cowboyd commented 4 years ago

Right now, we support "one-shot" queries for accessing the state of the server. However, stateful user interfaces like our CLI and eventually our dashboard will need to be refreshed constantly whenever an update happens like a test starts running or is completed. This we can do by taking the concept of a one-off graphql query and turning it into a stream.

If a request is made to the command server and the web socket headers like

Upgrade: websocket
Connection: Upgrade

are detected, then instead of taking the query as a one-shot, we store the query, and every time the state changes, we run the query and push the results out of the websocket (assuming that it has changed)

jnicklas commented 4 years ago

It seems like it's common to use a directive to indicate whether a query is meant to be "live", see this blog post.

We might consider going with this kind of pattern? I can imagine that it'd be convenient to allow both options over websocket, so if we do need a one-shot query we don't need to switch transport protocol.

cowboyd commented 4 years ago

Yeah, I have to say, I don't fully understand the spec on GraphQL subscriptions, and so the "live query" seems not only more in line with what we want, but simpler to implement.

It does seem silly to have to use one interface for one-shot queries and yet again another for live queries. So how about this:

The message format for the command socket (in typescript even though the format will be JSON):

interface QueryMessage {
  // source of the graphql query.
  query: string;

  // client generated value that can be used to reconcile responses.
  responseId: string;

  // If true, a new response with the same `responseId` will be sent
  // every time the state changes
  live?: boolean; 
}

interface MutationMessage {
  //graphql mutation source
  mutation: string;

  // client generated value that can be used to reconcile responses.
  responseId: string;
}

An example of a live query to get the agents:

yield sendData({ 
  query: `{ agents { browser { name version } } }`
  live: true, 
  responseId: 'agent-names'
} );

while (true) {
  let { data } = yield receiveData(connection, 'agent-names');
}

Or to create a test run:

yield sendReceive({
  mutation: `runTest(agent: "Safari")`
  responseId: `run`
});

I know that this doesn't use @live directive like the blog post says, but it is the exact same idea, because it's negotiated at the transport layer, we don't have to ignore the fact that a @live query won't work over the normal http API.

jnicklas commented 4 years ago

I think we can run both of those, websocket and http, in the same server on the same port?

My point was that if we have a client which uses the websocket protocol, that client should be able to choose between live and not live. It doesn't seem nice for that client to also have to implement support for the HTTP request/response protocol just to be able to perform a "not live" query. I think that's why using the @live directive is a good idea, since it makes the client implementation simpler.

cowboyd commented 4 years ago

I think we can run both of those, websocket and http, in the same server on the same port?

I get it now. I did not see how this was possible, but clearly it is with the server option to the web socket server constructor

My point was that if we have a client which uses the websocket protocol, that client should be able to choose between live and not live. It doesn't seem nice for that client to also have to implement support for the HTTP request/response protocol just to be able to perform a "not live" query. I think that's why using the @live directive is a good idea, since it makes the client implementation simpler.

The concept of a "live" switch does make the implementation a lot simpler, but my only issue is with using the @live graphql directive itself, rather than using a boolean on the web socket message to flip the switch instead. The complexity I see is that a directive is actually part of the graphql query itself, and so we can't know if the query is meant to be live without actually parsing the source first.

As it stands, the logic is really, simple:

function* handleQuery(message: QueryMessage, connection: Connection): Operation {
  yield publishQueryResult(message, connection);

  if (message.live) {
    yield fork(subscribe(message, connection));
  }
}

That's pretty simple. The graphql logic is 100% shared between the http interface and the web socket interface, and no need to have a streaming and pub/sub abstraction between the graphql layer and the transport layer.

Would perhaps make more sense to support the @live directive in a query if we eventually add streaming support to our http interface via a chunked response? As it stands we'd have to issue a warning, or flat out ignore the @live directive from http queries.

Here's a recording showing a "one-shot" query over websocketsc and a "live" query over websockets.

2020-01-31 08 56 28

The only difference really is where the live attribute is implemented. Here, it's implemented at the transport layer instead of the query layer.