fix: federation subgraph introspection does not require GraphQL introspection (#8018)

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: jdolle <1841898+jdolle@users.noreply.github.com>

Author: 3 people

.changeset/brave-feet-occur.md

@@ -0,0 +1,5 @@
+---
+'@graphql-hive/cli': patch
+---
+
+Correct fallback behavior for subgraph introspection. If subgraph introspection using graphql's standard introspection query fails, then it will fall back to Federation's Query._service query.

integration-tests/tests/cli/dev.spec.ts

@@ -318,31 +318,60 @@ describe('dev --remote', () => {
     await expect(cmd).rejects.toThrowError('Non-shareable field');
   });
 
-  test.concurrent('correct error on failed introspection', async ({ expect }) => {
-    const server = await createHTTPGraphQLServer();
-    const { createOrg } = await initSeed().createOwner();
-    const { createProject } = await createOrg();
-    const { createTargetAccessToken } = await createProject(ProjectType.Federation);
-    const { secret } = await createTargetAccessToken({});
-    const cli = createCLI({ readwrite: secret, readonly: secret });
-
-    const supergraph = tmpFile('graphql');
-    const cmd = cli.dev({
-      remote: true,
-      services: [
-        {
-          name: 'bar',
-          url: server.url + '/graphql-federation-no-introspection',
-        },
-      ],
-      write: supergraph.filepath,
-    });
-
-    await expect(cmd).rejects.toThrowError(
-      `Could not get introspection result from the service 'bar'`,
-    );
-    // make sure correct error  code is being thrown
-    await expect(cmd).rejects.toThrowError('[116]');
-    await expect(cmd).rejects.not.toThrow('[115]');
-  });
+  test.concurrent(
+    'subgraph introspection via `Query._service` succeeds when graphql introspection is disabled',
+    async ({ expect }) => {
+      const server = await createHTTPGraphQLServer();
+      const { createOrg } = await initSeed().createOwner();
+      const { createProject } = await createOrg();
+      const { createTargetAccessToken } = await createProject(ProjectType.Federation);
+      const { secret } = await createTargetAccessToken({});
+      const cli = createCLI({ readwrite: secret, readonly: secret });
+
+      const supergraph = tmpFile('graphql');
+      const cmd = cli.dev({
+        remote: true,
+        services: [
+          {
+            name: 'bar',
+            url: server.url + '/graphql-federation-no-introspection',
+          },
+        ],
+        write: supergraph.filepath,
+      });
+
+      await expect(cmd).resolves.toContain('Composition successful');
+    },
+  );
+
+  test.concurrent(
+    'subgraph introspection fails when target is not a subgraph',
+    async ({ expect }) => {
+      const server = await createHTTPGraphQLServer();
+      const { createOrg } = await initSeed().createOwner();
+      const { createProject } = await createOrg();
+      const { createTargetAccessToken } = await createProject(ProjectType.Federation);
+      const { secret } = await createTargetAccessToken({});
+      const cli = createCLI({ readwrite: secret, readonly: secret });
+
+      const supergraph = tmpFile('graphql');
+      const cmd = cli.dev({
+        remote: true,
+        services: [
+          {
+            name: 'bar',
+            url: server.url + '/graphql',
+          },
+        ],
+        write: supergraph.filepath,
+      });
+
+      await expect(cmd).rejects.toThrow(
+        `The provided service URL does not point to a valid Federation subgraph.`,
+      );
+      // make sure correct error  code is being thrown
+      await expect(cmd).rejects.toThrow('Cannot query field "_service" on type "Query"');
+      await expect(cmd).rejects.toThrow('[121]');
+    },
+  );
 });

integration-tests/tests/cli/introspect.spec.ts

@@ -100,6 +100,43 @@ export async function createHTTPGraphQLServer() {
   });
 
   const yogaNoIntrospection = createYoga({
+    logging: false,
+    graphqlEndpoint: '/graphql-no-introspection',
+    schema: createSchema({
+      typeDefs: /* GraphQL */ `
+        type Query {
+          hello: String!
+        }
+      `,
+    }),
+    plugins: [useDisableIntrospection()],
+  });
+
+  const yogaBadService = createYoga({
+    logging: false,
+    graphqlEndpoint: '/graphql-bad-service',
+    schema: createSchema({
+      typeDefs: /* GraphQL */ `
+        type _Service {
+          sdl: String
+        }
+
+        type Query {
+          _service: _Service
+          hello: String!
+        }
+      `,
+      resolvers: {
+        Query: {
+          _service: () => {
+            throw new Error('Something went wrong');
+          },
+        },
+      },
+    }),
+  });
+
+  const yogaFederationNoIntrospection = createYoga({
     graphqlEndpoint: '/graphql-federation-no-introspection',
     logging: false,
     schema: buildSubgraphSchema({
@@ -150,11 +187,23 @@ export async function createHTTPGraphQLServer() {
 
   server.route({
     // Bind to the Yoga's endpoint to avoid rendering on any path
+    url: yogaFederationNoIntrospection.graphqlEndpoint,
+    method: ['GET', 'POST', 'OPTIONS'],
+    handler: (req, reply) => yogaFederationNoIntrospection.handleNodeRequestAndResponse(req, reply),
+  });
+
+  server.route({
     url: yogaNoIntrospection.graphqlEndpoint,
     method: ['GET', 'POST', 'OPTIONS'],
     handler: (req, reply) => yogaNoIntrospection.handleNodeRequestAndResponse(req, reply),
   });
 
+  server.route({
+    url: yogaBadService.graphqlEndpoint,
+    method: ['GET', 'POST', 'OPTIONS'],
+    handler: (req, reply) => yogaBadService.handleNodeRequestAndResponse(req, reply),
+  });
+
   await server.listen({
     port: 0,
     host: '0.0.0.0',
@@ -289,12 +338,46 @@ test.concurrent('can introspect protected federation with header', async ({ expe
   `);
 });
 
-test.concurrent('error handling on server with no introspection enabled', async ({ expect }) => {
+test.concurrent(
+  'error handling on server with introspection is disabled and _service does not respond',
+  async ({ expect }) => {
+    const server = await createHTTPGraphQLServer();
+    const command = introspect([server.url + '/graphql-no-introspection']);
+    await expect(command).rejects.toThrow('Could not get introspection result from the service.');
+    await expect(command).rejects.toThrow('[116]');
+    await expect(command).rejects.not.toThrow('[115]');
+  },
+);
+
+test.concurrent(
+  'can introspect federated service even if introspection is disabled',
+  async ({ expect }) => {
+    const server = await createHTTPGraphQLServer();
+    const command = introspect([server.url + '/graphql-federation-no-introspection']);
+
+    await expect(command).resolves.toContain('type Query {');
+  },
+);
+
+test.concurrent('error is thrown when _service exists but fails', async ({ expect }) => {
   const server = await createHTTPGraphQLServer();
-  const command = introspect([server.url + '/graphql-federation-no-introspection']);
-  await expect(command).rejects.toThrowError(
-    'Could not get introspection result from the service.',
-  );
+  const command = introspect([server.url + '/graphql-bad-service']);
+
+  await expect(command).rejects.toThrow('Could not get introspection result from the service.');
   await expect(command).rejects.toThrow('[116]');
   await expect(command).rejects.not.toThrow('[115]');
 });
+
+test.concurrent(
+  'federation can be introspected when explicitly defined even if introspection is disabled',
+  async ({ expect }) => {
+    const server = await createHTTPGraphQLServer();
+    const command = introspect([
+      server.url + '/graphql-federation-no-introspection',
+      '--type',
+      'federation',
+    ]);
+
+    await expect(command).resolves.toContain('type Query {');
+  },
+);

packages/libraries/cli/src/commands/dev.ts

@@ -514,7 +514,7 @@ export default class Dev extends Command<typeof Dev> {
       logger: this.logger,
     }).catch(err => {
       this.logFailure(err);
-      throw new IntrospectionError(serviceName);
+      throw err;
     });
 
     if (!sdl) {

packages/libraries/cli/src/helpers/errors.ts

@@ -228,7 +228,11 @@ export class NetworkError extends HiveCLIError {
 /** GraphQL Errors returned from an operation. Note that some GraphQL Errors that require specific steps to correct are handled through other error types. */
 export class APIError extends HiveCLIError {
   public ref?: string;
-  constructor(cause: Error | string, requestId?: string) {
+  constructor(
+    cause: Error | string,
+    requestId?: string,
+    public graphQLErrors?: ReadonlyArray<GraphQLError>,
+  ) {
     super(
       ExitCode.ERROR,
       errorCode(ErrorCategory.GENERIC, 15),
@@ -396,6 +400,16 @@ export class InvalidTargetError extends HiveCLIError {
   }
 }
 
+export class InvalidFederationSubgraphError extends HiveCLIError {
+  constructor(reason?: string) {
+    super(
+      ExitCode.BAD_INIT,
+      errorCode(ErrorCategory.GENERIC, 21),
+      `The provided service URL does not point to a valid Federation subgraph.${reason ? `\n${reason}\n` : ''}`,
+    );
+  }
+}
+
 export class SchemaNotFoundError extends HiveCLIError {
   constructor(commit?: string) {
     super(

packages/libraries/cli/src/helpers/graphql-request.ts

@@ -102,6 +102,7 @@ export function graphqlRequest(config: {
         throw new APIError(
           jsonData.errors.map(e => e.message).join('\n'),
           cleanRequestId(response?.headers?.get('x-request-id')),
+          jsonData.errors,
         );
       }
 

packages/libraries/cli/src/helpers/schema.ts

@@ -9,6 +9,7 @@ import type { BaseLoaderOptions, Loader, Source } from '@graphql-tools/utils';
 import type { TypedDocumentNode } from '@graphql-typed-document-node/core';
 import { FragmentType, graphql, useFragment as unmaskFragment, useFragment } from '../gql';
 import { SchemaWarningConnection, SeverityLevelType } from '../gql/graphql';
+import { APIError, IntrospectionError, InvalidFederationSubgraphError } from './errors';
 import { graphqlRequest } from './graphql-request';
 import { Texture } from './texture/texture';
 
@@ -217,66 +218,69 @@ class FederationSubgraphUrlLoader implements Loader {
       },
     });
 
-    this.logger?.debug?.('Attempt "_Service" type lookup via "Query.__type".');
-
-    // We can check if the schema is a subgraph by looking for the `_Service` type.
-    const isSubgraph = await client.request({
-      operation: parse(/* GraphQL */ `
-        query ${'LookupService'} {
-          __type(name: "_Service") ${' '}{
-            name
-          }
-        }
-      `) as TypedDocumentNode<{ __type: null | { name: string } }, Record<string, never>>,
-    });
-
-    if (isSubgraph.__type === null) {
-      this.logger?.debug?.('Type not found, this is not a Federation subgraph.');
-      return [];
-    }
-
-    this.logger?.debug?.(
-      'Resolved "_Service" type. Federation subgraph detected.' +
-        'Attempt Federation introspection via "Query._service" field.',
-    );
-
-    const response = await client.request({
-      operation: parse(/* GraphQL */ `
+    try {
+      const response = await client.request({
+        operation: parse(/* GraphQL */ `
         query ${'GetFederationSchema'} {
           _service {
             sdl
           }
         }
       `) as TypedDocumentNode<{ _service: { sdl: string } }, Record<string, never>>,
-    });
+      });
 
-    this.logger?.debug?.('Resolved subgraph SDL successfully.');
+      this.logger?.debug?.('Resolved subgraph SDL successfully.');
 
-    const sdl = minifySchema(response._service.sdl);
+      const sdl = minifySchema(response._service.sdl);
 
-    return [
-      {
-        document: parse(sdl),
-        rawSDL: sdl,
-      },
-    ];
+      return [
+        {
+          document: parse(sdl),
+          rawSDL: sdl,
+        },
+      ];
+    } catch (err) {
+      if (
+        err instanceof APIError &&
+        err.graphQLErrors?.some(
+          err =>
+            err.message.includes('Cannot query field "_service" on type "Query"') ||
+            err.message.includes('Cannot query field "sdl" on type "_Service"'),
+        )
+      ) {
+        throw new InvalidFederationSubgraphError(
+          'The GraphQL server responded with the following errors:\n' +
+            err.graphQLErrors.map(error => `- ${error.message}`).join('\n'),
+        );
+      }
+      throw err;
+    }
   }
 }
 
 class FederationSubgraphIntrospectionThenGraphQLIntrospectionUrlLoader implements Loader {
   private urlLoader = new UrlLoader();
   private federationLoader: FederationSubgraphUrlLoader;
+
   constructor(private logger?: LegacyLogger) {
     this.federationLoader = new FederationSubgraphUrlLoader(logger);
   }
 
   async load(pointer: string, options: BaseLoaderOptions & { headers?: Record<string, string> }) {
-    this.logger?.debug?.('Attempt federation introspection');
-    let result = await this.federationLoader.load(pointer, options);
-    if (!result.length) {
-      this.logger?.debug?.('Attempt GraphQL introspection');
-      result = await this.urlLoader.load(pointer, options);
+    try {
+      return await this.federationLoader.load(pointer, options);
+    } catch (e) {
+      // if this error is because because federated introspection isnt supported, then ignore and try
+      // normal introspection.
+      if (!(e instanceof IntrospectionError || e instanceof InvalidFederationSubgraphError)) {
+        // otherwise, raise an introspection error because some unknown error happened during introspection.
+        // this may be unintuitive, but we don't want to raise an API Error since users may believe our API is the one at fault.
+        // We'd rather nudge them to look into their service's behavior.
+        throw new IntrospectionError();
+      }
     }
-    return result;
+
+    this.logger?.debug?.('Query._service not found. This is a not a Federation subgraph.');
+    return await this.urlLoader.load(pointer, options);
   }
 }