Supporting subsequent middleware

[mariushab]

I am actually not sure if this is a bug or a feature request.
Consider the example below:

const server = new ApolloServer({
    typeDefs,
    resolvers,
)};
const app = express();

app.use((req, res, next) => {
    console.log('before apollo');
    next();
});
server.applyMiddleware({ app });
app.use((req, res, next) => {
    console.log('after apollo');
    next();
)};

app.listen();

Unfortunately, the third middleware never gets called. This means that every middleware behind the apollo middleware never gets called, which I don't think is a useful behaviour.

I looked around in the source code, and it seemed to me that adding a next(); at line 53 in src/expressApollo.ts would solve the problem.

However, I don't know about the implications this change would bring and if there's a reason why currently the next middleware's don't get called. That's why I opened this issue to get some comments on the idea, and if there are any consequences on implementing this.

If there are no objections, I would happily open a pull request to fix this bug/implement the feature.

[TAnas0]

Hey there,

I have a different setup than yours, but could you please try using compose-middleware and see if it solves the problem.

If true, then that would be your temporary fix and it would be solved by this proposal.

[mariushab]

Unfortunately, this does not solve my problem.

But I don't see how using compose-middleware could have solved it, could you explain your thoughts behind it? Maybe this would give some input to help me solving my problem.

[TAnas0]

I have/had a very different error than yours and stumbled on your issue: I had trouble setting up an authentication middleware on Apollo Server with Express, but with a completely different setup and deployment problems.

I went over the proposal before and noticed:

• Move away from using applyMiddleware, opting instead for patterns appropriate for with their HTTP framework (e.g. Express, Koa, Hapi).

These HTTP frameworks should be able to evolve at their own pace without the close coupling of their major versions with Apollo Server's own. Currently, because of our close coupling, Apollo Server is responsible for (too) many updates to those frameworks and their typings because they're baked into our integrations.

And further below it:

An HTTP framework is still not mandatory though, even with all of these options, since packages like compose-middleware work well with http.createServer too, allowing the composition/chaining of various middleware into a single handler. For example, we might consider providing suggestions like this one, passing in various middleware like cors(), graphqlPlayground, json body-parsing, etc.)

These are the only reasons why I thought it might work for you.

I would advise you go over the proposal, it might help you. Besides that, I think your are right in thinking that the error comes from the Apollo Midlleware.

[abernix]

This goes back to #462. I understand how not calling next is problematic for some scenarios (and as noted, the proposal aims to fix some of this), but it's worth noting that Apollo will have already sent headers and a body in the response, which — depending on what the middleware intends to do — prevents subsequent middleware in the chain from doing much.

As a way to provide you more concrete suggestions and hopefully unblock your use-case: can you provide the use-case for what it is you need this middleware to do after Apollo has handled the GraphQL bits? Is it something that could be accomplished with the Plugin API?

[mariushab]

I already tried using the Plugin API, unfortunately my use case was not supported with it.

I am using Oracle as DB, and I want to manage the database connections from the node-oracledb package on a per-request-basis. This means that a connection should be opened at the beginning of an incoming request and closed after the request was handled.

My first idea was to use the context object. My context initialization function opened a database connection and added it to the context object, and every resolver function could use the database connection from the context object.

As I realized that I can not use express middlewares to close the database connection (because they do not get called), I found the Plugin API.

It seemed working fine at the beginning, but after a while I noticed that my server did not close every connection. I found out that the context object initialization function gets called everytime a request was made to the server, but the Plugin API functions only get called when a request to the /grahql endpoint was made. Because of this, every time a request is made to an other endpoint than /graphql, a connection gets opened but not closed.

I then tried opening the database connection in the requestDidStartevent, but it seems that the Plugin API does not support async behaviour, meaning that my resolvers can not wait until a connection to the database is available.

I have now switched to use a custom express server with the official express-graphql middleware, because this seems like the only setup wich supports my requirements.

If you have any suggestions on how to implement my use case with apollo-server, I would be more than happy to try them out.

[yueyueniao90]

i think the following code should be able to function as you need.

const server = new ApolloServer({
    typeDefs,
    resolvers,
)};
const app = express();

app.use((req, res, next) => {
    console.log('before apollo');
    next();
});

app.use((req, res, next) => {
  // process when res emit 'finish' event
  res.on('finish', function () {
    console.log('after apollo')
  })
  next()
})

server.applyMiddleware({ app });


app.listen();

However, i am still curious about your way of solving it using another grapqhl server.
this setting does not fit in with what i need, as i want to be able to modify response object after apollo finish process.
But i cannot edit response anymore when it is finished. could you let me know what is your workaround? thanks.

[glasser]

I think @yueyueniao90's suggestion is the appropriate one. When apollo-server-express handles a request completely, it doesn't call next, and you can use res.on('finish') if you want to handle something at the end of the request.

This is analogous to how, say, express.static (aka serve-static) works. When a result is actually served, next isn't called.

Are there use cases that really can't be solved with res.on('finish')? If so, I guess we could add some sort of server.applyMiddleware({ alwaysCallNext: true }), but otherwise 'finish' seems reasonable. (Or a related package like on-finished.) I'm closing but can reopen with a clear example of why the finish event isn't adequate.

[heyqbnk]

Hey there. I have met the same problem as @mariushab did. The problem is, using res.on('finish') is not express way. "Finish" does not mean call after some middleware. I mean, it is rather implicit way of solving the problem and not solving it at all. What if we have several middlewares which should be called right after the apollo's one? So, it will be working much more implicit than we can imagine.

I dont really know why serve static middleware works this way, but I am not sure apollo middleware has to follow it.

My case was using Sentry. Setting it up, I have found that I cant use it correctly due to apollo problem. Here is the guide (watch Express section):
https://docs.sentry.io/platforms/node/guides/express/

As you can see, firstly, we have to use requestHandler, then controllers (aka apollo middleware) and errorHandler. But the problem is when something is wrong, apollo does not allow sentry to be flushed in errorHandler, due to next is not called.

[glasser]

@wolframdeus This issue is a duplicate of #5560. The last comment on that is a solution I proposed. I'd be happy to take a look at a PR for it; I've reopened that issue with some more details.