Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrating from 1.0 to 2.0: How to add middleware? #1117

Closed
darkadept opened this issue May 31, 2018 · 29 comments
Closed

Migrating from 1.0 to 2.0: How to add middleware? #1117

darkadept opened this issue May 31, 2018 · 29 comments
Milestone

Comments

@darkadept
Copy link

I'm using express middleware on before my graphql endpoint to authenticate and do some other stuff. How do I do that in 2.0?

I had apollo-server-express configured in 1.0 like this:

const schema = makeExecutableSchema({typeDefs, resolvers, schemaDirectives});

app.use(
  '/api/graphql',
  jwt({
    secret: process.env.MYSECRET,
  }),
  someCustomMiddlewareOfMyOwn(),
  graphqlExpress({schema}),
);

Looking at how I'm supposed to configure ApolloServer() and use registerServer() it makes sense until I want to add my own middleware. There doesn't seem to be a place to add this middleware.

@eugene1g
Copy link

registerServer mounts the Apollo middleware on the endpoint. With express, it's possible to mount middleware onto the same point several times, so what I do is something like this -

// Mount my middleware to  run *before* Apollo which can break the request chain if required
app.use("/graphql", requireAuth, throttle);

// Mount Apollo middleware
await registerServer({ app, server: apolloServ });

@darkadept
Copy link
Author

@eugene1g Great! I solved this a slightly different way in rewriting my middleware and putting everything in the graphql context function instead. I ran into the problem of context not resolving promises though. (#1066, which you commented on already, thanks!).

I know they aren't the same (middleware vs context), but for me it achieves roughly the same thing. Between the two, it seems like both require a bit of a hack to make it work. The double mounting middleware method requires you to overwrite the mount point and the context method (currently) requires the wrapper function you mentioned in #1066.

Which of the two methods would you recommend? Right now it actually seems like the double mounting endpoint is less hacky. (and supports third-party express middleware out of the box.)

@darkadept
Copy link
Author

FYI, I switched to using the double mounting method as stated above and it's working beautifully.

@eugene1g
Copy link

eugene1g commented Jun 1, 2018

I think Apollo Server is still figuring out the best way to plugin into existing app servers.

If all you do is GraphQL, then it's an awesome experience out of the box. If you need it to be a piece of a larger app, then it's a bit messy - multiple express instances involved etc. For example, the above method of "double mounting" middleware only helps if you want to run middleware before Apollo, but I think there is no way to run anything after Apollo (without monkey patching) because it breaks the chain and the response. I expect they will figure it out soon!

Personally, I use both methods. I put "http" stuff like auth into middleware and mount it before Apollo. I also use the async context function to do "business/app" stuff. For example, passport middleware decodes the user_id from the session and places it into req, then my async context function takes that user_id and runs a service to populate a user object into GraphQL context. This means I can run the same GraphQL code from CLI or from a library script as long as I feed it { user_id: 1}

@evans
Copy link
Contributor

evans commented Jun 1, 2018

The double mounting makes sense. The one suggestion I would make is setting the path explicitly in the registerServer call, rather than depending on the default @unicodeveloper is going to write it up in #1123 🎉

For middleware after Apollo, it can be added after the registerServer call, though the graphqlExpress middleware shouldn't normally call next. Are there use cases that you've come across where you need that sort of functionality @eugene1g ?

@eugene1g
Copy link

eugene1g commented Jun 1, 2018

Nope, I don't have a real use-case for calling middleware after Apollo - it's just a "what-if".

When I quickly tested by adding another middleware with 'app.use()' after registerServer, that last middleware never executed for me. Not sure if it's a bug, related to #953, or I did something wrong in haste.

@tonychuuy
Copy link

tonychuuy commented Jun 5, 2018

Hello, @evans,
I'm trying to add some cookies in a resolver but I don't have access to express response object inside the resolvers, so I tried adding a middleware after the registerServer but as @eugene1g said it is never executed, I tried the formatResponse function but I couldn't add headers there. Any Idea how can I get access to the response object inside a resolver?

@eugene1g
Copy link

eugene1g commented Jun 5, 2018

@avirdz One workaround is to use the context function and the provided req parameter to manually inject res into your GraphQL execution context like so -

const myserver =   new ApolloServer({
    schema: executableSchema,
    context: ({req}) => { return { res: req.res }},
    formatError: formatResponseError
  });

Then you can access res directly from your resolver and use it to set a cookie. A bit hacky, but should work.

@tonychuuy
Copy link

Thanks a lot, I tried that before and it didn't work, I needed to use req.res instead of req.response 👍

@leovanhaaren
Copy link

@evans I'm mainly running into trouble regarding CORS middleware in version 2. In version 1 it worked out fine. Any ideas?

@jardakotesovec
Copy link

jardakotesovec commented Jun 13, 2018

I also would be careful about trying to over-simplify the setup. As mentioned above I agree that for standalone options thats fine. But for middleware scenario having magic function like registerServer is confusing to me.. why should I call listen on ApolloServer since its supposed to be just middleware. (I already spent hour trying to migrate middleware scenario from version 1 to version 2 and still does not work and have no idea how that would play if I want both http and https which I had before).

I had similar experience with apollo-boost. I think just having boilerplate in doc to be copy&pasted is better than hiding that behind magic functions/objects/package. It does not make difference if newcomer has to install multiple packages and copy&paste more lines. It gets confusing once some of the customization needs to be done.. and developer has to replace the apollo-boost.

So I actually preferred the previous more explicit middleware setup.

@evans
Copy link
Contributor

evans commented Jun 13, 2018

@jardakotesovec We hear you! With the middleware, we've definitely run into issues with https #1155 and hot module reloading, accessing the underlying listeners #1137.

The reality of registerServer is that it is really an applyMiddleware, so it will either be aliased or renamed. Totally see that idea that calling server.listen and hiding the http/https listeners causes issues. With beta.10, we're moving towards a world that makes the app/integration call listen

@leovanhaaren You're able to pass a cors options to registerServer, which is then provided to the cors middleware.

@evans evans added this to the Release 2.0 milestone Jun 14, 2018
@evans
Copy link
Contributor

evans commented Jun 14, 2018

In the latest beta.11, we moved to an applyMiddleware architecture, so the new flow looks something like: https://www.apollographql.com/docs/apollo-server/v2/essentials/server.html#middleware

The new api should the implementor to understand what is going on under the hood without having to dig through the source.

Subscriptions are now created in this manner: https://glitch.com/edit/#!/mountainous-suggestion?path=index.js:49:0

If there are still things we can do to improve the middleware api, I would love to hear them!

@jardakotesovec
Copy link

@evans That was quick. Just updated and looks great. Thanks!

@SkaterDad
Copy link

@evans I was just looking yesterday at how I would integrate Apollo 2.0 into an existing hapi application, so the beta.11 change is very much appreciated!

@wyattjoh
Copy link

@evans It seems like if you wanted to mount multiple graphql endpoints on a server, it might cause some conflicting behavior, specifically around the health checks as it attaches global routes.

@lionkeng
Copy link

Similar to the concern raised in #1117 (comment), we have a use case where we maintain multiple graphql endpoints (protected vs public APIs). Each of these endpoints have different schemas. What is the recommended approach in Apollo Server 2.0?

@majelbstoat
Copy link
Contributor

I want to make a call using a request-scoped logger in middleware just before a response is returned to log success or otherwise.

Per the docs, I would expect to be able to do:

server.applyMiddleware({ app })
app.use(loggingMiddleware as RequestHandler)

(where the logger has been added to the request at an earlier stage.)

However, with 2.0.0-rc.7, the loggingMiddleware is never called. What's the suggested way to achieve this?

@edelreal
Copy link

edelreal commented Jul 20, 2018

@lionkeng : One idea that was mentioned in ardatan/graphql-tools#323 and graphql/graphql-js#113, was controlling the visibility of portions of the schema, based on a role. If one could control the visibility of portions of the schema (not just their execution), then you could still have a single end point that provides both an admin and non-admin view of the same original API. I would also like to know what the recommended approach for Apollo server 2.0 is for migrating from multiple endpoints, each with different middleware.

@vjpr
Copy link

vjpr commented Jul 21, 2018

@lionkeng Were you able to get something working for multiple schemas?

@lionkeng
Copy link

Hi @vjpr, we are still experimenting to see what works best.
For our use-case, we have 2 separate paths for the 2 sets of APIs (public, protected) that we are maintaining. So the recipe we are using looks something like this (with some details omitted):

const app = express()

// other middleware for express

const server1 = new ApolloServer({schema1, context1})
const server2 = new ApolloServer({schema2, context2})

server1.applyMiddleware({app, path: path1})
server2.applyMiddleware({app, path: path2})
 
app.listen(YOUR_API_PORT, () => {
   // some logging
})

@tspecht
Copy link

tspecht commented Jul 27, 2018

Any other hints on this? Currently trying to change the response status code after the request has finished processing inside the GraphQL-layer but any app.use(...) I place after server.applyMiddleware({app}) doesn't get called.

@wyattjoh
Copy link

I am loving the direction with the new Apollo server, as it makes it much easier for new developers to get started.

My only ask is this, allow us to only apply the GraphQL middleware as you've done in apollo-server-express@^1.3.6:

const express = require('express');

const app = express();

// ...

const server = new ApolloServer({ schema, context });

// Only apply the GraphQL middleware, nothing else.
app.use('/graphql', express.json(), server.middleware.graphql);

// ...

I think that gives the best solution for most existing implementations, and still allows the use of the "all-in-one" solution that's proposed already.

Would love to get some feedback from the Apollo team on this 😄

@javierblancosp
Copy link

javierblancosp commented Aug 10, 2018

I have the same problem but regarding with the express 404 middleware. Since apollo let the response pass, the res.end is executed twice. So i ended up doing this workaround:

  graphqlService.applyMiddleware({ app });

  app.use((req, res, next) => {
    if (res.headerSent === false) {
      next(notFoundError());
    } else {
      res.end();
    }
  });

@franher
Copy link

franher commented Sep 6, 2018

seems like this PR fix this issue #1436 Could you verify and close if the issue is not happening anymore with v2.0.2

@einnjo
Copy link

einnjo commented Sep 27, 2018

I am loving the direction with the new Apollo server, as it makes it much easier for new developers to get started.

My only ask is this, allow us to only apply the GraphQL middleware as you've done in apollo-server-express@^1.3.6:

const express = require('express');

const app = express();

// ...

const server = new ApolloServer({ schema, context });

// Only apply the GraphQL middleware, nothing else.
app.use('/graphql', express.json(), server.middleware.graphql);

// ...

I think that gives the best solution for most existing implementations, and still allows the use of the "all-in-one" solution that's proposed already.

Would love to get some feedback from the Apollo team on this 😄

I agree that this would be better for us that need to customize the endpoint further beyond what is provided by default. It was provided in an earlier version and worked great.

@kfrajtak
Copy link

Hi,
I am using the "standalone" ApolloServer and I would like to add passport for authentication. I don't know how since the standalone version is not exposing neither the app nor a method to apply middleware. The applyMiddleware method is now throwing an error

public applyMiddleware() {
  throw new Error(
    "To use Apollo Server with an existing express application, please use apollo-server-express"
  );
}

I am using these packages

"apollo-server": "^2.4.8"
"apollo-server-core": "^2.5.0"
"apollo-server-express": "^2.4.8"

Can you help?
Thanks,
Karel

@myhendry
Copy link

If u ll b using express middleware, u should use apollo-server-express

The information on Apollo Docs can be found under the heading of "Adding Additional Middleware to Apollo Server 2". Basically, it is just app.use(...) before using applyMiddleware call

@jbaxleyiii jbaxleyiii added the 🚧👷‍♀️👷‍♂️🚧 in triage Issue currently being triaged label Jul 8, 2019
@jbaxleyiii
Copy link
Contributor

We offer apollo-server-* for integrations which allow for customizing the middleware as part of an overall other server framework!

@abernix abernix removed 🚧👷‍♀️👷‍♂️🚧 in triage Issue currently being triaged labels Jul 9, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests