ApolloServer returns null for whole request when one of `__resolveReference` returns `null`

[vad3x]

ApolloServer returns null for whole request when one of __resolveReference returns null. That happens only when array item type is not optional.

assets: [Asset!]!

Code to reproduce:

import { buildFederatedSchema } from '@apollo/federation';
import { ApolloGateway } from '@apollo/gateway';
import { ApolloServer, gql, ServerInfo } from 'apollo-server';
import { ApolloServerTestClient, createTestClient } from 'apollo-server-testing';

describe('#handler', () => {
  let gatewayServer: ApolloServer;
  let landingPagesService: ServerInfo;
  let assetsService: ServerInfo;
  let testClient: ApolloServerTestClient;

  beforeAll(async () => {
    landingPagesService = await runLandingPages();
    assetsService = await runAssets();

    const gateway = new ApolloGateway({
      serviceList: [
        { name: 'landingPages', url: `${landingPagesService.url}graphql` },
        { name: 'assets', url: `${assetsService.url}graphql` },
      ],
    });

    gatewayServer = new ApolloServer({
      gateway,
      engine: false,
      subscriptions: false,
      logger: console,
    });

    testClient = createTestClient(gatewayServer);
  });

  afterAll(() => {
    landingPagesService.server.close();
    assetsService.server.close();
  });

  it('should perform query', async () => {
    // arrange
    // run query against the server and snapshot the output
    const body = await testClient.query({
      query: '{ landingPage { assets { id name } } }',
    });

    expect(body.errors).not.toBeDefined();
    expect(body.data).not.toBe(null);
    expect(body.data).toMatchSnapshot();
  }, 10000);
});

async function runAssets() {
  const typeDefs = gql`
    type Asset @key(fields: "id") {
      id: String!
      name: String!
    }
  `;

  const assets = [
    {
      id: '1',
      name: 'Asset0',
    },
    {
      id: '2',
      name: 'Asset1',
    },
  ];

  const resolvers = {
    Asset: {
      // eslint-disable-next-line @typescript-eslint/naming-convention
      __resolveReference(object: any) {
        return assets.find(asset => asset.id === object.id);
      },
    },
  };

  const server = new ApolloServer({
    schema: buildFederatedSchema([
      {
        typeDefs,
        resolvers,
      },
    ]),
    engine: false,
    tracing: false,
  });

  const listener = await server.listen({ port: 40001 });

  return listener;
}

async function runLandingPages() {
  const typeDefs = gql`
    extend type Query {
      landingPage: LandingPage!
    }

    extend type Asset @key(fields: "id") {
      id: String! @external
    }

    type LandingPage {
      assets: [Asset!]!
    }
  `;

  const resolvers = {
    Query: {
      landingPage() {
        return {
          assets: [
            { id: '1' },
            { id: '2' },
            // uncomment to see incorrect behavior
            // { id: '10' },
          ],
        };
      },
    },
  };

  const server = new ApolloServer({
    schema: buildFederatedSchema([
      {
        typeDefs,
        resolvers,
      },
    ]),
    engine: false,
    tracing: false,
  });

  const listener = await server.listen({ port: 40002 });

  return listener;
}

The expected behavior

Null items are filter out, and errors is filled.

The actual behavior.

data is null.

versions:

• @apollo/gateway@0.28.1
• apollo-server@2.24.0
• apollo-server-testing@2.24.0

[vad3x]

Same problem for @apollo/gateway@0.43.1 apollo-server@3.4.1

import { buildFederatedSchema } from '@apollo/federation';
import { ApolloGateway } from '@apollo/gateway';
import { ApolloServer, gql, ServerInfo } from 'apollo-server';

describe('#handler', () => {
  let gatewayServer: ApolloServer;
  let landingPagesService: ServerInfo;
  let assetsService: ServerInfo;

  beforeAll(async () => {
    landingPagesService = await runLandingPages();
    assetsService = await runAssets();

    const gateway = new ApolloGateway({
      serviceList: [
        { name: 'landingPages', url: `${landingPagesService.url}graphql` },
        { name: 'assets', url: `${assetsService.url}graphql` },
      ],
    });

    gatewayServer = new ApolloServer({
      gateway,
      logger: console,
    });
  });

  afterAll(() => {
    landingPagesService.server.close();
    assetsService.server.close();
  });

  it('should perform query', async () => {
    // arrange
    // run query against the server and snapshot the output
    const body = await gatewayServer.executeOperation({
      query: '{ landingPage { assets { id name } } }',
    });

    expect(body.errors).not.toBeDefined();
    expect(body.data).not.toBe(null);
    expect(body.data).toMatchSnapshot();
  }, 10000);
});

async function runAssets() {
  const typeDefs = gql`
    type Asset @key(fields: "id") {
      id: String!
      name: String!
    }
  `;

  const assets = [
    {
      id: '1',
      name: 'Asset0',
    },
    {
      id: '2',
      name: 'Asset1',
    },
  ];

  const resolvers = {
    Asset: {
      // eslint-disable-next-line @typescript-eslint/naming-convention
      __resolveReference(object: any) {
        return assets.find((asset) => asset.id === object.id);
      },
    },
  };

  const server = new ApolloServer({
    schema: buildFederatedSchema([
      {
        typeDefs,
        resolvers,
      },
    ]),
  });

  const listener = await server.listen({ port: 40001 });

  return listener;
}

async function runLandingPages() {
  const typeDefs = gql`
    extend type Query {
      landingPage: LandingPage!
    }

    extend type Asset @key(fields: "id") {
      id: String! @external
    }

    type LandingPage {
      assets: [Asset!]!
    }
  `;

  const resolvers = {
    Query: {
      landingPage() {
        return {
          assets: [
            { id: '1' },
            { id: '2' },
            // TODO: uncomment to see incorrect behavior
            // { id: '10' },
          ],
        };
      },
    },
  };

  const server = new ApolloServer({
    schema: buildFederatedSchema([
      {
        typeDefs,
        resolvers,
      },
    ]),
  });

  const listener = await server.listen({ port: 40002 });

  return listener;
}

[lennyburdette]

Hi, thanks for the reproduction! I put it in a sandbox to make it easier to run: https://codesandbox.io/s/adoring-http-4ibys?file=/src/services/landingpage.js

I expected a top-level error similar to

{
  "data": null,
  "errors": [
    { "message": "Cannot return null for non-nullable field LandingPage.assets." }
  ]
}

because that's what would happen in a non-federated context. But all I got was { data: null }. That might be a bug, but I'm not sure.

We typically suggest using nullable field types when joining data across subgraphs. It's a bit unfortunate that the federated architecture leaks into the schema, but you're also introducing a potential network failure and if you want to return as much data as possible you'll need to use nullable field types.

[vad3x]

Hi @lennyburdette,

We typically suggest using nullable field types when joining data across subgraphs.

This is what I ended up doing.

However, I still think it's the gateway bug because { data: null } response does not make much sense. Not sure why 1 incorrect field kills the whole response.

[lennyburdette]

Gotcha, thanks for the clarification. Consider a monolithic graph — if you had:

type Query {
  landingPage: LandingPage!
}

type LandingPage {
  assets: [Asset!]!
}

type Asset  {
  id: ID!
}

and you tried to return:

{
  "data": {
    "landingPage": {
      "assets": [{ "id": "1" }, null, { "id": "2" }]
    }
  }
}

The null in the list would violate the contract of the schema. The validation error bubbles up to the first nullable field, and in this case all fields are non-null so it bubbles all the way up to the root. You can test this by changing the nullability of the fields above the null:

If LandingPage.assets was [Asset!] (nullable list with non-null items), you'd get:

{
  "data": {
    "landingPage": {
      "assets": null
    }
  }
}

If Query.landingPage was nullable, you would get

{
  "data": {
    "landingPage": null
  }
}

which of course is not much better.

I confirmed with the team that the lack of an error in the response is a bug. I believe we're addressing this either in the Federation 2 release or soon after.

[vad3x]

Thanks @lennyburdette, I think that makes sense. I'm looking forward to use Federation 2!

[martin-svanberg-northvolt]

@lennyburdette We've run into the same issue. Is a fix for this on the roadmap?

[lennyburdette]

I'm still not seeing an error using Fed 2: https://stackblitz.com/edit/basic-federation-2-ks15jz?file=one.js,two.js,package.json

@pcmanus @clenfest Do you have an opinion on this? Query._entities returns a sparse array ([_Entity]) but if those null values are used in a non-sparse array shouldn't we see a top-level error?