Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Client doesn't report the server abruptly going away in the middle of event emission (only in NodeJS) #22

Closed
enisdenjo opened this issue May 3, 2022 · 5 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@enisdenjo
Copy link
Owner

enisdenjo commented May 3, 2022

Kudos, and big thanks, to @brentmjohnson for discovering this bug over at #21!

As per #22 (comment), this issue seems to be exclusive to NodeJS environments.

Expected Behaviour

If the server goes away abruptly during event emission, the client reports the error to the sink.

Actual Behaviour

If the server goes away abruptly during event emission, the client gets stuck and the sink is never finalised.

@enisdenjo enisdenjo added the bug Something isn't working label May 3, 2022
@enisdenjo enisdenjo added the help wanted Extra attention is needed label May 16, 2022
@enisdenjo
Copy link
Owner Author

enisdenjo commented Jun 1, 2022

Hmm, seems like this bug is exclusive to NodeJS. I've tried replicating the same behaviour in the browser, but it fails immediately when the server gets killed. Here are my repro steps:

  1. Write up a test.mjs server:
import { GraphQLSchema, GraphQLObjectType, GraphQLString } from 'graphql';
import http from 'http';
import { createHandler } from 'graphql-sse';

const schema = new GraphQLSchema({
  query: new GraphQLObjectType({
    name: 'Query',
    fields: {
      hello: {
        type: GraphQLString,
        resolve: () => 'world',
      },
    },
  }),
  subscription: new GraphQLObjectType({
    name: 'Subscription',
    fields: {
      greetings: {
        type: GraphQLString,
        subscribe: async function* () {
          for (const hi of ['Hi', 'Bonjour', 'Hola', 'Ciao', 'Zdravo']) {
            yield { greetings: hi };
            await new Promise((resolve) => setTimeout(resolve, 3000));
          }
        },
      },
    },
  }),
});

const handler = createHandler({ schema });

const server = http.createServer(async (req, res) => {
  if (!req.url.startsWith('/graphql/stream')) {
    return res.writeHead(404).end();
  }

  try {
    res.setHeader('Access-Control-Allow-Origin', '*');
    res.setHeader('Access-Control-Allow-Methods', '*');
    res.setHeader('Access-Control-Allow-Headers', '*');
    if (req.method === 'OPTIONS') {
      // preflight
      return res.writeHead(204).end();
    }

    return await handler(req, res);
  } catch (err) {
    console.error(err);
    if (!res.headersSent) {
      res.writeHead(500);
    }
    return res.end();
  }
});

server.listen(4000);
console.log('Listening to port 4000');
  1. node test.mjs
  2. Load GraphiQL in the browser using this gist
  3. Subscribe to greetings:
subscription {
  greetings
}
  1. Kill the server whenever after the 1st event emission
  2. The browser immediately reports POST http://localhost:4000/graphql/stream net::ERR_INCOMPLETE_CHUNKED_ENCODING 200 (OK) to the console (also when running the client in "single connection mode")

I've Googled around but couldn't find any clues about why this test fails:

it('should report error to sink if server goes away during generator emission', async () => {
const server = await startTServer();
const client = createClient({
url: server.url,
fetchFn: fetch,
retryAttempts: 0,
});
const sub = tsubscribe(client, {
query: 'subscription { slowGreetings }',
});
await sub.waitForNext();
await server.dispose();
// TODO: should waitForError; but for debugging purposes, we wait for either
await Promise.race([sub.waitForError(), sub.waitForComplete()]);
});

Could it be node-fetch or just Node itself? Any ideas @brentmjohnson?

@brentmjohnson
Copy link

Sure I'll take a look @enisdenjo - will let you know what I find.

@enisdenjo enisdenjo changed the title Client doesn't report the server abruptly going away in the middle of event emission Client doesn't report the server abruptly going away in the middle of event emission (only in NodeJS) Jun 9, 2022
@enisdenjo
Copy link
Owner Author

I am closing this issue with 8f002f1 because the issue stems from v2 of node-fetch and is fixed with v3.

See node-fetch/node-fetch#1604.

@enisdenjo

This comment was marked as resolved.

@enisdenjo enisdenjo added released Has been released and published and removed released Has been released and published labels Jul 17, 2022
@brentmjohnson
Copy link

I am closing this issue with 8f002f1 because the issue stems from v2 of node-fetch and is fixed with v3.

See node-fetch/node-fetch#1604.

@enisdenjo what a great find. Thank you so much for your persistence in identifying the resolution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants