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

"Upload" defined in resolvers, but not in schema #1534

Closed
tab00 opened this issue Aug 15, 2018 · 10 comments
Closed

"Upload" defined in resolvers, but not in schema #1534

tab00 opened this issue Aug 15, 2018 · 10 comments

Comments

@tab00
Copy link

tab00 commented Aug 15, 2018

After passing resolvers into ApolloServer, it adds Upload: Upload to it. This causes the above error when reusing it in makeExecutableSchema

  const server = new ApolloServer({
    typeDefs,
    resolvers,
    dataSources: () => ({
      dsBooks: new DSBooks()
    }),
    context: async ({ req }) => ({ user: await getUser(req.headers.authorization) })
  })

  import { makeExecutableSchema } from 'graphql-tools'
  const schema = makeExecutableSchema({ typeDefs, resolvers })

Currently I work around it by delete resolvers.Upload

My app has no file upload function, so it is very strange to see anything about Upload.

Please fix this problem.

@sbrichardson
Copy link
Contributor

I think you can pass 'uploads: false' in the new ApolloServer options object.

uploads?: boolean | FileUploadOptions;

https://github.com/apollographql/apollo-server/blob/master/packages/apollo-server-core/src/ApolloServer.ts#L135

if (uploads !== false) {
      if (this.supportsUploads()) {
        if (uploads === true || typeof uploads === 'undefined') {
          this.uploadsConfig = {};
        } else {
          this.uploadsConfig = uploads;
        }
        //This is here to check if uploads is requested without support. By
        //default we enable them if supported by the integration
      } else if (uploads) {
        throw new Error(
          'This implementation of ApolloServer does not support file uploads because the environmnet cannot accept multi-part forms',
        );
      }
    }

    //Add upload resolver
    if (this.uploadsConfig) {
      if (resolvers && !resolvers.Upload) {
        resolvers.Upload = GraphQLUpload;
      }
    }

@tab00
Copy link
Author

tab00 commented Aug 15, 2018

Yes, uploads: false in ApolloServer options worked, thanks.

So it appears that uploads is true by default, which shouldn't be the case. I think it should be false, as not every Apollo app needs to provide a file upload function.

@sbrichardson
Copy link
Contributor

If you pass typedefs/resolvers separately, and don't pass false for uploads, Apollo will add the upload type, and upload resolver automatically.

In you code, you are currently passing typedefs and resolvers directly to the apollo server options. But below it you are also calling makeExecutableSchema below it.

I can't tell why since the rest of your code is not listed, but if you remove typeDefs/resolvers and replace with schema (like below), Apollo won't add the resolver automatically.

const server = new ApolloServer({
    schema: makeExecutableSchema({ typeDefs, resolvers })
    dataSources: () => ({
      dsBooks: new DSBooks()
    }),
    context: async ({ req }) => ({ user: await getUser(req.headers.authorization) })
  })

@tab00
Copy link
Author

tab00 commented Aug 15, 2018

I merely followed the Apollo Server README which shows passing typedefs and resolvers rather than a schema.

I only needed to call makeExecutableSchema because the ServerOptions argument in SubscriptionServer.create() expects schema, not typedefs and resolvers separately.

I don't think any of this should be relevant to whether Upload is added to resolvers by default.

@sbrichardson
Copy link
Contributor

sbrichardson commented Aug 15, 2018

Yes, I only mentioned since it was unclear why you were instantiating another copy of the schema.

You shouldn't need to define another SubscriptionServer, it's handled internally by ApolloServer

see:

https://github.com/apollographql/apollo-server/blob/master/packages/apollo-server-core/src/ApolloServer.ts#L280

By calling makeExecutableSchema, you are creating a different instance of schema. Since ApolloServer is calling makeExecutableSchema internally when you pass { typeDefs, resolvers }.

You can access the schema at server.schema to reuse that instance with the updated upload functionality, but it's not needed anyways, since SubscriptionServer is handled internally in 2.0.

I realize you don't wish to have the upload, so you can pass false still. I want to ensure that there's not a bug though.

@sbrichardson
Copy link
Contributor

sbrichardson commented Aug 15, 2018

This is the problem line that caused confusion, since it's mutating the original resolvers object,
but not mutating the original query variable.

https://github.com/apollographql/apollo-server/blame/master/packages/apollo-server-core/src/ApolloServer.ts#L151

So if you use makeExecutable schema again, you will have 2 different schemas, without realizing it, or an error with the upload resolver defined, but not in the query type.

In the ApolloServer source:

if (this.uploadsConfig) {
  if (resolvers && !resolvers.Upload) {
    resolvers.Upload = GraphQLUpload; // Mutates original object (not good)
   }
}


// ideally, don't mutate the resolver object

{
  resolvers: {
    ...(resolvers || {}),
    ...(uploads !== false && resolvers && !resolvers.Upload && this.uploadsConfig
       ? { Upload: GraphQLUpload }
       : {}
     )
   }
}

They are not mutating query, but are mutating resolvers

    // we add in the upload scalar, so that schemas that don't include it
        // won't error when we makeExecutableSchema
        typeDefs: this.uploadsConfig
          ? [
              gql`
                scalar Upload
              `,
            ].concat(typeDefs)
          : typeDefs,

@tab00
Copy link
Author

tab00 commented Aug 15, 2018

You shouldn't need to define another SubscriptionServer, it's handled internally by ApolloServer

I've been trying to use Apollo Server's built-in SubscriptionServer (with Meteor's WebApp HTTP server). Is this the correct way?:

  const server = new ApolloServer({
    typeDefs,
    resolvers,
    dataSources: () => ({
      dsBooks: new DSBooks()
    }),
    context: async ({ req }) => ({ user: await getUser(req.headers.authorization) }),
    uploads: false,
    subscriptions: {
      path: "/subscriptions",
      onConnect: async (connectionParams, webSocket) => {
        console.log(`Subscription client connected using Apollo server's built-in SubscriptionServer.`)
      }
    }
  })

  server.installSubscriptionHandlers(WebApp.httpServer)

The client successfully connects, but gets Subscription error: {"message":"Cannot read property 'headers' of undefined"}.

@tab00
Copy link
Author

tab00 commented Aug 15, 2018

I've made a new issue for the problem with using the built-in SubscriptionServer: #1537

@sbrichardson
Copy link
Contributor

Could this issue be closed now?

@tab00 tab00 closed this as completed Aug 18, 2018
@mrdulin
Copy link

mrdulin commented Oct 11, 2019

I met this issue too when I ran the integration tests.

typeDefs.ts:

import { gql } from 'apollo-server';

export const typeDefs = gql`
  enum Device {
    UNKNOWN
    DESKTOP
    HIGH_END_MOBILE
    TABLET
    CONNECTED_TV
  }

  type CampaignPerformanceReport {
    campaignNme: String!
    campaignId: ID!
    device: Device
  }

  type Query {
    campaignPerformanceReports: [CampaignPerformanceReport]!
  }
`;

resolvers.ts:

import { IResolvers } from 'graphql-tools';
import { IAppContext } from './appContext';

export const resolvers: IResolvers = {
  Device: {
    UNKNOWN: 'Other',
    DESKTOP: 'Computers',
    HIGH_END_MOBILE: 'Mobile devices with full browsers',
    TABLET: 'Tablets with full browsers',
    CONNECTED_TV: 'Devices streaming video content to TV screens',
  },
  Query: {
    async campaignPerformanceReports(_, __, { db }: IAppContext) {
      return db.campaignPerformanceReports;
    },
  },
};

The typeDefs and resolvers are very simple. Now, I wrote some integration tests for it.

import { createTestClient, ApolloServerTestClient } from 'apollo-server-testing';
import { ApolloServer, makeExecutableSchema } from 'apollo-server';
import { resolvers } from '../resolvers';
import { typeDefs } from '../typeDefs';
import { db } from '../db';
import { ApolloServerBase, gql, GraphQLResponse } from 'apollo-server-core';
import { printSchema, GraphQLSchema } from 'graphql';

const Q = {
  campaignPerformanceReports: gql`
    query campaignPerformanceReports {
      campaignPerformanceReports {
        campaignNme
        campaignId
        device
      }
    }
  `,
};

describe('custom-scalar-and-enum integration tests', () => {
  describe('Query#campaignPerformanceReports', () => {
    test('should query campaign performance reports correctly with correct device enum value', async () => {
      const server: ApolloServerBase = new ApolloServer({ typeDefs, resolvers, context: { db } });
      // tslint:disable-next-line: no-string-literal
      console.log(printSchema(server['schema'] as GraphQLSchema));
      const { query }: ApolloServerTestClient = createTestClient(server);
      const res: GraphQLResponse = await query({ query: Q.campaignPerformanceReports });
      expect(res).toMatchInlineSnapshot(`
        Object {
          "data": Object {
            "campaignPerformanceReports": Array [
              Object {
                "campaignId": "1",
                "campaignNme": "test",
                "device": "DESKTOP",
              },
            ],
          },
          "errors": undefined,
          "extensions": undefined,
          "http": Object {
            "headers": Headers {
              Symbol(map): Object {},
            },
          },
        }
      `);
    });

    test('should query campaign performance reports correctly with executable graphql schema', async () => {
      const schema = makeExecutableSchema({ typeDefs, resolvers });
      console.log(printSchema(schema));
      const server: ApolloServerBase = new ApolloServer({ schema, context: { db } });
      const { query }: ApolloServerTestClient = createTestClient(server);
      const res: GraphQLResponse = await query({ query: Q.campaignPerformanceReports });
      expect(res).toMatchInlineSnapshot(`
        Object {
          "data": Object {
            "campaignPerformanceReports": Array [
              Object {
                "campaignId": "1",
                "campaignNme": "test",
                "device": "DESKTOP",
              },
            ],
          },
          "errors": undefined,
          "extensions": undefined,
          "http": Object {
            "headers": Headers {
              Symbol(map): Object {},
            },
          },
        }
      `);
    });
  });
});

When I ran these two tests, here is the output:

 FAIL   apollo-graphql-tutorial  src/custom-scalar-and-enum/__tests__/index.integration.spec.ts (6.894s)
  custom-scalar-and-enum integration tests
    Query#campaignPerformanceReports
      ✓ should query campaign performance reports correctly with correct device enum value (55ms)
      ✕ should query campaign performance reports correctly with executable graphql schema (3ms)

  ● custom-scalar-and-enum integration tests › Query#campaignPerformanceReports › should query campaign performance reports correctly with executable graphql schema

    "Upload" defined in resolvers, but not in schema



  console.log src/custom-scalar-and-enum/__tests__/index.integration.spec.ts:26
    enum CacheControlScope {
      PUBLIC
      PRIVATE
    }
    
    type CampaignPerformanceReport {
      campaignNme: String!
      campaignId: ID!
      device: Device
    }
    
    enum Device {
      UNKNOWN
      DESKTOP
      HIGH_END_MOBILE
      TABLET
      CONNECTED_TV
    }
    
    type Query {
      campaignPerformanceReports: [CampaignPerformanceReport]!
    }
    
    """The `Upload` scalar type represents a file upload."""
    scalar Upload
    

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 passed, 2 total
Snapshots:   1 passed, 1 total
Time:        7.114s

In the first test, I passed typeDefs and resolvers to ApolloServer directly. In the second one, I made a executable schema and passed it to ApolloServer.

As you can see, the second test failed and throw error:

"Upload" defined in resolvers, but not in schema

But, if I only run one of the integration tests, the test will pass. So, why this error happened?

Dependencies version:

"apollo": "^2.18.0",
"apollo-server-testing": "^2.9.3",
"apollo-server": "^2.9.3",
"graphql": "^14.5.4",

Yeah, I found a new graphql scalar type Upload is added to my typeDefs, why apollo server do this? My thinking is when the user(developer) pass upload: true to apollo server, then apollo server add Upload scalar type. Why design like this?

Here is the minimal repo to reproduce the issue: https://github.com/mrdulin/apollo-graphql-tutorial/tree/master/src/custom-scalar-and-enum

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

3 participants