[apollo-server-koa] sending wrong content-type returns 500 without description

[selaux]

Expected Behavior:
Requesting the graphql endpoint using POST and a content-type header that is not application/json or maybe application/graphql should result in a 400 Bad Request or 415 Unsupported Media Type response from the server with a meaningful response in the body.

Current Behavior:
Requesting the POST endpoint with e.g. content-type header text/plain results in a 500 Internal Server Error response with the following body: {}. Imho this is semantically incorrect (the client did the mistake, not the sever) and not helpful (no explanation of the error at all).

Details:
I looked into this a bit and I cant find where the error is coming from. The request goes into the bodyParser middleware, which resolves with {}. Then the subsequent middleware is not called anymore and I don't know why...

Steps to reproduce:

Install:
npm i [email protected] [email protected] [email protected]

Run:

const Koa = require('koa');
const { ApolloServer } = require('apollo-server-koa');
const supertest = require('supertest');

const RootQuery = 'type RootQuery { ping: String!, pingUserId: String! }';
const SchemaDefinition = 'schema { query: RootQuery }';
const resolvers = {
    RootQuery: {
        ping() {
            return 'pong';
        }
    }
};
const graphqlServerOptions = { typeDefs: [ SchemaDefinition, RootQuery ], resolvers };

const app = new Koa();
const graphqlServer = new ApolloServer(graphqlServerOptions);

graphqlServer.applyMiddleware({
    app,
    path: '/api'
});


async function runExample() {
    const contentType = 'text/plain';
    const server = app.listen();
    const request = supertest(server);
    const response = await request.post('/api')
        .set('Content-Type', contentType)
        .send(JSON.stringify({ query: '{ping}' }));

    console.log(response.statusCode);
    console.log(response.body);

    server.close();
}

runExample();

[kratos-42]

So... any news about this?

[abernix]

Runnable reproduction of the above:

[nihalgonsalves]

This also applies to the default server setup without using any specific webserver (though this means that it's using Express under the hood).

It looks like you receive a 500 unless you have both a valid Content-Type & valid JSON body (an invalid Content-Type has the same effect as not providing the header entirely).

Here are some examples against the default sandbox. I would expect all of these to be 400 errors, and even though the invalid JSON body returns a 400, I would expect that it returns a JSON error, and not an HTML page exposing a stacktrace.

🚫 No Content-Type; 🚫 No body:

$ curl -X POST -sD - https://apollo-server.sse.codesandbox.io/graphql
HTTP/2 500 
server: nginx/1.15.9
date: Tue, 27 Aug 2019 11:14:14 GMT
x-powered-by: Express
access-control-allow-origin: *
strict-transport-security: max-age=15724800; includeSubDomains

POST body missing. Did you forget use body-parser middleware?

✅ Valid Content-Type; 🚫 No body:

HTTP/2 500 
server: nginx/1.15.9
date: Tue, 27 Aug 2019 11:15:56 GMT
x-powered-by: Express
access-control-allow-origin: *
strict-transport-security: max-age=15724800; includeSubDomains

POST body missing. Did you forget use body-parser middleware?

🚫 No Content-Type; ✅ Valid body:

$ curl -X POST -sD - https://apollo-server.sse.codesandbox.io/graphql -d '{ "query": ""}'
HTTP/2 500 
server: nginx/1.15.9
date: Tue, 27 Aug 2019 11:16:38 GMT
x-powered-by: Express
access-control-allow-origin: *
strict-transport-security: max-age=15724800; includeSubDomains

POST body missing. Did you forget use body-parser middleware?

✅ Valid Content-Type; ✅ Valid body:

$ curl -X POST -sD - https://apollo-server.sse.codesandbox.io/graphql -H 'Content-Type: application/json' -d '{ "query": ""}'
HTTP/2 400 
server: nginx/1.15.9
date: Tue, 27 Aug 2019 11:17:34 GMT
x-powered-by: Express
access-control-allow-origin: *
strict-transport-security: max-age=15724800; includeSubDomains

Must provide query string.

✅ Valid Content-Type; 🚫 Malformed body:

$ curl -X POST -sD - https://apollo-server.sse.codesandbox.io/graphql -H 'Content-Type: application/json' -d 'x'
HTTP/2 400 
server: nginx/1.15.9
date: Tue, 27 Aug 2019 11:20:02 GMT
content-type: text/html; charset=utf-8
x-powered-by: Express
access-control-allow-origin: *
content-security-policy: default-src 'self'
x-content-type-options: nosniff
strict-transport-security: max-age=15724800; includeSubDomains

<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>SyntaxError: Unexpected token x in JSON at position 0<br> &nbsp; &nbsp;at JSON.parse (&lt;anonymous&gt;)<br> &nbsp; &nbsp;at createStrictSyntaxError (/sandbox/node_modules/apollo-server-express/node_modules/body-parser/lib/types/json.js:158:10)<br> &nbsp; &nbsp;at parse (/sandbox/node_modules/apollo-server-express/node_modules/body-parser/lib/types/json.js:83:15)<br> &nbsp; &nbsp;at /sandbox/node_modules/apollo-server-express/node_modules/body-parser/lib/read.js:121:18<br> &nbsp; &nbsp;at invokeCallback (/sandbox/node_modules/apollo-server-express/node_modules/raw-body/index.js:224:16)<br> &nbsp; &nbsp;at done (/sandbox/node_modules/apollo-server-express/node_modules/raw-body/index.js:213:7)<br> &nbsp; &nbsp;at IncomingMessage.onEnd (/sandbox/node_modules/apollo-server-express/node_modules/raw-body/index.js:273:7)<br> &nbsp; &nbsp;at IncomingMessage.emit (events.js:203:15)<br> &nbsp; &nbsp;at endReadableNT (_stream_readable.js:1129:12)<br> &nbsp; &nbsp;at process._tickCallback (internal/process/next_tick.js:63:19)</pre>
<script src="https://sse.codesandbox.io/client-hook.js"></script></body>
</html>

[abernix]

@nihalgonsalves Thank you for providing the research you have here. I've tagged this for the Apollo Server 3.x release since I think it highlights some problematic patterns with the Apollo Server 2.x series that we'll need a major version bump to resolve safely.

And specifically, I think any resolution should be built into the transport, which I've proposed in #3184, so as to be universally resolved in all frameworks (e.g. Koa, Express, etc.)

[glasser]

We're no longer planning to resolve #3184 in AS3. I will look into how reasonable it is to resolve this issue specifically in the context of AS3 vs waiting for a release with #3184.

[glasser]

Fixed on release-3.0 by #5305.