return next() on route completion

[chemdrew]

In order to chain pre and post completion middleware the next keyword must be called in all steps along the way. This is mostly valid for Express projects as a way to simulate Restify's 'after' event.

app.use(<pre-execution middleware>);

app.use('/graphql', graphqlExpress({ schema: MyGraphQLSchema }));

app.use(<post-execution middleware>); // my use case: log response without overriding res.end

I'll attach a PR to this issue as well, since it's a super simple and unobtrusive change and we can discuss from there!

Thanks

[freiksenet]

Merged. Thanks a lot!

[freiksenet]

Reopening. There seems to be a disagreement whether that's the best practice for middlewares that are endpoints. However I do feel the use case that @chemdrew made is valid.

Pinging @Friss and @RAFREX

[Friss]

I agree that it's a valid use case but only if you know it will happen.

If it was configurable and off by default I think it would be a good change.

[chemdrew]

I’m good with it being configurable, that makes sense to me. At least then the behavior is explicit.

If we make this change here we should also take a look at the restify package as well

[rafgraph]

I agree w/ @Friss - it's a valid use case. I think making it configurable and off by default is a good solution.

[sheerun]

exactly, for now it's impossible to run any post-execution middleware without overriding res..

[chemdrew]

I was working on adding the configurable flag for this but it made the API a tad ugly and more confusing without the background of this. Another option I'd like to suggest, which will be more work but cleaner, is that graphqlExpress returns a promise.

Then the user can either choose to use that or ignore it and chain middleware like so:

app.use(<pre-execution middleware>);

const graphqlHandler = (req, res, next) => {
  const graphqlHTTPHandler = graphqlExpress({ schema: MyGraphQLSchema });
  return graphqlHTTPHandler(req, res).then(() => {
    next();
  }).catch(next);
};

app.use('/graphql', graphqlHandler);

app.use(<post-execution middleware>); // my use case: log response without overriding res.end

[govorov]

The same case with Koa.
GraphQL is not always an endpoint. For example, I want to map GraphQL errors to http status codes after query execution.
Now it's impossible to run any post-process middleware:

// this won't work as expected

// postprocessor:
export const myPostProcessor = async (ctx, next) => {
    // my custom magic happens next...
    await next();
};

// routing:
routes.post(apiEntrypointPath, ensureAuthenticated, bodyParser(), myPostProcessor, graphQlMiddleware);

That said, there's a workaround -- change routes.post parameters order and call await next(); before postprocessing inside postprocessor source code. But I find that ugly, as from my point of view, as middleware order and execution order don't match anymore, which I find irrational.

+1 for having that option configurable. Thank you.

[the-noob]

I've recently moved from express-graphql to apollo-server-express just to see the solution for this is to go back and customise the graphqlExpress call ( + setting up Playground).
This not be good.

I need to close my AMQP channels after everything.

[jbaxleyiii]

@chemdrew @the-noob we are going to be improving the relationship between integrations and Apollo Server in the next version (Apollo Server 3) which will make this possible. I'm going to close this issue as its goals are tracked in the #2360

[timbrownsf]

Is there still no way to apply a post completion middleware on a graphql endpoint?

[cuzzlor]

You can using a Plugin

[timbrownsf]

Yes, found the answer in this thread. For future readers, beware, the official answer using didEncounterErrors in the thread doesn't work for many, you must jump to the issue comment I linked.