From 9352c9a46957a83b13b086ef79e0693589c7daaa Mon Sep 17 00:00:00 2001 From: Paolo Ragone Date: Mon, 4 Feb 2019 22:13:31 +1300 Subject: [PATCH 1/3] Fixes #1709. Based on the PR from @MrLoh but ensuring linting is OK and doing the rest of the homework so it can be merged --- packages/apollo-server-core/CHANGELOG.md | 1 + packages/apollo-server-core/src/runHttpQuery.ts | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/packages/apollo-server-core/CHANGELOG.md b/packages/apollo-server-core/CHANGELOG.md index a188c3d0e47..03ed98ee7bb 100644 --- a/packages/apollo-server-core/CHANGELOG.md +++ b/packages/apollo-server-core/CHANGELOG.md @@ -13,3 +13,4 @@ * `apollo-server-core`: context creation can be async and errors are formatted to include error code [PR #1024](https://github.com/apollographql/apollo-server/pull/1024) * `apollo-server-core`: add `mocks` parameter to the base constructor(applies to all variants) [PR#1017](https://github.com/apollographql/apollo-server/pull/1017) * `apollo-server-core`: Remove printing of stack traces with `debug` option and include response in logging function[PR#1018](https://github.com/apollographql/apollo-server/pull/1018) +* `apollo-server-core`: Returned relevant error codes when an error is thrown by a context function [#1709](https://github.com/apollographql/apollo-server/issues/1709) diff --git a/packages/apollo-server-core/src/runHttpQuery.ts b/packages/apollo-server-core/src/runHttpQuery.ts index 977593bcdd3..876ba260366 100644 --- a/packages/apollo-server-core/src/runHttpQuery.ts +++ b/packages/apollo-server-core/src/runHttpQuery.ts @@ -135,6 +135,11 @@ export async function runHttpQuery( e.extensions.code && e.extensions.code !== 'INTERNAL_SERVER_ERROR' ) { + if (e.extensions.code === 'UNAUTHENTICATED') { + return throwHttpGraphQLError(401, [e], options); + } else if (e.extensions.code === 'FORBIDDEN') { + return throwHttpGraphQLError(403, [e], options); + } return throwHttpGraphQLError(400, [e], options); } else { return throwHttpGraphQLError(500, [e], options); From 98e5a65c9ce8cb5ce0752e993621af945d1084c7 Mon Sep 17 00:00:00 2001 From: Paolo Ragone Date: Mon, 4 Feb 2019 22:31:37 +1300 Subject: [PATCH 2/3] Fixes #1709. Based on the PR from @MrLoh but ensuring linting is OK and doing the rest of the homework so it can be merged --- .../src/__tests__/runHttpQuery.test.ts | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts b/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts index 2f0a1565d0b..8740309fab6 100644 --- a/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts +++ b/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts @@ -4,6 +4,7 @@ import MockReq = require('mock-req'); import { GraphQLSchema, GraphQLObjectType, GraphQLString } from 'graphql'; import { runHttpQuery, HttpQueryError } from '../runHttpQuery'; +import { AuthenticationError, ForbiddenError, ValidationError } from 'apollo-server-core'; const queryType = new GraphQLObjectType({ name: 'QueryType', @@ -46,4 +47,81 @@ describe('runHttpQuery', () => { }); }); }); + describe('error handling in context function', () => { + const mockQueryRequest = { + method: 'GET', + query: { + query: '{ testString }', + }, + options: { + schema, + }, + request: new MockReq(), + }; + + it('raises a 401 error if an AuthenticationError is thrown', () => { + const noQueryRequest = Object.assign({}, mockQueryRequest, { + options: { + ...mockQueryRequest.options, + context: () => { + throw new AuthenticationError('This is the error'); + } + } + }); + + expect.assertions(2); + return runHttpQuery([], noQueryRequest).catch((err: HttpQueryError) => { + expect(err.statusCode).toEqual(401); + expect(err.message.trim()).toEqual("{\"errors\":[{\"message\":\"Context creation failed: This is the error\",\"extensions\":{\"code\":\"UNAUTHENTICATED\"}}]}"); + }); + }); + it('raises a 403 error if a ForbiddenError is thrown', () => { + const noQueryRequest = Object.assign({}, mockQueryRequest, { + options: { + ...mockQueryRequest.options, + context: () => { + throw new ForbiddenError('This is the error'); + } + } + }); + + expect.assertions(2); + return runHttpQuery([], noQueryRequest).catch((err: HttpQueryError) => { + expect(err.statusCode).toEqual(403); + expect(err.message.trim()).toEqual("{\"errors\":[{\"message\":\"Context creation failed: This is the error\",\"extensions\":{\"code\":\"FORBIDDEN\"}}]}"); + }); + }); + it('raises a 400 error if any other GraphQL error is thrown', () => { + const noQueryRequest = Object.assign({}, mockQueryRequest, { + options: { + ...mockQueryRequest.options, + context: () => { + throw new ValidationError('This is the error'); + } + } + }); + + expect.assertions(2); + return runHttpQuery([], noQueryRequest).catch((err: HttpQueryError) => { + expect(err.statusCode).toEqual(400); + expect(err.message.trim()).toEqual("{\"errors\":[{\"message\":\"Context creation failed: This is the error\",\"extensions\":{\"code\":\"GRAPHQL_VALIDATION_FAILED\"}}]}"); + }); + }); + it('raises a 500 error if any other error is thrown', () => { + const noQueryRequest = Object.assign({}, mockQueryRequest, { + options: { + ...mockQueryRequest.options, + context: () => { + throw new Error('This is the error'); + } + } + }); + + expect.assertions(2); + return runHttpQuery([], noQueryRequest).catch((err: HttpQueryError) => { + expect(err.statusCode).toEqual(500); + expect(err.message.trim()).toEqual("{\"errors\":[{\"message\":\"Context creation failed: This is the error\",\"extensions\":{\"code\":\"INTERNAL_SERVER_ERROR\"}}]}"); + }); + }); + }); }); From e77da21fd7ef80bd2080cc5a0475003146603bd1 Mon Sep 17 00:00:00 2001 From: Paolo Ragone Date: Mon, 4 Feb 2019 22:46:01 +1300 Subject: [PATCH 3/3] Made prettier --- .../src/__tests__/runHttpQuery.test.ts | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts b/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts index 8740309fab6..840d4bfbc2a 100644 --- a/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts +++ b/packages/apollo-server-core/src/__tests__/runHttpQuery.test.ts @@ -4,7 +4,11 @@ import MockReq = require('mock-req'); import { GraphQLSchema, GraphQLObjectType, GraphQLString } from 'graphql'; import { runHttpQuery, HttpQueryError } from '../runHttpQuery'; -import { AuthenticationError, ForbiddenError, ValidationError } from 'apollo-server-core'; +import { + AuthenticationError, + ForbiddenError, + ValidationError, +} from 'apollo-server-core'; const queryType = new GraphQLObjectType({ name: 'QueryType', @@ -65,14 +69,16 @@ describe('runHttpQuery', () => { ...mockQueryRequest.options, context: () => { throw new AuthenticationError('This is the error'); - } - } + }, + }, }); expect.assertions(2); return runHttpQuery([], noQueryRequest).catch((err: HttpQueryError) => { expect(err.statusCode).toEqual(401); - expect(err.message.trim()).toEqual("{\"errors\":[{\"message\":\"Context creation failed: This is the error\",\"extensions\":{\"code\":\"UNAUTHENTICATED\"}}]}"); + expect(err.message.trim()).toEqual( + '{"errors":[{"message":"Context creation failed: This is the error","extensions":{"code":"UNAUTHENTICATED"}}]}', + ); }); }); it('raises a 403 error if a ForbiddenError is thrown', () => { @@ -81,14 +87,16 @@ describe('runHttpQuery', () => { ...mockQueryRequest.options, context: () => { throw new ForbiddenError('This is the error'); - } - } + }, + }, }); expect.assertions(2); return runHttpQuery([], noQueryRequest).catch((err: HttpQueryError) => { expect(err.statusCode).toEqual(403); - expect(err.message.trim()).toEqual("{\"errors\":[{\"message\":\"Context creation failed: This is the error\",\"extensions\":{\"code\":\"FORBIDDEN\"}}]}"); + expect(err.message.trim()).toEqual( + '{"errors":[{"message":"Context creation failed: This is the error","extensions":{"code":"FORBIDDEN"}}]}', + ); }); }); it('raises a 400 error if any other GraphQL error is thrown', () => { @@ -97,14 +105,16 @@ describe('runHttpQuery', () => { ...mockQueryRequest.options, context: () => { throw new ValidationError('This is the error'); - } - } + }, + }, }); expect.assertions(2); return runHttpQuery([], noQueryRequest).catch((err: HttpQueryError) => { expect(err.statusCode).toEqual(400); - expect(err.message.trim()).toEqual("{\"errors\":[{\"message\":\"Context creation failed: This is the error\",\"extensions\":{\"code\":\"GRAPHQL_VALIDATION_FAILED\"}}]}"); + expect(err.message.trim()).toEqual( + '{"errors":[{"message":"Context creation failed: This is the error","extensions":{"code":"GRAPHQL_VALIDATION_FAILED"}}]}', + ); }); }); it('raises a 500 error if any other error is thrown', () => { @@ -113,14 +123,16 @@ describe('runHttpQuery', () => { ...mockQueryRequest.options, context: () => { throw new Error('This is the error'); - } - } + }, + }, }); expect.assertions(2); return runHttpQuery([], noQueryRequest).catch((err: HttpQueryError) => { expect(err.statusCode).toEqual(500); - expect(err.message.trim()).toEqual("{\"errors\":[{\"message\":\"Context creation failed: This is the error\",\"extensions\":{\"code\":\"INTERNAL_SERVER_ERROR\"}}]}"); + expect(err.message.trim()).toEqual( + '{"errors":[{"message":"Context creation failed: This is the error","extensions":{"code":"INTERNAL_SERVER_ERROR"}}]}', + ); }); }); });