apollo-fetch icon indicating copy to clipboard operation
apollo-fetch copied to clipboard

What's the advantage of afterware vs. middleware?

Open inxilpro opened this issue 7 years ago • 5 comments

Middleware could easily function before/after a request:

const middleware = (req, options, next) => {
  options.headers = options.headers || {};
  options.headers['authorization'] = 'created token';
  return next(req, options);
};

const afterware = (req, opts, next) => {
  const res = next(req, opts);
  if (401 === res.status) {
    logout();
  }
  return res;
};

Is there a reason to have explicit middle/after stacks that I'm not seeing?

inxilpro avatar Jul 11 '17 22:07 inxilpro

I should have a better answer for you in a couple of hours. The current reasons are simplicity of the middleware and aftwerware in their implementations and clarity of the order of their application. Your suggestion is certainly worth investigating. Are there any use cases you can think of that might need this sort of access to request and response?

While we are thinking about the middleware/afterware structure, another plan to look at is the specification for apollo-link, especially "Composing Links into a chain". Links are designed to modify GraphQL control flow with an HttpLink(calls ApolloFetch) or other transport link at the end of the chain. apollo-fetch deals with everything fetch and http specific. I'm curious to hear your thoughts on apollo-link and the interplay between link and fetch.

evans avatar Jul 12 '17 06:07 evans

I guess I just think adding the concept of "afterware" just means that people have to deal with two concepts and the library has to handle two stacks/loops/etc. When everything is implemented as middleware it keeps it simple, and it allows for middleware that modifies the request and response. So in your auth example:

function makeAuthMiddleware(auth) {
  return (req, opts, next) => {
    opts.headers = opts.headers || {};
    opts.headers.authorization = auth.createAuthorizationToken();
    
    const res = next(req, opts);
    if (401 === res.status) {
      auth.logout();
    }
    return res;
  };
}

Using a pipeline is pretty straightforward:

function runMiddleware(middleware, req = {}, opts = {}) {
  const final = (req, opts) => {
    // FIXME:
    return `build body from ${JSON.stringify(req)} and do fetch() with ${JSON.stringify(opts)}`;
  };
  
  if (0 === middleware.length) {
    return final(req, opts);
  }
  
  if (1 === middleware.length) {
    return middleware[0](req, opts, final);
  }
  
  const pipeline = middleware.reduce((next, current) => {
    return (req, opts) => current(req, opts, next);
  }, final);
  
  return pipeline(req, opts);
}

Demo:

const req = { foo: 'bar' };
const opts = { headers: { 'x-bar': 'baz'}};
const middleware = [
  (req, opts, next) => {
    opts.headers = opts.headers || {};
    opts.headers.authorization = 'auth header';
    
    const res = next(req, opts);
    return `Check auth, then ${res}`;
  },
  (req, opts, next) => next(req, opts).replace(/"/g, ''),
];

console.log(runMiddleware(middleware, req, opts));

// Check auth, then build body from {foo:bar} and do fetch() with {headers:{x-bar:baz,authorization:auth header}}

I think the apollo-link spec looks interesting in a lot of ways, but it seems to me that apollo-fetch should be as simple as possible.

inxilpro avatar Jul 12 '17 15:07 inxilpro

There are lots of cases where middleware would want to modify both the request and response. Another common one would be logging:

function log(req, opts, next) => {
  console.log('Requesting:', req, opts);
  const res = next(req, opts);
  console.log('Result:', res);
  return res;
};

inxilpro avatar Jul 12 '17 15:07 inxilpro

@inxilpro Thank you for the suggestions! Right now, we are going to stick with the current interface for middleware and afterware, so that we get backwards compatibility with Apollo Client's middleware and afterware. Are you interested in contributing the functionality for middleware's next to return the fetch result from the afterware?

The proposed order of execution would be:

apolloFetch.use(m1).use(m2).useAfter(a1).useAfter(a2)

m1 > m2 > fetch > a1 > a2 > m2 > m1

Let me know what you think! This could be a good migration path toward deprecating afterware in apollo-fetch

It also sound like you had some ideas on how to apply the middleware/afterware more efficiently. If you are interested in that more, this code handles the ware application.

evans avatar Jul 26 '17 06:07 evans

Another use case is for JWT-refresh. The order of operation is basically:

  1. request 1 gets a 401. It is implied, but not known, that it is because of an expired JWT.
  2. a separate request is made to another resource to refresh the JWT
  3. request 1 is retried. If it gets a 401 again, then it should be treated as a fatal error, as now it is no longer implied to be caused by an expired JWT.

The implementations I've seen (outside of Apollo) require keeping some state:

  • a boolean "has the JWT refresh request been made?", so that the retry does not get stuck in a loop in case some other issue is causing a 401 that is unrelated to the JWT.
  • a boolean "is this the refresh JWT request itself?", so that the retry logic is not used on that request
  • at the time of the response of request 1, a reference to the original request would be needed, so it could be retried.

Just my 2 cents. :)

(I eventually got here via this discussion.)

twelve17 avatar Aug 10 '17 18:08 twelve17