From 49a564d723726118ff5a8b448e09b8fea48f202b Mon Sep 17 00:00:00 2001 From: Joao Dias Date: Mon, 1 Jun 2020 18:38:40 +0200 Subject: [PATCH 1/4] test(stitch): add a reproduction for issue #1571 --- packages/stitch/tests/repro1571.test.ts | 105 ++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 packages/stitch/tests/repro1571.test.ts diff --git a/packages/stitch/tests/repro1571.test.ts b/packages/stitch/tests/repro1571.test.ts new file mode 100644 index 00000000000..b231c878b3d --- /dev/null +++ b/packages/stitch/tests/repro1571.test.ts @@ -0,0 +1,105 @@ +import { linkToExecutor } from '@graphql-tools/links'; +import { makeExecutableSchema } from '@graphql-tools/schema'; +import { wrapSchema } from '@graphql-tools/wrap'; +import { ApolloLink, Observable } from 'apollo-link'; +import gql from 'graphql-tag'; + +import { stitchSchemas } from '../src/stitchSchemas'; +import { ExecutionResult, GraphQLError, graphql } from 'graphql'; + +export const typeDefs = gql` + input LoginInput { + username: String + password: String + } + + type LoginPayload { + accessToken: String! + } + + type Query { + meh: Boolean + } + + type Mutation { + login(input: LoginInput!): LoginPayload! + } +`; + +const link = new ApolloLink(operation => { + return new Observable(observer => { + const responses: Record = { + 'whatever@goodpass': { + data: { + login: { + accessToken: 'at', + }, + }, + }, + 'whatever@wrongpass': { + errors: [ + ({ + message: 'INVALID_CREDENTIALS', + path: ['login'], + } as unknown) as GraphQLError, + ], + data: null, + }, + }; + + const response = responses[`${operation.variables.username}@${operation.variables.password}`]; + if (response) { + observer.next(response); + observer.complete(); + } else { + observer.error(new Error('UNEXPECTED_ERROR')); + } + }); +}); + +const authSchema = wrapSchema({ + executor: linkToExecutor(link), + schema: makeExecutableSchema({ typeDefs }), +}); + +const stitchedSchema = stitchSchemas({ + subschemas: [{ schema: authSchema }], +}); + +describe('Repro for issue #1571', () => { + it.each` + username | password | response + ${'whatever'} | ${'goodpass'} | ${{ + data: { + login: { + accessToken: 'at', + }, + }, + }} + ${'whatever'} | ${'wrongpass'} | ${{ + errors: [{ + message: 'INVALID_CREDENTIALS', + path: ['login'], + }], + data: null, + }} + `( + 'should return the expected response for $username@$password', + async ({ username, password, response }: { username: string; password: string; response: string }) => { + const stitchedResult = await graphql( + stitchedSchema, + ` + mutation Login($username: String!, $password: String!) { + login(input: { username: $username, password: $password }) { + accessToken + } + } + `, + undefined, + undefined, + { username, password }, + ); + expect(stitchedResult).toEqual(response); + }, + ); +}); From 2d908814df1ba560c371aa2925f67d1ca72f1947 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Mon, 1 Jun 2020 17:17:55 -0400 Subject: [PATCH 2/4] fix note that even for testing purposes, remote links should return Error objects with paths, which can conveniently be created using the GraphQLError constructor --- packages/stitch/tests/repro1571.test.ts | 99 ++++++++++++++----------- 1 file changed, 54 insertions(+), 45 deletions(-) diff --git a/packages/stitch/tests/repro1571.test.ts b/packages/stitch/tests/repro1571.test.ts index b231c878b3d..cff0bfd27b0 100644 --- a/packages/stitch/tests/repro1571.test.ts +++ b/packages/stitch/tests/repro1571.test.ts @@ -1,13 +1,11 @@ -import { linkToExecutor } from '@graphql-tools/links'; -import { makeExecutableSchema } from '@graphql-tools/schema'; -import { wrapSchema } from '@graphql-tools/wrap'; +import { ExecutionResult, GraphQLError, graphql, buildSchema } from 'graphql'; + import { ApolloLink, Observable } from 'apollo-link'; -import gql from 'graphql-tag'; -import { stitchSchemas } from '../src/stitchSchemas'; -import { ExecutionResult, GraphQLError, graphql } from 'graphql'; +import { linkToExecutor } from '@graphql-tools/links'; +import { stitchSchemas } from '@graphql-tools/stitch'; -export const typeDefs = gql` +const typeDefs = ` input LoginInput { username: String password: String @@ -38,12 +36,19 @@ const link = new ApolloLink(operation => { }, 'whatever@wrongpass': { errors: [ - ({ - message: 'INVALID_CREDENTIALS', - path: ['login'], - } as unknown) as GraphQLError, + // note that even for testing purposes, you have to return an Error object with a path here + // which can conveniently be creating using the GraphQLError constructor + new GraphQLError( + 'INVALID_CREDENTIALS', + undefined, + undefined, + undefined, + ['login'], + ) ], - data: null, + data: { + login: null + }, }, }; @@ -57,49 +62,53 @@ const link = new ApolloLink(operation => { }); }); -const authSchema = wrapSchema({ - executor: linkToExecutor(link), - schema: makeExecutableSchema({ typeDefs }), +const authSchema = stitchSchemas({ + schemas: [{ + schema: buildSchema(typeDefs), + executor: linkToExecutor(link), + }] }); -const stitchedSchema = stitchSchemas({ - subschemas: [{ schema: authSchema }], -}); +const login = (username: string, password: string) => graphql( + authSchema, ` + mutation Login($username: String!, $password: String!) { + login(input: { username: $username, password: $password }) { + accessToken + } + } + `, + undefined, + undefined, + { username, password } +); describe('Repro for issue #1571', () => { - it.each` - username | password | response - ${'whatever'} | ${'goodpass'} | ${{ + it('can log in', async () => { + const expectedResult: ExecutionResult = { data: { login: { accessToken: 'at', }, }, - }} - ${'whatever'} | ${'wrongpass'} | ${{ - errors: [{ - message: 'INVALID_CREDENTIALS', - path: ['login'], - }], + }; + + const result = await login('whatever', 'goodpass'); + expect(result).toEqual(expectedResult); + }); + + it('can receive error', async () => { + const expectedResult: ExecutionResult = { data: null, - }} - `( - 'should return the expected response for $username@$password', - async ({ username, password, response }: { username: string; password: string; response: string }) => { - const stitchedResult = await graphql( - stitchedSchema, - ` - mutation Login($username: String!, $password: String!) { - login(input: { username: $username, password: $password }) { - accessToken - } - } - `, + errors: [new GraphQLError( + 'INVALID_CREDENTIALS', + undefined, undefined, undefined, - { username, password }, - ); - expect(stitchedResult).toEqual(response); - }, - ); + ['login'] + )], + } + + const result = await login('whatever', 'wrongpass'); + expect(result).toEqual(expectedResult); + }); }); From 2fdae0ac21beeaf722b6e28a72e571e59c290fa8 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 2 Jun 2020 00:02:34 -0400 Subject: [PATCH 3/4] properly handle remote (POJO) errors closes #1571 --- packages/delegate/src/results/handleNull.ts | 4 +- packages/stitch/tests/errors.test.ts | 61 ++++++++++- packages/stitch/tests/fixtures/schemas.ts | 42 +++++--- packages/stitch/tests/repro1571.test.ts | 114 -------------------- packages/utils/src/errors.ts | 4 +- packages/wrap/tests/fixtures/schemas.ts | 42 +++++--- 6 files changed, 120 insertions(+), 147 deletions(-) delete mode 100644 packages/stitch/tests/repro1571.test.ts diff --git a/packages/delegate/src/results/handleNull.ts b/packages/delegate/src/results/handleNull.ts index cfea57daf00..791682fc674 100644 --- a/packages/delegate/src/results/handleNull.ts +++ b/packages/delegate/src/results/handleNull.ts @@ -1,6 +1,6 @@ import { GraphQLError } from 'graphql'; -import { getErrorsByPathSegment, CombinedError } from '@graphql-tools/utils'; +import { getErrorsByPathSegment, CombinedError, relocatedError } from '@graphql-tools/utils'; export function handleNull(errors: ReadonlyArray) { if (errors.length) { @@ -10,7 +10,7 @@ export function handleNull(errors: ReadonlyArray) { return combinedError; } const error = errors[0]; - return error.originalError || error; + return error.originalError || relocatedError(error, null); } else if (errors.some(error => typeof error.path[1] === 'string')) { const childErrors = getErrorsByPathSegment(errors); diff --git a/packages/stitch/tests/errors.test.ts b/packages/stitch/tests/errors.test.ts index b690b253881..f9107d9162c 100644 --- a/packages/stitch/tests/errors.test.ts +++ b/packages/stitch/tests/errors.test.ts @@ -1,8 +1,10 @@ -import { graphql } from 'graphql'; +import { graphql, GraphQLError, buildSchema } from 'graphql'; import { makeExecutableSchema } from '@graphql-tools/schema'; import { stitchSchemas } from '../src/stitchSchemas'; +import { ExecutionResult } from '@graphql-tools/utils'; +import { Executor } from 'packages/delegate/src'; describe('passes along errors for missing fields on list', () => { test('if non-null', async () => { @@ -143,3 +145,60 @@ describe('passes along errors when list field errors', () => { expect(stitchedResult).toEqual(originalResult); }); }); + +describe('passes along errors for remote schemas', () => { + it('it works', async () => { + const typeDefs = ` + type Test { + field: String! + } + + type Query { + test: Test! + } + `; + + const schema = buildSchema(typeDefs) + + const executor: Executor = () => ({ + data: { + test: null + }, + errors: [ + { + message: 'INVALID_CREDENTIALS', + path: ['test'], + } as unknown as GraphQLError + ], + }) as ExecutionResult; + + const stitchedSchema = stitchSchemas({ + schemas: [{ + schema, + executor, + }] + }); + + const expectedResult: ExecutionResult = { + data: null, + errors: [ + new GraphQLError( + 'INVALID_CREDENTIALS', + undefined, + undefined, + undefined, + ['test'], + ) + ], + }; + + const query = `{ + test { + field + } + }` + + const result = await graphql(stitchedSchema, query); + expect(result).toEqual(expectedResult); + }); +}); diff --git a/packages/stitch/tests/fixtures/schemas.ts b/packages/stitch/tests/fixtures/schemas.ts index 35dda15dc99..09a5365d3e0 100644 --- a/packages/stitch/tests/fixtures/schemas.ts +++ b/packages/stitch/tests/fixtures/schemas.ts @@ -16,6 +16,7 @@ import { introspectSchema } from '@graphql-tools/wrap'; import { IResolvers, ExecutionResult, + mapAsyncIterator, } from '@graphql-tools/utils'; import { makeExecutableSchema } from '@graphql-tools/schema'; import { PromiseOrValue } from 'graphql/jsutils/PromiseOrValue'; @@ -680,23 +681,36 @@ export const subscriptionSchema: GraphQLSchema = makeExecutableSchema({ }); function makeExecutorFromSchema(schema: GraphQLSchema) { - return async ({ document, variables, context }: ExecutionParams) => graphql( - schema, - print(document), - null, - context, - variables, - ) as PromiseOrValue>; + return async ({ document, variables, context }: ExecutionParams) => { + const result = graphql( + schema, + print(document), + null, + context, + variables, + ) as PromiseOrValue>; + if (result instanceof Promise) { + return result.then(originalResult => JSON.parse(JSON.stringify(originalResult))); + } + return JSON.parse(JSON.stringify(result)); + }; } function makeSubscriberFromSchema(schema: GraphQLSchema) { - return async ({ document, variables, context }: ExecutionParams) => subscribe( - schema, - document, - null, - context, - variables, - ) as Promise> | ExecutionResult> + return async ({ document, variables, context }: ExecutionParams) => { + const result = subscribe( + schema, + document, + null, + context, + variables, + ) as Promise> | ExecutionResult>; + if (result instanceof Promise) { + return result.then(asyncIterator => + mapAsyncIterator(asyncIterator as AsyncIterator, (originalResult: ExecutionResult) => JSON.parse(JSON.stringify(originalResult)))); + } + return JSON.parse(JSON.stringify(result)); + }; } export async function makeSchemaRemote( diff --git a/packages/stitch/tests/repro1571.test.ts b/packages/stitch/tests/repro1571.test.ts deleted file mode 100644 index cff0bfd27b0..00000000000 --- a/packages/stitch/tests/repro1571.test.ts +++ /dev/null @@ -1,114 +0,0 @@ -import { ExecutionResult, GraphQLError, graphql, buildSchema } from 'graphql'; - -import { ApolloLink, Observable } from 'apollo-link'; - -import { linkToExecutor } from '@graphql-tools/links'; -import { stitchSchemas } from '@graphql-tools/stitch'; - -const typeDefs = ` - input LoginInput { - username: String - password: String - } - - type LoginPayload { - accessToken: String! - } - - type Query { - meh: Boolean - } - - type Mutation { - login(input: LoginInput!): LoginPayload! - } -`; - -const link = new ApolloLink(operation => { - return new Observable(observer => { - const responses: Record = { - 'whatever@goodpass': { - data: { - login: { - accessToken: 'at', - }, - }, - }, - 'whatever@wrongpass': { - errors: [ - // note that even for testing purposes, you have to return an Error object with a path here - // which can conveniently be creating using the GraphQLError constructor - new GraphQLError( - 'INVALID_CREDENTIALS', - undefined, - undefined, - undefined, - ['login'], - ) - ], - data: { - login: null - }, - }, - }; - - const response = responses[`${operation.variables.username}@${operation.variables.password}`]; - if (response) { - observer.next(response); - observer.complete(); - } else { - observer.error(new Error('UNEXPECTED_ERROR')); - } - }); -}); - -const authSchema = stitchSchemas({ - schemas: [{ - schema: buildSchema(typeDefs), - executor: linkToExecutor(link), - }] -}); - -const login = (username: string, password: string) => graphql( - authSchema, ` - mutation Login($username: String!, $password: String!) { - login(input: { username: $username, password: $password }) { - accessToken - } - } - `, - undefined, - undefined, - { username, password } -); - -describe('Repro for issue #1571', () => { - it('can log in', async () => { - const expectedResult: ExecutionResult = { - data: { - login: { - accessToken: 'at', - }, - }, - }; - - const result = await login('whatever', 'goodpass'); - expect(result).toEqual(expectedResult); - }); - - it('can receive error', async () => { - const expectedResult: ExecutionResult = { - data: null, - errors: [new GraphQLError( - 'INVALID_CREDENTIALS', - undefined, - undefined, - undefined, - ['login'] - )], - } - - const result = await login('whatever', 'wrongpass'); - expect(result).toEqual(expectedResult); - }); -}); diff --git a/packages/utils/src/errors.ts b/packages/utils/src/errors.ts index 330c4099b89..b3562d94b40 100644 --- a/packages/utils/src/errors.ts +++ b/packages/utils/src/errors.ts @@ -2,13 +2,13 @@ import { GraphQLError } from 'graphql'; export const ERROR_SYMBOL = Symbol('subschemaErrors'); -export function relocatedError(originalError: GraphQLError, path: ReadonlyArray): GraphQLError { +export function relocatedError(originalError: GraphQLError, path?: ReadonlyArray): GraphQLError { return new GraphQLError( originalError.message, originalError.nodes, originalError.source, originalError.positions, - path != null ? path : originalError.path, + path === null ? undefined : path === undefined ? originalError.path : path, originalError.originalError, originalError.extensions ); diff --git a/packages/wrap/tests/fixtures/schemas.ts b/packages/wrap/tests/fixtures/schemas.ts index d134a35e904..bf2fe7df7e0 100644 --- a/packages/wrap/tests/fixtures/schemas.ts +++ b/packages/wrap/tests/fixtures/schemas.ts @@ -16,6 +16,7 @@ import { introspectSchema } from '../../src/introspect'; import { IResolvers, ExecutionResult, + mapAsyncIterator, } from '@graphql-tools/utils'; import { makeExecutableSchema } from '@graphql-tools/schema'; import { SubschemaConfig, ExecutionParams } from '@graphql-tools/delegate'; @@ -680,23 +681,36 @@ export const subscriptionSchema: GraphQLSchema = makeExecutableSchema({ }); function makeExecutorFromSchema(schema: GraphQLSchema) { - return async ({ document, variables, context }: ExecutionParams) => graphql( - schema, - print(document), - null, - context, - variables, - ) as PromiseOrValue>; + return async ({ document, variables, context }: ExecutionParams) => { + const result = graphql( + schema, + print(document), + null, + context, + variables, + ) as PromiseOrValue>; + if (result instanceof Promise) { + return result.then(originalResult => JSON.parse(JSON.stringify(originalResult))); + } + return JSON.parse(JSON.stringify(result)); + }; } function makeSubscriberFromSchema(schema: GraphQLSchema) { - return async ({ document, variables, context }: ExecutionParams) => subscribe( - schema, - document, - null, - context, - variables, - ) as Promise> | ExecutionResult> + return async ({ document, variables, context }: ExecutionParams) => { + const result = subscribe( + schema, + document, + null, + context, + variables, + ) as Promise> | ExecutionResult>; + if (result instanceof Promise) { + return result.then(asyncIterator => + mapAsyncIterator(asyncIterator as AsyncIterator, (originalResult: ExecutionResult) => JSON.parse(JSON.stringify(originalResult)))); + } + return JSON.parse(JSON.stringify(result)); + }; } export async function makeSchemaRemote( From 5b178bf1bd07970efff05b079a7febd4fe00103e Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 2 Jun 2020 06:58:51 -0400 Subject: [PATCH 4/4] fix imports --- packages/stitch/tests/errors.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/stitch/tests/errors.test.ts b/packages/stitch/tests/errors.test.ts index f9107d9162c..c0caf77f734 100644 --- a/packages/stitch/tests/errors.test.ts +++ b/packages/stitch/tests/errors.test.ts @@ -1,10 +1,9 @@ import { graphql, GraphQLError, buildSchema } from 'graphql'; +import { Executor } from '@graphql-tools/delegate'; import { makeExecutableSchema } from '@graphql-tools/schema'; - -import { stitchSchemas } from '../src/stitchSchemas'; +import { stitchSchemas } from '@graphql-tools/stitch'; import { ExecutionResult } from '@graphql-tools/utils'; -import { Executor } from 'packages/delegate/src'; describe('passes along errors for missing fields on list', () => { test('if non-null', async () => {