mercurius-js / mercurius

Implement GraphQL servers and gateways with Fastify
https://mercurius.dev/
MIT License
2.34k stars 234 forks source link

Federated schema is not merging Queries and Mutations #219

Closed voslartomas closed 4 years ago

voslartomas commented 4 years ago

Hello,

trying to define federated schema like this, but only last service Queries and Mutations are show in final schema. If I change order of services, it will always generate only last one (Types get merged though). Any ideas what can be wrong? I am using apollo federation and it is merging correctly, I would love to move to fastify-gql as you guys are supporting subscriptions and it seems like there should be performance boost.

Another thing, is there a way to define different route for http://.../graphql and websockets ws:/.../ ?

const Fastify = require('fastify')
import GQL from 'fastify-gql'

const gateway = Fastify()
gateway.register(GQL, {
  graphiql: 'playground',
  subscription: true,
  gateway: {
    services: [
      { name: 'user', url: 'http://localhost:5020/graphql' },
      { name: 'contest', url: 'http://localhost:5000/graphql' },
      { name: 'event', url: 'http://localhost:5010/graphql' }
    ]
  }
})

gateway.listen(4001, '0.0.0.0').then((d) => {
  console.log(`Server listen on ${d}`)
})
mcollina commented 4 years ago

Can you please create a full example to reproduce? Or even better, send a PR to fix with a test? Thanks

voslartomas commented 4 years ago

Unfortunately I don't have any idea how fastify or this plugin works, I just randomly find that this might be another solution instead of using apollo server.

voslartomas commented 4 years ago

I guess this library would be first to have subscription and federation support (gateway). Was so excited this could work :)

mcollina commented 4 years ago

But I guess it's not working for anyone right?

I don't know, you are the first one reporting this. Overall, I asked for a full repro which you did not provide.

@pscarey is using federation in production and it's working for them, it's working for us, so I'm not sure I would claim it's not working. It's working well enough for my uses.

As usual, I'm happy to review a PR to fix this.

asciidisco commented 4 years ago

@voslartomas It does work for my projects.

In regards to your problem, can it be that your Query, Mutation & Subscription definitions don't contain an @extends directive? It might be helpful to see the corresponding bits of your partial schemas & the federated endresult.

voslartomas commented 4 years ago

@mcollina Sorry, I was just trying your example with subscriptions and it was not working. I created PR to fix example #220

voslartomas commented 4 years ago

@asciidisco Great to hear, thanks. Have you been using apollo server before, so can you compare performance? And are you using subscriptions with federation?

I am using type-graphql to define schemas in services, I will send schemas of each service. And create example.

voslartomas commented 4 years ago

@asciidisco Here are the definitions of schemas

Schemas definition


type Mutation {
  createContest(data: ContestInput!): Contest!
  updateContest(data: ContestUpdateInput!, contestId: String!): Contest!
  notifyWinners(contestId: String!): Boolean!
}

type Query {
  getActiveLeaderboard: Contest
}

type Mutation {
  createEvent(eventData: EventInput!): Event!
}

type Query {
  getCurrentEvents: [Event!]!
 }
asciidisco commented 4 years ago

@voslartomas

Yes, we are currently transitioning from Apollo Federation to a Fastify based one. As we are dealing with a rather large project, which also contains work by other teams building parts of the application in Kotlin, Scala etc. it'll take us some time & we're talking about months rather than weeks to put this into production.

In regards to your other questions, I'm not very prolific in creating benchmarks, plus I don't think that one can compare the performance of such large & distributet systems quite easily. F.e. in order to get subscriptions with Apollo Federation working, we have a Schema-Stitching server in front of the Federation one, use apollo-opentracing to integrate Jaeger, etc.; so things where you can't easily have a 1:1 comparison with a fastify based implementation... As far as I'm able to test this, for some Queries I've worked on, we managed to double the throughput when using fastify-gql over apollo, measured with the max. amount of requests we could throw at the system using autocannon. I tend to believe that fastify (and especially fastify-gql) will eventually perform better than our current system.

I kinda figured that you're using somethign like type-graphql (we do too) & I've seen a similar behaviour to the problem you described before. The federated schemas type-graphql generates do work with Apollo out of the box, but not with fastify-gql. I'm using a workaround when building the local schemas, in order to make them compatible with the spec. I'm posting you the code we use (feel free to re-use it for your need), but please bare in mind that we use this to "just make it work" atm. & we plan to re-iterate on this as it isn't performant & we'd like to remove the dependency on the printSchema method of @apollo/federation.

import { printSchema } from '@apollo/federation'
import { buildSchema, createResolversMap, BuildSchemaOptions } from 'type-graphql'
import { 
  specifiedDirectives, 
  GraphQLFieldResolver, 
  GraphQLScalarType,
  GraphQLString,
  GraphQLNonNull,
  GraphQLDirective,
  DirectiveLocation
} from 'graphql'

export const KeyDirective = new GraphQLDirective({
  name: 'key',
  locations: [DirectiveLocation.OBJECT, DirectiveLocation.INTERFACE],
  args: {
    fields: {
      type: GraphQLNonNull(GraphQLString),
    },
  },
})

export const ExtendsDirective = new GraphQLDirective({
  name: 'extends',
  locations: [DirectiveLocation.OBJECT, DirectiveLocation.INTERFACE],
})

export const ExternalDirective = new GraphQLDirective({
  name: 'external',
  locations: [DirectiveLocation.OBJECT, DirectiveLocation.FIELD_DEFINITION],
})

export const RequiresDirective = new GraphQLDirective({
  name: 'requires',
  locations: [DirectiveLocation.FIELD_DEFINITION],
  args: {
    fields: {
      type: GraphQLNonNull(GraphQLString),
    },
  },
})

export const ProvidesDirective = new GraphQLDirective({
  name: 'provides',
  locations: [DirectiveLocation.FIELD_DEFINITION],
  args: {
    fields: {
      type: GraphQLNonNull(GraphQLString),
    },
  },
})

const federationDirectives = [
  KeyDirective,
  ExtendsDirective,
  ExternalDirective,
  RequiresDirective,
  ProvidesDirective,
]

export interface SchemaOptions {
  schema: string,
  resolvers: GraphQLResolverMap,
}

export interface GraphQLResolverMap<TContext = {}> {
  [typeName: string]:
    | {
        [fieldName: string]:
          | GraphQLFieldResolver<any, TContext>
          | { requires?: string, resolve: GraphQLFieldResolver<any, TContext> }
      }
    | GraphQLScalarType
    | { [enumValue: string]: string | number }
}

export default async function buildFederatedSchema(
  options: Omit<BuildSchemaOptions, 'skipCheck'>
): Promise<SchemaOptions> {
  const schema = await buildSchema({
    ...options,
    skipCheck: true,  
    directives: [...specifiedDirectives, ...federationDirectives, ...(options.directives || [])],
  })

  const modifiedSchema = printSchema(schema)
    .replace('type Query {', 'type Query @extends {')
    .replace('type Mutation {', 'type Mutation @extends {')
    .replace('type Subscription {', 'type Subscription @extends {')

  return { 
    schema: modifiedSchema,
    resolvers: createResolversMap(schema) as GraphQLResolverMap,
  }
}

As you see, there is quite some boilerplate that is only there to enable us to add @extends directives to each of the Query, Mutation & Subscription types. It is also not related to fastify-gql in my opinion, more related with how type-graphql generates the federated schemas, which is tailored to what Apollo expects.

voslartomas commented 4 years ago

@asciidisco Thank you very much, for your explanation, I will give it a try. I was hoping it would be possible to get rid of schema-stitching server in front of apollo federation, because it is our solution at the moment and it's not performing very well, in fact it's causing us trouble in production when there is high load for longer time. But right now we have apollo-server as schema stitching server, so as you suggested it might help to exchange it with fastify-gql.

Are you using apollo engine (now studio I guess), it seems we will loose this function.

voslartomas commented 4 years ago

@asciidisco When added extends to all services I got this error (node:58676) UnhandledPromiseRejectionWarning: Error: The directive "@extends" can only be used once at this location. Any idea what is wrong?

asciidisco commented 4 years ago

@voslartomas Yes, this is due to this issue: https://github.com/mcollina/fastify-gql/issues/184 Issue comment https://github.com/mcollina/fastify-gql/issues/184#issuecomment-643789399 has a fix for this, you can apply manually for the time being, until the PR is merged.

voslartomas commented 4 years ago

@asciidisco Ok fixed, thank you very much for the support. Now I am on subscriptions, I know you are using schema stitching server for subscriptions, but will give it a try here, it's giving me this error

fastify-gql/lib/subscription-client.js:198
        throw new Error(`Invalid message type: "${data.type}"`)
              ^
Error: Invalid message type: "error"
    at SubscriptionClient.handleMessage (...../fastify-gql/lib/subscription-client.js:198:15)
asciidisco commented 4 years ago

@voslartomas I haven't seen that error before, it might be interesting to see what's the concrete error here (coming from the concrete service), can you log the full data object here, please: https://github.com/mcollina/fastify-gql/blob/master/lib/subscription-client.js#L165

Also, in regards to Apollo Engine (or whatever it is called today), this is actually one of the reasons we're moving away from Apollo. It's more a philosophical view, than a pure technical one & I'm not in a position to judge some of the decision taken in Apollo in regards to implementations in apollo-server.

Once you're trying to do Federation "seriously", you might run into some problems that get increasingly harder to solve with the current direction Apollo is heading in. F.e. changing a schema at runtime, starting the federation with one of the Federated service not being available at startup & then joining the federation later, not using HTTP/WS based communication between services, not to mention subscriptions... etc.

We're currently jumping through lots of burning rings to make this work with Apollo, need to throw a lot of (self written/maintained) boilerplate code at it to make it work & most of the time, it feels like fighting the framework which is supposed to help us with those things. We can't use Apollo-Engine due to security contraints of our corp & a lot of those features within Apollo are build to work with Apollo Engine exclusively.

I do understand that Apollo is a company & that providing that Saas platform is their way of generating money, which then also is used to fund the work on their libraries. All fine, just not a model that works for us.

I'm currently working on a self-hosted solution for schema management, which should work with fastify-gql (not released & not ready for primetime yet) & partly solves the same issues as Apollo-Engine does. Also one of the reasons why our transition from Apollo to Fastify takes some time.

voslartomas commented 4 years ago

@asciidisco It was wrong address, but now I am getting this result is not async iterable, I've created fastify-gql federation server and then one fastify-gql service which has subscriptions, if I connect directly to service subscriptions are working fine, but via federation server it throws error mentioned above.

Changing schema at runtime is possible via apollo studio nowadays, but I agress it is not really nice from them, to have features like that in their own service, which you need to pay and it's not possible to host it on your own. On the other hand it's really nice service, I was looking for some alternative which would be open sourced and you could host it on your own.

Will be your solution for schema management open source?

paolochiodi commented 4 years ago

I tried to replicate the errors @voslartomas is experiencing here and these are my results:

  1. the gateway subscriptions example doesn't work Calling node examples/gateway-subscription.js will results in an unhandled exception:

    (node:49669) UnhandledPromiseRejectionWarning: Error: The directive "@key" can only be used once at this location.
    at assertValidSDLExtension (/fastify-gql/node_modules/graphql/validation/validate.js:124:11)
    at extendSchema (/fastify-gql/node_modules/graphql/utilities/extendSchema.js:77:43)
    at buildFederationSchema (/fastify-gql/lib/federation.js:253:22)
    at buildGateway (/fastify-gql/lib/gateway.js:151:18)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async fp.name (/fastify-gql/index.js:144:15)
  2. I was not able to replicate the error where queries and mutations do not show up. If I "fix" the error at point 1 and run the example, query and mutations seems to work properly.

voslartomas commented 4 years ago

I tried to replicate the errors @voslartomas is experiencing here and these are my results:

  1. the gateway subscriptions example doesn't work Calling node examples/gateway-subscription.js will results in an unhandled exception:
(node:49669) UnhandledPromiseRejectionWarning: Error: The directive "@key" can only be used once at this location.
    at assertValidSDLExtension (/fastify-gql/node_modules/graphql/validation/validate.js:124:11)
    at extendSchema (/fastify-gql/node_modules/graphql/utilities/extendSchema.js:77:43)
    at buildFederationSchema (/fastify-gql/lib/federation.js:253:22)
    at buildGateway (/fastify-gql/lib/gateway.js:151:18)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async fp.name (/fastify-gql/index.js:144:15)
  1. I was not able to replicate the error where queries and mutations do not show up. If I "fix" the error at point 1 and run the example, query and mutations seems to work properly.

1) is because of this #184 2) Is because I am using type-graphql in services, which does not extend Query, Mutation and Subscription @asciidisco suggested solution above.

asciidisco commented 4 years ago

@paolochiodi

1.) If you apply the "fix" (or workaround, what you prefer to call it) mentioned here it should work. So yes, it's a known bug & afaik. due to the reasons discussed in #184 there's no PR for it yet. Basically waiting for more information to not release a fix which could have some unforseen implications.

asciidisco commented 4 years ago

@voslartomas The async iteratable problem is described in https://github.com/mcollina/fastify-gql/issues/208; so far it seems like there's no PR for it yet, so, if you want to have a go, please, that would not only help you 🙂

To emphasize again on the Apollo situation. I'm absolutely fine with the model Apollo as a company pursues. Think about it that way, they are a company that releases a lot of assets/software for free, for some of the features (or a service) they need to charge in order to create revenue (I believe we've all seen how devastating the current situation of monetizing open source is). Because they exist, and because they had some funding or other financial backing, it allowed them to pay people to work on the GraphQL ecosystem full time which in fact laid out the foundation for features/concepts like federation. Concepts we've all benefit from now, regardless if we use the libraries build by them or not.

The solution I'm building will be open source, most probably MIT licensed & I have no intention to benefit from it monetary wise. We need something like this for our projects, there's no solution out there, so I'm builidng one. But that also means you shouldn't hold your breath, in the best case it'll be out in September.

voslartomas commented 4 years ago

@asciidisco Agree, it is just surprising that they were doing all of this open source and for some reason they go with closed source studio. I believe they could go the way Gitlab went, so you have SaaS which generates you money, but also it's open source and you can host it yourself if you want. It seems working.

Looking forward to see your solution, is it somewhere on Github already?

asciidisco commented 4 years ago

@voslartomas I don't feel equipped to dicuss or judge business decisions like this (and kinda think that this issue might not be a good place to dicuss this further).

Not yet public, because not yet working at all & I don't want to mislead people in thinking there is a working solution, nor could I handle supporting potential contributers at the time being, sorry.

voslartomas commented 4 years ago

@asciidisco You are right, not right place to discuss thinks like this.

Ok, will subscribe to your profile then. Thanks

voslartomas commented 4 years ago

@asciidisco Trying to continue with sockets and it seems it depends on order of services defined in federation, I am not able to make everything working, because of the order, I have errors like this

cached.jit.query is not a function

This one is trying to createSubscription in service which does not have defined wsUrl

'service.createSubscription is not a function'

Not sure why should depend on order of services, any idea?

asciidisco commented 4 years ago

@voslartomas Pulling in these changes should mitigate your first error: https://github.com/mcollina/fastify-gql/pull/174

Not sure about the second, haven't seen it before tbh.

asciidisco commented 4 years ago

@voslartomas Have you had a chance to pull in the mentioned fix & retest again? I‘d like to see if the other error vanishes with it, and if not, try to trace that one down.

voslartomas commented 4 years ago

Hello, first error was fixed, but still there were errors with createSubsription and also I have another errors when you have same ENUM in two services, there was another error when you switched order. So after all I gave up and went with apollo federation/gateway and created websocket server with fastify.

asciidisco commented 4 years ago

@voslartomas Okay, I suggest to close the issue then & I‘ll have a dedicated look and will open issues once I was able to replicate the errors on my own. I think there are one or two which should be dedicated issues. I have a rough idea which setup you‘re sporting, so I should be fine with tracking them once I find the time.

voslartomas commented 4 years ago

Ok cool, closing. Thanks for your help