Test: Missing subscription field should not be an "internal error"

A remaining issue with subscriptions is attempting to subscribe to a missing field. The existing implementation creates an internal error which is an improper blame since there is no possibility to fix it internally - it is an issue with the query.

For consistency with the same scenario for query/mutation operations, this simply returns nothing. I interpret the equivalent of "undefined" for a subscribe operation as an empty iteration.

Note: this will require spec changes, however spec changes are necessary either way to resolve the ambiguity of this exact scenario.

Author: leebyron

src/subscription/__tests__/subscribe-test.js

@@ -72,7 +72,6 @@ function emailSchemaWithResolvers(subscribeFn, resolveFn) {
           type: EmailEventType,
           resolve: resolveFn,
           subscribe: subscribeFn,
-          // TODO: remove
           args: {
             priority: {type: GraphQLInt}
           }
@@ -347,7 +346,7 @@ describe('Subscription Initialization Phase', () => {
     );
   });
 
-  it('throws an error for unknown root field', async () => {
+  it('unknown field should result in closed subscription', async () => {
     const ast = parse(`
       subscription {
         unknownField
@@ -356,10 +355,10 @@ describe('Subscription Initialization Phase', () => {
 
     const pubsub = new EventEmitter();
 
-    expectPromiseToThrow(
-      () => createSubscription(pubsub, emailSchema, ast),
-      'This subscription is not defined by the schema.'
-    );
+    const { subscription } = await createSubscription(pubsub, emailSchema, ast);
+
+    const payload = await subscription.next();
+    expect(payload).to.deep.equal({ done: true, value: undefined });
   });
 
   it('throws an error if subscribe does not return an iterator', async () => {

src/subscription/emptyAsyncIterator.js

@@ -0,0 +1,32 @@
+/**
+ * Copyright (c) 2017-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+import { $$asyncIterator } from 'iterall';
+
+/**
+ * Returns an AsyncIterable which yields no values.
+ */
+export default function emptyAsyncIterator(): AsyncIterator<void> {
+  // TODO: Flow doesn't support symbols as keys:
+  // https://github.com/facebook/flow/issues/3258
+  return ({
+    next() {
+      return Promise.resolve({ value: undefined, done: true });
+    },
+    return() {
+      return Promise.resolve({ value: undefined, done: true });
+    },
+    throw(error) {
+      return Promise.reject(error);
+    },
+    [$$asyncIterator]() {
+      return this;
+    },
+  }: any);
+}

src/subscription/subscribe.js

@@ -23,7 +23,7 @@ import {
   responsePathAsArray,
 } from '../execution/execute';
 import { GraphQLSchema } from '../type/schema';
-import invariant from '../jsutils/invariant';
+import emptyAsyncIterator from './emptyAsyncIterator';
 import mapAsyncIterator from './mapAsyncIterator';
 
 import type { ObjMap } from '../jsutils/ObjMap';
@@ -227,10 +227,10 @@ export function createSourceEventStream(
     const fieldNodes = fields[responseName];
     const fieldNode = fieldNodes[0];
     const fieldDef = getFieldDef(schema, type, fieldNode.name.value);
-    invariant(
-      fieldDef,
-      'This subscription is not defined by the schema.'
-    );
+
+    if (!fieldDef) {
+      return resolve(emptyAsyncIterator());
+    }
 
     // Call the `subscribe()` resolver or the default resolver to produce an
     // AsyncIterable yielding raw payloads.