From 713480fb819d7e8c038cc43016c595825228bee0 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Tue, 31 May 2022 10:49:35 +0300 Subject: [PATCH 1/6] align return types from execution and subscription with respect to possible promises --- src/execution/__tests__/subscribe-test.ts | 62 +++++++----- src/execution/subscribe.ts | 114 +++++++++++++++++----- 2 files changed, 129 insertions(+), 47 deletions(-) diff --git a/src/execution/__tests__/subscribe-test.ts b/src/execution/__tests__/subscribe-test.ts index d943ef4006..2e7077ff1a 100644 --- a/src/execution/__tests__/subscribe-test.ts +++ b/src/execution/__tests__/subscribe-test.ts @@ -5,6 +5,8 @@ import { expectJSON } from '../../__testUtils__/expectJSON'; import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick'; import { isAsyncIterable } from '../../jsutils/isAsyncIterable'; +import { isPromise } from '../../jsutils/isPromise'; +import type { PromiseOrValue } from '../../jsutils/PromiseOrValue'; import { parse } from '../../language/parser'; @@ -135,9 +137,6 @@ async function expectPromise(promise: Promise) { } return { - toReject() { - expect(caughtError).to.be.an.instanceOf(Error); - }, toRejectWith(message: string) { expect(caughtError).to.be.an.instanceOf(Error); expect(caughtError).to.have.property('message', message); @@ -152,9 +151,9 @@ const DummyQueryType = new GraphQLObjectType({ }, }); -async function subscribeWithBadFn( +function subscribeWithBadFn( subscribeFn: () => unknown, -): Promise { +): PromiseOrValue { const schema = new GraphQLSchema({ query: DummyQueryType, subscription: new GraphQLObjectType({ @@ -165,13 +164,28 @@ async function subscribeWithBadFn( }), }); const document = parse('subscription { foo }'); - const result = await subscribe({ schema, document }); - assert(!isAsyncIterable(result)); - expectJSON(await createSourceEventStream(schema, document)).toDeepEqual( - result, - ); - return result; + const subscribeResult = subscribe({ schema, document }); + const streamResult = createSourceEventStream(schema, document); + + if (isPromise(subscribeResult)) { + assert(isPromise(streamResult)); + return Promise.all([subscribeResult, streamResult]).then((resolved) => + expectEquivalentStreamErrors(resolved[0], resolved[1]), + ); + } + + assert(!isPromise(streamResult)); + return expectEquivalentStreamErrors(subscribeResult, streamResult); +} + +function expectEquivalentStreamErrors( + subscribeResult: ExecutionResult | AsyncGenerator, + createSourceEventStreamResult: ExecutionResult | AsyncIterable, +): ExecutionResult { + assert(!isAsyncIterable(subscribeResult)); + expectJSON(createSourceEventStreamResult).toDeepEqual(subscribeResult); + return subscribeResult; } /* eslint-disable @typescript-eslint/require-await */ @@ -379,24 +393,22 @@ describe('Subscription Initialization Phase', () => { }); // @ts-expect-error (schema must not be null) - (await expectPromise(subscribe({ schema: null, document }))).toRejectWith( + expect(() => subscribe({ schema: null, document })).to.throw( 'Expected null to be a GraphQL schema.', ); // @ts-expect-error - (await expectPromise(subscribe({ document }))).toRejectWith( + expect(() => subscribe({ document })).to.throw( 'Expected undefined to be a GraphQL schema.', ); // @ts-expect-error (document must not be null) - (await expectPromise(subscribe({ schema, document: null }))).toRejectWith( + expect(() => subscribe({ schema, document: null })).to.throw( 'Must provide document.', ); // @ts-expect-error - (await expectPromise(subscribe({ schema }))).toRejectWith( - 'Must provide document.', - ); + expect(() => subscribe({ schema })).to.throw('Must provide document.'); }); it('resolves to an error if schema does not support subscriptions', async () => { @@ -450,11 +462,11 @@ describe('Subscription Initialization Phase', () => { }); // @ts-expect-error - (await expectPromise(subscribe({ schema, document: {} }))).toReject(); + expect(() => subscribe({ schema, document: {} })).to.throw(); }); it('throws an error if subscribe does not return an iterator', async () => { - expectJSON(await subscribeWithBadFn(() => 'test')).toDeepEqual({ + const expectedResult = { errors: [ { message: @@ -463,7 +475,13 @@ describe('Subscription Initialization Phase', () => { path: ['foo'], }, ], - }); + }; + + expectJSON(subscribeWithBadFn(() => 'test')).toDeepEqual(expectedResult); + + const result = subscribeWithBadFn(() => Promise.resolve('test')); + assert(isPromise(result)); + expectJSON(await result).toDeepEqual(expectedResult); }); it('resolves to an error for subscription resolver errors', async () => { @@ -479,12 +497,12 @@ describe('Subscription Initialization Phase', () => { expectJSON( // Returning an error - await subscribeWithBadFn(() => new Error('test error')), + subscribeWithBadFn(() => new Error('test error')), ).toDeepEqual(expectedResult); expectJSON( // Throwing an error - await subscribeWithBadFn(() => { + subscribeWithBadFn(() => { throw new Error('test error'); }), ).toDeepEqual(expectedResult); diff --git a/src/execution/subscribe.ts b/src/execution/subscribe.ts index e54949830c..526b66459e 100644 --- a/src/execution/subscribe.ts +++ b/src/execution/subscribe.ts @@ -1,12 +1,15 @@ import { inspect } from '../jsutils/inspect'; import { isAsyncIterable } from '../jsutils/isAsyncIterable'; +import { isPromise } from '../jsutils/isPromise'; import type { Maybe } from '../jsutils/Maybe'; +import type { Path } from '../jsutils/Path'; import { addPath, pathToArray } from '../jsutils/Path'; +import type { PromiseOrValue } from '../jsutils/PromiseOrValue'; import { GraphQLError } from '../error/GraphQLError'; import { locatedError } from '../error/locatedError'; -import type { DocumentNode } from '../language/ast'; +import type { DocumentNode, FieldNode } from '../language/ast'; import type { GraphQLFieldResolver } from '../type/definition'; import type { GraphQLSchema } from '../type/schema'; @@ -47,9 +50,11 @@ import { getArgumentValues } from './values'; * * Accepts either an object with named arguments, or individual arguments. */ -export async function subscribe( +export function subscribe( args: ExecutionArgs, -): Promise | ExecutionResult> { +): PromiseOrValue< + AsyncGenerator | ExecutionResult +> { const { schema, document, @@ -61,7 +66,7 @@ export async function subscribe( subscribeFieldResolver, } = args; - const resultOrStream = await createSourceEventStream( + const resultOrStream = createSourceEventStream( schema, document, rootValue, @@ -71,6 +76,42 @@ export async function subscribe( subscribeFieldResolver, ); + if (isPromise(resultOrStream)) { + return resultOrStream.then((resolvedResultOrStream) => + mapSourceToResponse( + schema, + document, + resolvedResultOrStream, + contextValue, + variableValues, + operationName, + fieldResolver, + ), + ); + } + + return mapSourceToResponse( + schema, + document, + resultOrStream, + contextValue, + variableValues, + operationName, + fieldResolver, + ); +} + +function mapSourceToResponse( + schema: GraphQLSchema, + document: DocumentNode, + resultOrStream: ExecutionResult | AsyncIterable, + contextValue?: unknown, + variableValues?: Maybe<{ readonly [variable: string]: unknown }>, + operationName?: Maybe, + fieldResolver?: Maybe>, +): PromiseOrValue< + AsyncGenerator | ExecutionResult +> { if (!isAsyncIterable(resultOrStream)) { return resultOrStream; } @@ -81,7 +122,7 @@ export async function subscribe( // the GraphQL specification. The `execute` function provides the // "ExecuteSubscriptionEvent" algorithm, as it is nearly identical to the // "ExecuteQuery" algorithm, for which `execute` is also used. - const mapSourceToResponse = (payload: unknown) => + return mapAsyncIterator(resultOrStream, (payload: unknown) => execute({ schema, document, @@ -90,10 +131,8 @@ export async function subscribe( variableValues, operationName, fieldResolver, - }); - - // Map every source value to a ExecutionResult value as described above. - return mapAsyncIterator(resultOrStream, mapSourceToResponse); + }), + ); } /** @@ -124,7 +163,7 @@ export async function subscribe( * or otherwise separating these two steps. For more on this, see the * "Supporting Subscriptions at Scale" information in the GraphQL specification. */ -export async function createSourceEventStream( +export function createSourceEventStream( schema: GraphQLSchema, document: DocumentNode, rootValue?: unknown, @@ -132,7 +171,7 @@ export async function createSourceEventStream( variableValues?: Maybe<{ readonly [variable: string]: unknown }>, operationName?: Maybe, subscribeFieldResolver?: Maybe>, -): Promise | ExecutionResult> { +): PromiseOrValue | ExecutionResult> { // If arguments are missing or incorrectly typed, this is an internal // developer mistake which should throw an early error. assertValidExecutionArguments(schema, document, variableValues); @@ -155,7 +194,10 @@ export async function createSourceEventStream( } try { - const eventStream = await executeSubscription(exeContext); + const eventStream = executeSubscription(exeContext); + if (isPromise(eventStream)) { + return eventStream.then(undefined, (error) => ({ errors: [error] })); + } return eventStream; } catch (error) { @@ -163,9 +205,9 @@ export async function createSourceEventStream( } } -async function executeSubscription( +function executeSubscription( exeContext: ExecutionContext, -): Promise> { +): PromiseOrValue | ExecutionResult> { const { schema, fragments, operation, variableValues, rootValue } = exeContext; @@ -220,22 +262,44 @@ async function executeSubscription( // Call the `subscribe()` resolver or the default resolver to produce an // AsyncIterable yielding raw payloads. const resolveFn = fieldDef.subscribe ?? exeContext.subscribeFieldResolver; - const eventStream = await resolveFn(rootValue, args, contextValue, info); - - if (eventStream instanceof Error) { - throw eventStream; - } + const eventStream = resolveFn(rootValue, args, contextValue, info); - // Assert field returned an event stream, otherwise yield an error. - if (!isAsyncIterable(eventStream)) { - throw new GraphQLError( - 'Subscription field must return Async Iterable. ' + - `Received: ${inspect(eventStream)}.`, + if (isPromise(eventStream)) { + return eventStream.then( + (resolvedEventStream) => + ensureAsyncIterable(resolvedEventStream, fieldNodes, path), + (error) => { + throw locatedError(error, fieldNodes, pathToArray(path)); + }, ); } - return eventStream; + return ensureAsyncIterable(eventStream, fieldNodes, path); } catch (error) { throw locatedError(error, fieldNodes, pathToArray(path)); } } + +function ensureAsyncIterable( + eventStream: unknown, + fieldNodes: ReadonlyArray, + path: Path, +): AsyncIterable { + if (eventStream instanceof Error) { + throw locatedError(eventStream, fieldNodes, pathToArray(path)); + } + + // Assert field returned an event stream, otherwise yield an error. + if (!isAsyncIterable(eventStream)) { + throw locatedError( + new GraphQLError( + 'Subscription field must return Async Iterable. ' + + `Received: ${inspect(eventStream)}.`, + ), + fieldNodes, + pathToArray(path), + ); + } + + return eventStream; +} From e9700ca52d37ed7bf9c2584b7cc58c4cd98b872e Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 9 Jun 2022 00:36:20 +0300 Subject: [PATCH 2/6] fix typo --- src/execution/subscribe.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/execution/subscribe.ts b/src/execution/subscribe.ts index 526b66459e..2a2c90a6ee 100644 --- a/src/execution/subscribe.ts +++ b/src/execution/subscribe.ts @@ -207,7 +207,7 @@ export function createSourceEventStream( function executeSubscription( exeContext: ExecutionContext, -): PromiseOrValue | ExecutionResult> { +): PromiseOrValue> { const { schema, fragments, operation, variableValues, rootValue } = exeContext; From ba4c80f6ea12cc3bc98a64d72c4254bd8dc76dd3 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Thu, 9 Jun 2022 14:22:10 +0300 Subject: [PATCH 3/6] refactor suggestion --- src/execution/subscribe.ts | 33 ++++++++++----------------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/src/execution/subscribe.ts b/src/execution/subscribe.ts index 2a2c90a6ee..d1d9a8075a 100644 --- a/src/execution/subscribe.ts +++ b/src/execution/subscribe.ts @@ -2,14 +2,13 @@ import { inspect } from '../jsutils/inspect'; import { isAsyncIterable } from '../jsutils/isAsyncIterable'; import { isPromise } from '../jsutils/isPromise'; import type { Maybe } from '../jsutils/Maybe'; -import type { Path } from '../jsutils/Path'; import { addPath, pathToArray } from '../jsutils/Path'; import type { PromiseOrValue } from '../jsutils/PromiseOrValue'; import { GraphQLError } from '../error/GraphQLError'; import { locatedError } from '../error/locatedError'; -import type { DocumentNode, FieldNode } from '../language/ast'; +import type { DocumentNode } from '../language/ast'; import type { GraphQLFieldResolver } from '../type/definition'; import type { GraphQLSchema } from '../type/schema'; @@ -265,39 +264,27 @@ function executeSubscription( const eventStream = resolveFn(rootValue, args, contextValue, info); if (isPromise(eventStream)) { - return eventStream.then( - (resolvedEventStream) => - ensureAsyncIterable(resolvedEventStream, fieldNodes, path), - (error) => { - throw locatedError(error, fieldNodes, pathToArray(path)); - }, - ); + return eventStream.then(assertEventStream).then(undefined, (error) => { + throw locatedError(error, fieldNodes, pathToArray(path)); + }); } - return ensureAsyncIterable(eventStream, fieldNodes, path); + return assertEventStream(eventStream); } catch (error) { throw locatedError(error, fieldNodes, pathToArray(path)); } } -function ensureAsyncIterable( - eventStream: unknown, - fieldNodes: ReadonlyArray, - path: Path, -): AsyncIterable { +function assertEventStream(eventStream: unknown): AsyncIterable { if (eventStream instanceof Error) { - throw locatedError(eventStream, fieldNodes, pathToArray(path)); + throw eventStream; } // Assert field returned an event stream, otherwise yield an error. if (!isAsyncIterable(eventStream)) { - throw locatedError( - new GraphQLError( - 'Subscription field must return Async Iterable. ' + - `Received: ${inspect(eventStream)}.`, - ), - fieldNodes, - pathToArray(path), + throw new GraphQLError( + 'Subscription field must return Async Iterable. ' + + `Received: ${inspect(eventStream)}.`, ); } From f88b523c175b1c1280dc4659be12426f8f9ea07f Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 9 Jun 2022 21:58:31 +0300 Subject: [PATCH 4/6] apply review feedback = remove unnecessary waits = simply subscribeWithBadFn --- src/execution/__tests__/subscribe-test.ts | 126 ++++++++++++---------- 1 file changed, 70 insertions(+), 56 deletions(-) diff --git a/src/execution/__tests__/subscribe-test.ts b/src/execution/__tests__/subscribe-test.ts index 2e7077ff1a..508278f867 100644 --- a/src/execution/__tests__/subscribe-test.ts +++ b/src/execution/__tests__/subscribe-test.ts @@ -125,25 +125,47 @@ function createSubscription(pubsub: SimplePubSub) { return subscribe({ schema: emailSchema, document, rootValue: data }); } -async function expectPromise(promise: Promise) { - let caughtError: Error; - - try { - /* c8 ignore next 2 */ - await promise; - expect.fail('promise should have thrown but did not'); - } catch (error) { - caughtError = error; - } +function expectPromise(maybePromise: unknown) { + assert(isPromise(maybePromise)); return { - toRejectWith(message: string) { + toResolve() { + return maybePromise; + }, + async toRejectWith(message: string) { + let caughtError: Error; + + try { + /* c8 ignore next 2 */ + await maybePromise; + expect.fail('promise should have thrown but did not'); + } catch (error) { + caughtError = error; + } + expect(caughtError).to.be.an.instanceOf(Error); expect(caughtError).to.have.property('message', message); }, }; } +function expectEqualPromisesOrValues( + value1: PromiseOrValue, + value2: PromiseOrValue, +): PromiseOrValue { + if (isPromise(value1)) { + assert(isPromise(value2)); + return Promise.all([value1, value2]).then((resolved) => { + expectJSON(resolved[1]).toDeepEqual(resolved[0]); + return resolved[0]; + }); + } + + assert(!isPromise(value2)); + expectJSON(value2).toDeepEqual(value1); + return value1; +} + const DummyQueryType = new GraphQLObjectType({ name: 'Query', fields: { @@ -153,7 +175,7 @@ const DummyQueryType = new GraphQLObjectType({ function subscribeWithBadFn( subscribeFn: () => unknown, -): PromiseOrValue { +): PromiseOrValue> { const schema = new GraphQLSchema({ query: DummyQueryType, subscription: new GraphQLObjectType({ @@ -165,27 +187,10 @@ function subscribeWithBadFn( }); const document = parse('subscription { foo }'); - const subscribeResult = subscribe({ schema, document }); - const streamResult = createSourceEventStream(schema, document); - - if (isPromise(subscribeResult)) { - assert(isPromise(streamResult)); - return Promise.all([subscribeResult, streamResult]).then((resolved) => - expectEquivalentStreamErrors(resolved[0], resolved[1]), - ); - } - - assert(!isPromise(streamResult)); - return expectEquivalentStreamErrors(subscribeResult, streamResult); -} - -function expectEquivalentStreamErrors( - subscribeResult: ExecutionResult | AsyncGenerator, - createSourceEventStreamResult: ExecutionResult | AsyncIterable, -): ExecutionResult { - assert(!isAsyncIterable(subscribeResult)); - expectJSON(createSourceEventStreamResult).toDeepEqual(subscribeResult); - return subscribeResult; + return expectEqualPromisesOrValues( + subscribe({ schema, document }), + createSourceEventStream(schema, document), + ); } /* eslint-disable @typescript-eslint/require-await */ @@ -207,7 +212,7 @@ describe('Subscription Initialization Phase', () => { yield { foo: 'FooValue' }; } - const subscription = await subscribe({ + const subscription = subscribe({ schema, document: parse('subscription { foo }'), rootValue: { foo: fooGenerator }, @@ -243,7 +248,7 @@ describe('Subscription Initialization Phase', () => { }), }); - const subscription = await subscribe({ + const subscription = subscribe({ schema, document: parse('subscription { foo }'), }); @@ -281,10 +286,13 @@ describe('Subscription Initialization Phase', () => { }), }); - const subscription = await subscribe({ + const promise = subscribe({ schema, document: parse('subscription { foo }'), }); + assert(isPromise(promise)); + + const subscription = await promise; assert(isAsyncIterable(subscription)); expect(await subscription.next()).to.deep.equal({ @@ -313,7 +321,7 @@ describe('Subscription Initialization Phase', () => { yield { foo: 'FooValue' }; } - const subscription = await subscribe({ + const subscription = subscribe({ schema, document: parse('subscription { foo }'), rootValue: { customFoo: fooGenerator }, @@ -363,7 +371,7 @@ describe('Subscription Initialization Phase', () => { }), }); - const subscription = await subscribe({ + const subscription = subscribe({ schema, document: parse('subscription { foo bar }'), }); @@ -415,7 +423,7 @@ describe('Subscription Initialization Phase', () => { const schema = new GraphQLSchema({ query: DummyQueryType }); const document = parse('subscription { unknownField }'); - const result = await subscribe({ schema, document }); + const result = subscribe({ schema, document }); expectJSON(result).toDeepEqual({ errors: [ { @@ -439,7 +447,7 @@ describe('Subscription Initialization Phase', () => { }); const document = parse('subscription { unknownField }'); - const result = await subscribe({ schema, document }); + const result = subscribe({ schema, document }); expectJSON(result).toDeepEqual({ errors: [ { @@ -479,9 +487,11 @@ describe('Subscription Initialization Phase', () => { expectJSON(subscribeWithBadFn(() => 'test')).toDeepEqual(expectedResult); - const result = subscribeWithBadFn(() => Promise.resolve('test')); - assert(isPromise(result)); - expectJSON(await result).toDeepEqual(expectedResult); + expectJSON( + await expectPromise( + subscribeWithBadFn(() => Promise.resolve('test')), + ).toResolve(), + ).toDeepEqual(expectedResult); }); it('resolves to an error for subscription resolver errors', async () => { @@ -509,12 +519,16 @@ describe('Subscription Initialization Phase', () => { expectJSON( // Resolving to an error - await subscribeWithBadFn(() => Promise.resolve(new Error('test error'))), + await expectPromise( + subscribeWithBadFn(() => Promise.resolve(new Error('test error'))), + ).toResolve(), ).toDeepEqual(expectedResult); expectJSON( // Rejecting with an error - await subscribeWithBadFn(() => Promise.reject(new Error('test error'))), + await expectPromise( + subscribeWithBadFn(() => Promise.reject(new Error('test error'))), + ).toResolve(), ).toDeepEqual(expectedResult); }); @@ -541,7 +555,7 @@ describe('Subscription Initialization Phase', () => { // If we receive variables that cannot be coerced correctly, subscribe() will // resolve to an ExecutionResult that contains an informative error description. - const result = await subscribe({ schema, document, variableValues }); + const result = subscribe({ schema, document, variableValues }); expectJSON(result).toDeepEqual({ errors: [ { @@ -559,10 +573,10 @@ describe('Subscription Publish Phase', () => { it('produces a payload for multiple subscribe in same subscription', async () => { const pubsub = new SimplePubSub(); - const subscription = await createSubscription(pubsub); + const subscription = createSubscription(pubsub); assert(isAsyncIterable(subscription)); - const secondSubscription = await createSubscription(pubsub); + const secondSubscription = createSubscription(pubsub); assert(isAsyncIterable(secondSubscription)); const payload1 = subscription.next(); @@ -601,7 +615,7 @@ describe('Subscription Publish Phase', () => { it('produces a payload per subscription event', async () => { const pubsub = new SimplePubSub(); - const subscription = await createSubscription(pubsub); + const subscription = createSubscription(pubsub); assert(isAsyncIterable(subscription)); // Wait for the next subscription payload. @@ -690,7 +704,7 @@ describe('Subscription Publish Phase', () => { it('produces a payload when there are multiple events', async () => { const pubsub = new SimplePubSub(); - const subscription = await createSubscription(pubsub); + const subscription = createSubscription(pubsub); assert(isAsyncIterable(subscription)); let payload = subscription.next(); @@ -756,7 +770,7 @@ describe('Subscription Publish Phase', () => { it('should not trigger when subscription is already done', async () => { const pubsub = new SimplePubSub(); - const subscription = await createSubscription(pubsub); + const subscription = createSubscription(pubsub); assert(isAsyncIterable(subscription)); let payload = subscription.next(); @@ -810,7 +824,7 @@ describe('Subscription Publish Phase', () => { it('should not trigger when subscription is thrown', async () => { const pubsub = new SimplePubSub(); - const subscription = await createSubscription(pubsub); + const subscription = createSubscription(pubsub); assert(isAsyncIterable(subscription)); let payload = subscription.next(); @@ -863,7 +877,7 @@ describe('Subscription Publish Phase', () => { it('event order is correct for multiple publishes', async () => { const pubsub = new SimplePubSub(); - const subscription = await createSubscription(pubsub); + const subscription = createSubscription(pubsub); assert(isAsyncIterable(subscription)); let payload = subscription.next(); @@ -954,7 +968,7 @@ describe('Subscription Publish Phase', () => { }); const document = parse('subscription { newMessage }'); - const subscription = await subscribe({ schema, document }); + const subscription = subscribe({ schema, document }); assert(isAsyncIterable(subscription)); expect(await subscription.next()).to.deep.equal({ @@ -1015,7 +1029,7 @@ describe('Subscription Publish Phase', () => { }); const document = parse('subscription { newMessage }'); - const subscription = await subscribe({ schema, document }); + const subscription = subscribe({ schema, document }); assert(isAsyncIterable(subscription)); expect(await subscription.next()).to.deep.equal({ @@ -1025,7 +1039,7 @@ describe('Subscription Publish Phase', () => { }, }); - (await expectPromise(subscription.next())).toRejectWith('test error'); + await expectPromise(subscription.next()).toRejectWith('test error'); expect(await subscription.next()).to.deep.equal({ done: true, From 5139ec2f9b719576b65beea0cef72f36c5eb92b6 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 9 Jun 2022 22:11:03 +0300 Subject: [PATCH 5/6] rename eventStream to result until it is asserted as eventStream --- src/execution/subscribe.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/execution/subscribe.ts b/src/execution/subscribe.ts index d1d9a8075a..9ff7cd2112 100644 --- a/src/execution/subscribe.ts +++ b/src/execution/subscribe.ts @@ -261,32 +261,32 @@ function executeSubscription( // Call the `subscribe()` resolver or the default resolver to produce an // AsyncIterable yielding raw payloads. const resolveFn = fieldDef.subscribe ?? exeContext.subscribeFieldResolver; - const eventStream = resolveFn(rootValue, args, contextValue, info); + const result = resolveFn(rootValue, args, contextValue, info); - if (isPromise(eventStream)) { - return eventStream.then(assertEventStream).then(undefined, (error) => { + if (isPromise(result)) { + return result.then(assertEventStream).then(undefined, (error) => { throw locatedError(error, fieldNodes, pathToArray(path)); }); } - return assertEventStream(eventStream); + return assertEventStream(result); } catch (error) { throw locatedError(error, fieldNodes, pathToArray(path)); } } -function assertEventStream(eventStream: unknown): AsyncIterable { - if (eventStream instanceof Error) { - throw eventStream; +function assertEventStream(result: unknown): AsyncIterable { + if (result instanceof Error) { + throw result; } // Assert field returned an event stream, otherwise yield an error. - if (!isAsyncIterable(eventStream)) { + if (!isAsyncIterable(result)) { throw new GraphQLError( 'Subscription field must return Async Iterable. ' + - `Received: ${inspect(eventStream)}.`, + `Received: ${inspect(result)}.`, ); } - return eventStream; + return result; } From 6714bbe814b60636fd6e85527d4b21a544ba783b Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Thu, 9 Jun 2022 22:22:27 +0300 Subject: [PATCH 6/6] add TODOs --- src/execution/__tests__/subscribe-test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/execution/__tests__/subscribe-test.ts b/src/execution/__tests__/subscribe-test.ts index 508278f867..03e3da2839 100644 --- a/src/execution/__tests__/subscribe-test.ts +++ b/src/execution/__tests__/subscribe-test.ts @@ -125,6 +125,7 @@ function createSubscription(pubsub: SimplePubSub) { return subscribe({ schema: emailSchema, document, rootValue: data }); } +// TODO: consider adding this method to testUtils (with tests) function expectPromise(maybePromise: unknown) { assert(isPromise(maybePromise)); @@ -149,6 +150,7 @@ function expectPromise(maybePromise: unknown) { }; } +// TODO: consider adding this method to testUtils (with tests) function expectEqualPromisesOrValues( value1: PromiseOrValue, value2: PromiseOrValue,