Not possible to have a middleware after graphqlExpress instantiation

[fjcero]

I have seen some issues related, here is more about a design question than just proposing a solution, since there are some interesting approaches that could solve this particular problem (#945).

Also, there are related issues to this, and some were merged but rolled back since it breaks the 404 policy of express, but I think breaking middlewares is even worst

• return next() on route completion #462
• adding in the callback to express package #463

[abernix]

I left a comment on #945 (comment) about why I don't believe returning a Promise from the middleware function is an okay solution (for the time being, at least). I hope we can find a solution that works for everyone, and I appreciate you opening this issue from a design perspective, @fjcero.

It would be great to put some concrete examples in here so we can track them in a single place (unless there is a single place already that I'm missing?)

[fjcero]

@abernix as you said, having a wrapper function like the next one, solve most of the "issues".

(req, res, next) => {
  graphqlExpress((req, res) => ({
    schema,
  })
})(req, res, next)

The thing is, that Express is used as a base of different frameworks and using this middleware directly is not suitable for how things are designed in each case. One of the solutions I found in that scenario is to create a custom middleware using apollo-core and handling things the best way. This is not always the best, beyond graphqlExpress could be opinionated (or not), having the conventional wisdom of the community helps to accelerate project implementation, which is why this middleware exists first of all.

I don't know if is the best approach, but perhaps having an easy way to create custom middlewares is an improvement of apollo-core to make this task simple enough in the cases that are necessary.

[cuzzlor]

Excuse me @fjcero and @abernix for not understanding, how can the wrapper above function fix the issue? I note that nothing is calling next() in the example code?

Even if you added a call to next() inside the wrapper function after the call to graphqlExpress, the graphqlExpress execution would likely not be finished, as there are underlying asynchronous calls which are not awaited within the graphqlExpress function.

If I'm missing something, please let me know, as we currently have to use a customised version of the graphqlExpress function to make our post execution middleware run, after graphql execution, as per my closed PR: #945.

I simply can't see any other way of implementing code to clean up request scoped context after graphql is finished.

[RobertStigsson]

@cuzzlor We're faced with the exact same issue, and you seem to be correct. There are no way to do post execution, since if you run next() you will always end up inside the graphql again. I saw your Promise-return and we've come to the same conclusion independently. No idea how to implement it in any other way (except adding next() no matter if it fail execution or not), but we will most likely need to use our own modified version (just like yours) since we would like to log after graphql-execution. I'm actually really surprised that it's not a bigger problem for other users!

[evans]

@RobertStigsson To add logging after GraphQL execution, you can use formatResponse

[abernix]

No updates here but let's continue to track this in #462. We're certainly aware of this being a problem for some cases (and working as desired in others).