From ee42a534c3e297f09ccadb7e2961226a82859fc0 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Sun, 4 Apr 2021 23:26:13 +0300 Subject: [PATCH] subscribe: drop mapping of AsyncIterable errors --- .../__tests__/mapAsyncIterator-test.js | 62 ------------------- src/subscription/__tests__/subscribe-test.js | 60 ------------------ src/subscription/mapAsyncIterator.js | 17 +---- src/subscription/subscribe.js | 7 +-- 4 files changed, 4 insertions(+), 142 deletions(-) diff --git a/src/subscription/__tests__/mapAsyncIterator-test.js b/src/subscription/__tests__/mapAsyncIterator-test.js index 80f6e45a94..7341595ada 100644 --- a/src/subscription/__tests__/mapAsyncIterator-test.js +++ b/src/subscription/__tests__/mapAsyncIterator-test.js @@ -269,35 +269,6 @@ describe('mapAsyncIterator', () => { .with.property('message', 'Goodbye'); }); - it('maps over thrown errors if second callback provided', async () => { - async function* source() { - yield 'Hello'; - throw new Error('Goodbye'); - } - - const doubles = mapAsyncIterator( - source(), - (x) => x + x, - (error) => error, - ); - - expect(await doubles.next()).to.deep.equal({ - value: 'HelloHello', - done: false, - }); - - const result = await doubles.next(); - expect(result.value) - .to.be.an.instanceOf(Error) - .with.property('message', 'Goodbye'); - expect(result.done).to.equal(false); - - expect(await doubles.next()).to.deep.equal({ - value: undefined, - done: true, - }); - }); - async function testClosesSourceWithMapper(mapper: (number) => T) { let didVisitFinally = false; @@ -353,37 +324,4 @@ describe('mapAsyncIterator', () => { : Promise.resolve(x), ); }); - - it('closes source if mapper throws an error', async () => { - async function* source() { - yield 1; - throw new Error(2); - } - - const throwOver1 = mapAsyncIterator( - source(), - (x) => x, - (error) => { - throw new Error('Cannot count to ' + error.message); - }, - ); - - expect(await throwOver1.next()).to.deep.equal({ value: 1, done: false }); - - let expectedError; - try { - await throwOver1.next(); - } catch (error) { - expectedError = error; - } - - expect(expectedError) - .to.be.an.instanceOf(Error) - .with.property('message', 'Cannot count to 2'); - - expect(await throwOver1.next()).to.deep.equal({ - value: undefined, - done: true, - }); - }); }); diff --git a/src/subscription/__tests__/subscribe-test.js b/src/subscription/__tests__/subscribe-test.js index 446db6b564..54aa6714e0 100644 --- a/src/subscription/__tests__/subscribe-test.js +++ b/src/subscription/__tests__/subscribe-test.js @@ -9,8 +9,6 @@ import { isAsyncIterable } from '../../jsutils/isAsyncIterable'; import type { DocumentNode } from '../../language/ast'; import { parse } from '../../language/parser'; -import { GraphQLError } from '../../error/GraphQLError'; - import { GraphQLSchema } from '../../type/schema'; import { GraphQLList, GraphQLObjectType } from '../../type/definition'; import { GraphQLInt, GraphQLString, GraphQLBoolean } from '../../type/scalars'; @@ -1031,62 +1029,4 @@ describe('Subscription Publish Phase', () => { value: undefined, }); }); - - it('should resolve GraphQL error from source event stream', async () => { - async function* generateEmails() { - yield { email: { subject: 'Hello' } }; - throw new GraphQLError('test error'); - } - - const erroringEmailSchema = emailSchemaWithResolvers( - generateEmails, - (email) => email, - ); - - const subscription = await subscribe({ - schema: erroringEmailSchema, - document: parse(` - subscription { - importantEmail { - email { - subject - } - } - } - `), - }); - invariant(isAsyncIterable(subscription)); - - const payload1 = await subscription.next(); - expect(payload1).to.deep.equal({ - done: false, - value: { - data: { - importantEmail: { - email: { - subject: 'Hello', - }, - }, - }, - }, - }); - - const payload2 = await subscription.next(); - expect(payload2).to.deep.equal({ - done: false, - value: { - errors: [ - { - message: 'test error', - }, - ], - }, - }); - - const payload3 = await subscription.next(); - expect(payload3).to.deep.equal({ - done: true, - value: undefined, - }); - }); }); diff --git a/src/subscription/mapAsyncIterator.js b/src/subscription/mapAsyncIterator.js index f30a4ac513..b76a4cb35f 100644 --- a/src/subscription/mapAsyncIterator.js +++ b/src/subscription/mapAsyncIterator.js @@ -7,9 +7,6 @@ import type { PromiseOrValue } from '../jsutils/PromiseOrValue'; export function mapAsyncIterator( iterable: AsyncIterable | AsyncGenerator, callback: (T) => PromiseOrValue, - rejectCallback: (any) => U = (error) => { - throw error; - }, ): AsyncGenerator { // $FlowFixMe[prop-missing] const iteratorMethod = iterable[Symbol.asyncIterator]; @@ -38,28 +35,20 @@ export function mapAsyncIterator( } } - function mapReject(error: mixed) { - try { - return { value: rejectCallback(error), done: false }; - } catch (callbackError) { - return abruptClose(callbackError); - } - } - /* TODO: Flow doesn't support symbols as keys: https://github.com/facebook/flow/issues/3258 */ return ({ next(): Promise> { - return iterator.next().then(mapResult, mapReject); + return iterator.next().then(mapResult, abruptClose); }, return() { return typeof iterator.return === 'function' - ? iterator.return().then(mapResult, mapReject) + ? iterator.return().then(mapResult, abruptClose) : Promise.resolve({ value: undefined, done: true }); }, throw(error?: mixed): Promise> { if (typeof iterator.throw === 'function') { - return iterator.throw(error).then(mapResult, mapReject); + return iterator.throw(error).then(mapResult, abruptClose); } return Promise.reject(error).catch(abruptClose); }, diff --git a/src/subscription/subscribe.js b/src/subscription/subscribe.js index 1fa1eefc6a..94f1f83a00 100644 --- a/src/subscription/subscribe.js +++ b/src/subscription/subscribe.js @@ -104,12 +104,7 @@ export async function subscribe( }); // Map every source value to a ExecutionResult value as described above. - return mapAsyncIterator(resultOrStream, mapSourceToResponse, (error) => { - if (error instanceof GraphQLError) { - return { errors: [error] }; - } - throw error; - }); + return mapAsyncIterator(resultOrStream, mapSourceToResponse); } /**