Improved middleware handling

[ashleybartlett]

Perceived Problem

I tried to use graphql-request to replace a custom request client we use, and OTEL plus some metrics is a requirement for doing this.

Unfortunately the middleware right now lacked a few things, especially in how it works for us. As we would be initialising GraphQLClient once, and re-using it to make queries.

1. if I were to try and start a timer for the request, I would have to handle storing that timer somewhere, as well as being able to use the correct one on a response. Unfortunately I was struggling to work around hitting async problems trying to re-use the same variable for multiple calls. I tried to wrap calling request itself, and while this works, I shouldn't have to.
2. The request middleware doesn't include the query, if we wanted to get more details about it
3. Similarly, the response middleware includes no details about the request itself. Additionally, it's not a great experience having to infer if there is an error or not and who caused it.

I suspect these improvements would simplify your code somewhat, as you are exposing the internal elements more directly to be implement by developers.

Ideas / Proposed Solution(s)

As all things like this, there are many options to solve this. I am keenly aware that it is important to keep the basic principle of a light-weight graphql client.

My key elements to solve are:

1. Having all the context in both request and response middleware
2. Providing a mechanism that allows accessing data from request in the response middleware.
3. Clearer error determination on the response.

I think the first is easy enough to solve, by including a consistent context object to both request and response middleware calls.

The second I still think could be solved with either:

1. allowing a generic "context" object to be returned with the request middleware, with whatever data you want, that gets passed to the response middleware.

 const requestMiddleware: RequestMiddleware = async (request) => {
   const timing = startTimer(request.operationName);
    return {
      ...request,
     // this is passed to the response middleware
      context: {
         timing
      },
    }
  }

2. Allow returning a callback function to act as the response middleware. That way middleware is now a single function call, and might solve needing to pass context to both request and response functions. As the signature for the middleware would look like a very basic redux middleware

const middleware = requestContext => {request, response: (responseContext) => {} }

Where a contrived implementation could look like

const middleware = requestContext => {
 const timing = startTimer();
 const trace = startTrace();
 return {
   request: {
      ...requestContext,
      headers: {
        ...requestContext.headers,
        trace.headers(),
      }
   },
   response: responseData => {
    timing.record(requestContext.operationName);
   }
 }
}

Additionally, I think the design of the response middleware needing to infer the error response could be improved by explicitly passing error objects, as well as the response object itself to the handler.

maybe context looks like:

interface requestContext {
  request: RequestInit,
  meta: {
      url: string,
      query: Query,
      operationName: string,
      variables: Variables,
  }
}

interface ResponseContext<Context = void> {
  meta: {
      url: string,
      query: Query,
      operationName: string,
      variables: Variables,
  },
  error?: ClientError | Error,
  data: GraphQLClientResponse,
  response: Response, // fetch response 
 context: Context
}