Cannot separate route configs for POST and GET requests

[richiejd]

apollo-server/packages/apollo-server-hapi/src/hapiApollo.ts

Line 38 in 98268d1

method: ['GET', 'POST'],

This git issue was closed (incorrectly?). The reason it's an issue is because of the referenced.
#3354

This came up for us when trying to change the max payload size:

  await apolloServer.applyMiddleware({
    app: hapiServer,
    path: pathname,
    route: {
      cors: true,
      payload: { maxBytes: MAX_BYTES },
    },
  });

You cannot set payload on route because the config is used for both the GET and POST requests.
GET requests cannot have payload config.
The plugin should allow specifying different route config for the GET and POST requests.

[glasser]

That makes sense.

There's not a lot of hapi expertise on the Apollo Server core team nor would we be QAing any hapi changes in a real production environment, so this is the kind of change I'd be happy to review if it came in as a PR.

[champagnetony]

I haven't written any tests but here's how I "fixed" this and did some testing with it. The live testing did allow larger byte sizes
Maybe this will help someone get an actual PR with tests completed.

const handler = async (request, h) => {
        try {
            const { graphqlResponse, responseInit } = await runHttpQuery(
                [request, h],
                {
                    method: request.method.toUpperCase(),
                    options: options.graphqlOptions,
                    query:
                        request.method === 'post'
                            ? // TODO type payload as string or Record
                            (request.payload as any)
                            : request.query,
                    request: convertNodeHttpToRequest(request.raw.req),
                },
            );

            const response = h.response(graphqlResponse);
            Object.keys(responseInit.headers).forEach(key =>
                response.header(key, responseInit.headers[key]),
            );
            return response;
        } catch (error) {
            if ('HttpQueryError' !== error.name) {
                throw Boom.boomify(error);
            }

            if (true === error.isGraphQLError) {
                const response = h.response(error.message);
                response.code(error.statusCode);
                response.type('application/json');
                return response;
            }

            const err = new Boom(error.message, { statusCode: error.statusCode });
            if (error.headers) {
                Object.keys(error.headers).forEach(header => {
                    err.output.headers[header] = error.headers[header];
                });
            }
            // Boom hides the error when status code is 500
            err.output.payload.message = error.message;
            throw err;
        }
    };
    server.route({
      method: ['POST'],
      path: options.path || '/graphql',
      vhost: options.vhost || undefined,
      options: options.route || {},
      handler
    });
    if (options.route && options.route.payload) {
        delete options.route.payload;
    }
    server.route({
      method: ['GET'],
      path: options.path || '/graphql',
      vhost: options.vhost || undefined,
      options: options.route || {},
      handler
    });
    

[arimus]

@champagnetony I've created a pull request. Thanks for the pointers.

@glasser I've created a PR to address this ticket and I've validated that it works with a test and actually using it in my project successfully. Let me know if you need anything else, like more robust tests, etc. I did the minimal needed to prove that payload options would make themselves into the POST route configuration and not the GET ones. Not sophisticated, but it allows some more robust configuration in the HAPI configuration simply.

p.s. I have a v2 back port of the same if anyone is really interested. Didn't want to go through the PR hassle unless anyone actually cared though.

[trevor-scheer]

Looks like the person who took interest in resolving this issue (@arimus) is also the person who's currently working on the AS 4 Hapi integration. I recommend keeping an eye out or collaborating with them on the v4 integration.

[arimus]

For anyone stumbling onto this issue, the integration can now be found here: https://github.com/apollo-server-integrations/apollo-server-integration-hapi/

It's still a work in progress. Tests are passing, but there is some needed configuration support necessary.