graphql-js icon indicating copy to clipboard operation
graphql-js copied to clipboard

Subscriptions do not work with asynchronous resolver function

Open Cito opened this issue 7 years ago • 6 comments

I noticed that the subscribe function does not work properly when it is used with an asynchronous field resolver function.

Also, there is no unit test for this case. This is what I have in mind (must be added to subscribe-test.js):

  it('should work with an asynchronous resolver', async () => {
    const asyncEmailSchema = emailSchemaWithResolvers(
      async function*() {
        yield { email: { subject: 'Hello' } };
      },
      async function*(email) { // <-- this currently does not work
        return email;
      },
    );

    const subscription = await subscribe(
      asyncEmailSchema,
      parse(`
        subscription {
          importantEmail {
            email {
              subject
            }
          }
        }
      `),
    );

    const payload = await subscription.next();
    expect(payload).to.deep.equal({
      done: false,
      value: {
        data: {
          importantEmail: {
            email: {
              subject: 'Hello',
            },
          },
        },
      },
    });

    expect(await subscription.next()).to.deep.equal({
      done: true,
      value: undefined,
    });
  });

Is this an oversight? Or am I doing something stupid or against the spec? As far as I see the ResolveFieldEventStream section says nothing about whether the resolver function can be asynchronous, so I am assuming that is allowed, just at it is for resolver function used for normal queries, and as far as I understand it is intended that they are compatible.

Cito avatar Sep 26 '18 18:09 Cito

can you write a failing test?

jgcmarins avatar Sep 26 '18 19:09 jgcmarins

can you write a failing test?

See above. The test fails - email is null instead of {subject: 'Hello'}.

Cito avatar Sep 26 '18 19:09 Cito

I might be mistaken, but is this Issue not somewhat similar to Issue #1537 ? After kind and clarifying feedback from @Cito it became clear that indeed the above is a different Issue.

asishallab avatar Oct 01 '18 08:10 asishallab

@asishallab this is a different issue, it exists only in the context of a subscription.

Cito avatar Oct 01 '18 13:10 Cito

Hi @Cito

i think things have been reworked a bit since 2018, has this been fixed or otherwise addressed ?

https://github.com/graphql/graphql-js/blob/c7d7026982ceee536900a24ae31235127560297a/src/execution/tests/subscribe-test.ts#L299

yaacovCR avatar May 22 '22 17:05 yaacovCR

Is the old problem that generators return values are a separate beast from the yielded payloads?

yaacovCR avatar May 22 '22 17:05 yaacovCR