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

Federating two postgraphile endpoints with Apollo Federation results in "Query.query can only be defined once" error with Relay support or "Union type _Entity" error without Relay support #8

Open
cedricvidal opened this issue Nov 28, 2019 · 14 comments

Comments

@cedricvidal
Copy link

Hello guys,

I'm trying to federate two postgraphile endpoints with the Apollo Federation gateway but can't seem to find a working combination.

Postgraphile with both Federation and Relay support

Given I started the postgraphile endpoints with Federation support (using FederationPlugin) and Relay support (default NodePlugin activated), when I start the Apollo Federation gateway, it complains with the following error:

(node:19865) UnhandledPromiseRejectionWarning: GraphQLSchemaValidationError: Field "Query.query" can only be defined once.

Field "Query.nodeId" can only be defined once.

Field "Query.node" can only be defined once.
    at ApolloGateway.createSchema (/home/cvidal/Documents/Sources/pocs/postgraphile/federation/node_modules/@apollo/gateway/dist/index.js:210:19)
    at ApolloGateway.<anonymous> (/home/cvidal/Documents/Sources/pocs/postgraphile/federation/node_modules/@apollo/gateway/dist/index.js:181:32)
    at Generator.next (<anonymous>)
    at fulfilled (/home/cvidal/Documents/Sources/pocs/postgraphile/federation/node_modules/@apollo/gateway/dist/index.js:5:58)
    at Object.dynatraceRegularInvoke [as doInvoke] (/opt/dynatrace/oneagent/agent/res/nodeagent/nodejsagent.js:1732:20)
    at Object.a.safeInvoke (/opt/dynatrace/oneagent/agent/res/nodeagent/nodejsagent.js:1802:29)
    at /opt/dynatrace/oneagent/agent/res/nodeagent/nodejsagent.js:6952:25
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

I investigated a bit and looking at the Apollo Federation demo here https://github.com/apollographql/federation-demo, the federated services don't expose node and nodeId fields in the Query type.

Postgraphile with Federation support but without Relay support

I therefore tested starting the two postgraphile endpoints without Relay support (skipping the NodePlugin plugin) but the FederationPlugin plugin fails with :

Error: Union type _Entity must define one or more member types.
    at assertValidSchema (/home/cvidal/Documents/Sources/pocs/postgraphile/custom-postgraphile/node_modules/graphql/type/validate.js:71:11)
    at Object.validate (/home/cvidal/Documents/Sources/pocs/postgraphile/custom-postgraphile/node_modules/graphql/validation/validate.js:54:35)
    at parseQuery (/home/cvidal/Documents/Sources/pocs/postgraphile/custom-postgraphile/node_modules/postgraphile/src/postgraphile/http/createPostGraphileHttpRequestHandler.ts:352:32)
    at /home/cvidal/Documents/Sources/pocs/postgraphile/custom-postgraphile/node_modules/postgraphile/src/postgraphile/http/createPostGraphileHttpRequestHandler.ts:690:55
    at Array.map (<anonymous>)
    at requestHandler (/home/cvidal/Documents/Sources/pocs/postgraphile/custom-postgraphile/node_modules/postgraphile/src/postgraphile/http/createPostGraphileHttpRequestHandler.ts:650:20)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

Apparently, this is a known problem in Apollo Federation that Relay is not supported:
https://spectrum.chat/apollo/apollo-federation/federation-with-relays-node-interface~c0ae3cb1-d243-491a-ac58-17307629e31e

Conclusion

So, this problem may be spread between Postgraphile and Apollo Federation Gateway.

That being said, I can't find a working combination so far, with or without Relay support.

@phryneas
Copy link
Contributor

Heyo, you're entering experimental territory there :)

"Postgraphile with Federation support but without Relay support" should be working if you try it with #3 (Feedback very much appreciated).

For the "Postgraphile with both Federation and Relay support", I want to get a PR into graphile-engine, so that you can disable and/or rename the fields "Query.nodeId" and "Query.node" per service. I hope to do that the coming Weekend, but cannot make any guarantees.

@cedricvidal
Copy link
Author

Hey, thank you for the quick feedback!

Postgraphile with Federation support but without Relay support

I'll give a look at your PR #3 and give you feedback as soon as I get the chance.

Postgraphile with both Federation and Relay support

Not sure renaming "Query.nodeId" and "Query.node" is a good idea. It may break the Relay support at the gateway level. It may actually be more the responsibility of the federation gateway to handle that use case.

There is an attempt here at fixing apollo-federation to support relay:
https://github.com/victorandree/apollo-federation-relay

@phryneas
Copy link
Contributor

phryneas commented Nov 30, 2019

Apollo-Federation-Relay looks very interesting.

The following is based on these two conversations if someone seeing this want to read them up:

As a result of the second discussion, I think you have two options. Let me explain:
Currently, the nodeId is generated in getNodeIdForTypeAndIdentifiers. It usually looks like base64(JSON.stringify([tableName,primaryKey1,...])). The following method getTypeAndIdentifiersFromNodeId is reverting that process. Both those methods are added to the builder object in the build phase.
Actually, by default, the above would be base64(JSON.stringify([typeName,primaryKey1,...])) (note: typeName, not tableName) instead, but this is changed (due to compatiblity with postgraphile 3 in the PgNodeAliasPostGraphile plugin.
Unfortunately, the getNodeAlias/setNodeAlias methods used there can only modify the first element of the JSON-encoded array, not the structure itself.

So, from that realization, you have two methods going forward:

  • disable the PgNodeAliasPostGraphile plugin (this might happen in postgraphile 5 anyways) and patch the fromId/toId methods of apollo-federation-relay (probably even get a PR in) to support our format in addition of their own (might be easy to detect with the leading [)
  • override getNodeIdForTypeAndIdentifiers/getTypeAndIdentifiersFromNodeId.
    This could be done in a custom plugin that hooked into the build phase, after the NodePlugin has run.
    This might look something like this (untested pseudocode from the top of my head):
export default (function NodePlugin(
  builder
) {
   
  builder.hook(
    "build",
    (build) => {
      return build.extend(
        build,
        {
          getNodeIdForTypeAndIdentifiers(Type, ...identifiers) {
            return base64(
              `${Type}:`+JSON.stringify([this.getNodeAlias(Type), ...identifiers])
            );
          },
          getTypeAndIdentifiersFromNodeId(nodeId) {
            const decoded = base64Decode(nodeId);
            const [alias, ...identifiers] = JSON.parse(decoded.slice(decoded.indexOf(':')));
            return {
              Type: this.getNodeType(alias),
              identifiers,
            };
          },
        },
        `Describe what you are doing`
      );
    },
    ["Your plugin name"],
    [],
    ["Node"] // run this AFTER "Node"
  );

});

As you are already experimenting with this, it would be really cool if you tried one (or both) of those ways and reported back!

@cedricvidal
Copy link
Author

cedricvidal commented Dec 2, 2019

Thank you @phryneas for this very detailed step by step explanation! This is very helpful! I'll try to test this this week, I feel bad since your explanation is so detailed but I have design docs to write first. I will let you know!

@phryneas
Copy link
Contributor

phryneas commented Dec 4, 2019

No worries, Open Source takes time :)
I just gave you the full rundown immediately because later I'd have probably forgotten about it ^^

@stlbucket
Copy link

stlbucket commented Dec 30, 2019

i have just encountered this same problem, i think. tho i am not using the node plugin.

@phryneas - i could put my schemas in a repo for you if that would help. not sure it's adding anything tho.

i have not tried to dig into the issue deeper - maybe it's time for me to learn something!

this occurs when i have the FederationPlugin included, whether i try to use the apollo gateway or whether i just try to hit graphiql from the server.

if i remove the plugin, then graphiql finds my schema just fine.

@stlbucket
Copy link

my stack trace is different, tho:

Error: Union type _Entity must define one or more member types.
    at assertValidSchema (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/graphql/type/validate.js:71:11)
    at Object.validate (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/graphql/validation/validate.js:54:35)
    at parseQuery (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:254:48)
    at /Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:547:63
    at Array.map (<anonymous>)
    at requestHandler (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:510:52)
    at processTicksAndRejections (internal/process/task_queues.js:85:5)

@phryneas
Copy link
Contributor

phryneas commented Dec 30, 2019 via email

@stlbucket
Copy link

stlbucket commented Dec 30, 2019

i thought that might be the case so i've done some more investigating. my original db had two schemas - one with jwt_token and an id generator function, and another with one table. i've consolidated all into one schema (below). i now get a slightly different error (also below). the overall behaviour is the same - postgraphile works fine w/o the federation plugin.

dbscript;

create extension if not exists pgcrypto;
drop schema if exists iot cascade;
create schema iot;
grant usage on schema iot to app_anonymous;
ALTER SCHEMA iot OWNER TO app;

--
-- Name: generate_ulid(); Type: FUNCTION; Schema: iot; Owner: app
--

CREATE FUNCTION iot.generate_ulid() RETURNS text
    LANGUAGE plpgsql
    AS $$
DECLARE
  -- Crockford's Base32
  encoding   BYTEA = '0123456789ABCDEFGHJKMNPQRSTVWXYZ';
  timestamp  BYTEA = E'\\000\\000\\000\\000\\000\\000';
  output     TEXT = '';

  unix_time  BIGINT;
  ulid       BYTEA;
BEGIN
  -- 6 timestamp bytes
  unix_time = (EXTRACT(EPOCH FROM NOW()) * 1000)::BIGINT;
  timestamp = SET_BYTE(timestamp, 0, (unix_time >> 40)::BIT(8)::INTEGER);
  timestamp = SET_BYTE(timestamp, 1, (unix_time >> 32)::BIT(8)::INTEGER);
  timestamp = SET_BYTE(timestamp, 2, (unix_time >> 24)::BIT(8)::INTEGER);
  timestamp = SET_BYTE(timestamp, 3, (unix_time >> 16)::BIT(8)::INTEGER);
  timestamp = SET_BYTE(timestamp, 4, (unix_time >> 8)::BIT(8)::INTEGER);
  timestamp = SET_BYTE(timestamp, 5, unix_time::BIT(8)::INTEGER);

  -- 10 entropy bytes
  ulid = timestamp || gen_random_bytes(10);

  -- Encode the timestamp
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 0) & 224) >> 5));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 0) & 31)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 1) & 248) >> 3));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 1) & 7) << 2) | ((GET_BYTE(ulid, 2) & 192) >> 6)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 2) & 62) >> 1));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 2) & 1) << 4) | ((GET_BYTE(ulid, 3) & 240) >> 4)));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 3) & 15) << 1) | ((GET_BYTE(ulid, 4) & 128) >> 7)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 4) & 124) >> 2));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 4) & 3) << 3) | ((GET_BYTE(ulid, 5) & 224) >> 5)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 5) & 31)));

  -- Encode the entropy
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 6) & 248) >> 3));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 6) & 7) << 2) | ((GET_BYTE(ulid, 7) & 192) >> 6)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 7) & 62) >> 1));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 7) & 1) << 4) | ((GET_BYTE(ulid, 8) & 240) >> 4)));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 8) & 15) << 1) | ((GET_BYTE(ulid, 9) & 128) >> 7)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 9) & 124) >> 2));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 9) & 3) << 3) | ((GET_BYTE(ulid, 10) & 224) >> 5)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 10) & 31)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 11) & 248) >> 3));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 11) & 7) << 2) | ((GET_BYTE(ulid, 12) & 192) >> 6)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 12) & 62) >> 1));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 12) & 1) << 4) | ((GET_BYTE(ulid, 13) & 240) >> 4)));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 13) & 15) << 1) | ((GET_BYTE(ulid, 14) & 128) >> 7)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 14) & 124) >> 2));
  output = output || CHR(GET_BYTE(encoding, ((GET_BYTE(ulid, 14) & 3) << 3) | ((GET_BYTE(ulid, 15) & 224) >> 5)));
  output = output || CHR(GET_BYTE(encoding, (GET_BYTE(ulid, 15) & 31)));

  RETURN output;
END
$$;


CREATE TYPE iot.jwt_token AS (
	role text,
	app_user_id text,
	app_tenant_id text,
	token text
);


CREATE TABLE iot.thing (
    id text DEFAULT iot.generate_ulid() NOT NULL,
    app_tenant_id text NOT NULL,
    created_at timestamp with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL,
    updated_at timestamp with time zone NOT NULL,
    name text not null,
    CONSTRAINT ck_thing_name CHECK ((name <> ''::text))
);

CREATE FUNCTION iot.fn_timestamp_update_thing() RETURNS trigger
    LANGUAGE plpgsql
    AS $$
  BEGIN
    NEW.updated_at = current_timestamp;
    RETURN NEW;
  END; $$;
ALTER FUNCTION iot.fn_timestamp_update_thing() OWNER TO app;
CREATE TRIGGER tg_timestamp_update_thing BEFORE INSERT OR UPDATE ON iot.thing FOR EACH ROW EXECUTE PROCEDURE iot.fn_timestamp_update_thing();

grant all on iot.thing to app_anonymous;

current error trace:

Error: Union type _Entity must define one or more member types.
    at assertValidSchema (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/graphql/type/validate.js:71:11)
    at Object.validate (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/graphql/validation/validate.js:54:35)
    at parseQuery (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:254:48)
    at /Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:547:63
    at Array.map (<anonymous>)
    at requestHandler (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:510:52)
    at processTicksAndRejections (internal/process/task_queues.js:85:5)
Error: Union type _Entity must define one or more member types.
    at assertValidSchema (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/graphql/type/validate.js:71:11)
    at Object.validate (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/graphql/validation/validate.js:54:35)
    at parseQuery (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:254:48)
    at /Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:547:63
    at Array.map (<anonymous>)
    at requestHandler (/Users/buckfactor/pg/pg-fed/iot/api/node_modules/postgraphile/build/postgraphile/http/createPostGraphileHttpRequestHandler.js:510:52)
    at processTicksAndRejections (internal/process/task_queues.js:85:5)

@stlbucket
Copy link

stlbucket commented Dec 31, 2019

figured it out...

not that i shouldn't have done so for other reasons... but you have to have a primary key on your table in order for the Node interface to implemented in order for it to be included in federation types...

... so, my bad...

and now i am back to the first error:

(node:98342) UnhandledPromiseRejectionWarning: GraphQLSchemaValidationError: Field "Query.query" can only be defined once.

Field "Query.nodeId" can only be defined once.

Field "Query.node" can only be defined once.
    at ApolloGateway.createSchema (/Users/buckfactor/node_modules/@apollo/gateway/dist/index.js:207:19)
    at ApolloGateway.<anonymous> (/Users/buckfactor/node_modules/@apollo/gateway/dist/index.js:178:32)
    at Generator.next (<anonymous>)
    at fulfilled (/Users/buckfactor/node_modules/@apollo/gateway/dist/index.js:5:58)
    at processTicksAndRejections (internal/process/task_queues.js:85:5)
(node:98342) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:98342) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

@stlbucket
Copy link

apollographql/federation-jvm#20 (comment)

this sounds like it might be a bug in the gateway itself

@phryneas
Copy link
Contributor

phryneas commented Jan 1, 2020

"Query.query" can only be defined once.

Field "Query.nodeId" can only be defined once.

Field "Query.node" can only be defined once.

You are getting these because multiple microservices are defining those and federation cannot decide which one to use.

The apollo-federation-relay package linked above might be a fix for is, but it won't work out-of-the box with postgraphile. My comment above explains how to possibly get around that.

By the way, @cedricvidal is there any progress from your side? :)

@stlbucket
Copy link

@phryneas - thanks, and i'll try that out when i get back to this (hopefully this weekend), unless it moves forward by other paths.

@Pinqvin
Copy link

Pinqvin commented Jun 3, 2020

For people who don't depend on the node and nodeId support for their queries, you can bruteforce solve this issue by just removing the conflicting fields:

import { Plugin } from "graphile-build";
import { omit } from "lodash";

export default (function RemoveNodeAndQueryFieldsPlugin(builder) {
  builder.hook("GraphQLObjectType:fields", (fields, _, { Self }) => {
    if (Self.name !== "Query") {
      return fields;
    }

    return omit(fields, ["node", "nodeId", "query"]);
  });
} as Plugin);

We need to remove the Node interface as well:

import { Plugin } from "graphile-build";

export default (function StripNodeInterfacePlugin(builder) {
  builder.hook("GraphQLObjectType:interfaces", interfaces => {
    return interfaces.filter(int => int.name !== "Node");
  });
} as Plugin);

These two plugins should allow you to federate your services. Obviously this doesn't really help you if you need that functionality

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants