Infinite retries #55
Comments
|
Hey there, thanks for reporting!
The thing here is that after connecting to the server, the subscribe is considered a success. This is because you may subscribe to something that never emits (for example subscribing to chat messages but no one ever sending you a message). So, your bug feels possible - but the fix might be wrong... I am just thinking of how to create a failing test that is senseful so that a proper fix can be placed. Any suggestions? How does your server fail the connection? |
|
I was able to pinpoint the problem in my server. I will try and create a small poc |
|
@esfomeado any luck with the repro? |
|
@enisdenjo I will try get it done this weekend. |
|
I was not able to create a perfect replication of my original problem but I created something close. Run the server and the client. Navigate to Refresh the page (try a few times if doesn't work at first) You will see that no data is longer logged and you will see the client reconnecting over and over. |
|
Hi! this is also happening to me, is there a workaround for this? |
|
Just wanted to ping you guys that I've managed to assemble a failing test in #57 and am working on a fix. Thank you guys for reporting and for your patience! |
|
Thanks @enisdenjo , timing couldn't be better. If you need any help testing this, I am happy to help. Thank you! |
|
This is fixed and merged. However, GitHub is having issues currently - so I'll delay releasing a new version until issues are resolved. Thanks for understanding! |
|
Thanks for the fix @enisdenjo |
Thank you very much! |
|
🎉 This issue has been resolved in version 2.1.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
GitHub's still shaky, but I've pushed a new version nevertheless to unblock you guys. Try it out! |
It works great! Thanks!! |
|
Hi @enisdenjo not sure how, but this problem is still hapening to me on 2.1.2. But Im unable to reproduce it :( |
|
Oh, sad to hear that @2snEM6. Are you sure you updated the lib to 2.1.2? Is it the same problem? I would need your support to get it fixed, a failing test or a concrete repro would be of great help - otherwise I am in the dark. |
|
Yes, on 2.1.2. I will try to create a small server to reproduce it, but it's hard. I saw that if the NodeJS subscriptions server crashes abruptly, I get a 404 error on the frontend, meaning the stream cannot be found. I will share more information when I have it available |
|
@enisdenjo just happened to me again and I was able to see the error thrown on the server: I am running a NodeJS GraphQL Server (GraphQL Yoga) with subscriptions enabled with https://www.npmjs.com/package/graphql-redis-subscriptions pub sub package and what is timing out is the subscription client to the pub sub right in the subscription definition. For instance: aSubscriptionName: {
type: aGraphQLReturnType,
pubsub.asyncIterator([
'an-event-name'
]),
resolve: (eventData, _, context) => {
// resolver
},
},A command timeout to the Redis pub sub server is making my server crash abruptly. Of course the server should not crash this way but the frontend shouldn't be hitting the server with very high frequency calls. It should stop retrying at some point. I guess the way to reproduce this is creating a simple GraphQL Yoga server running on NodeJS + redis pubsub. Subscribing and then turning redis off |
|
@enisdenjo I forgot to say Im using single connection mode, it's crashing there |
|
Thanks for sharing, this is honestly too much of a setup to debug only graphql-sse. I'd really appreciate a repro. Furthermore, can you share your Yoga setup? Are you using the In the meantime, I'll see whether I can get a similar failing test to #57 for single connection mode. |
## [2.1.3](v2.1.2...v2.1.3) (2023-05-15) ### Bug Fixes * **client:** Respect retry attempts when server goes away after connecting in single connection mode ([#59](#59)) ([e895c5b](e895c5b)), closes [#55](#55) * **handler:** Detect `ExecutionArgs` in `onSubscribe` return value ([a16b921](a16b921)), closes [#58](#58)
|
🎉 This issue has been resolved in version 2.1.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
That is correct! I've tried to set up a simple repro repo but I fail to reproduce the issue.
Thank you very much! I will give it a try and let you know 🙏🙏 |
# 1.0.0 (2024-10-10) ### Bug Fixes * Add file extensions to imports/exports in ESM type definitions ([bbf23b1](bbf23b1)) * Add koa exports to package.json ([enisdenjo#85](https://github.com/CandisIO/graphql-sse/issues/85)) ([e99cf99](e99cf99)) * Add support for `graphql@v16` ([89367f2](89367f2)) * Add types path to package.json `exports` ([44f95b6](44f95b6)) * Bump `graphql` version to v16 in package.json ([af219f9](af219f9)) * **client:** Abort request when reporting error ([91057bd](91057bd)) * **client:** Avoid bundling DOM types, have the implementor supply his own `Response` type ([98780c0](98780c0)) * **client:** Leverage active streams for reliable network error retries ([607b468](607b468)) * **client:** Network errors during event emission contain the keyword "stream" in Firefox ([054f16b](054f16b)) * **client:** Operation requests are of application/json content-type ([0084de7](0084de7)) * **client:** Respect retry attempts when server goes away after connecting ([enisdenjo#57](https://github.com/CandisIO/graphql-sse/issues/57)) ([75c9f17](75c9f17)), closes [enisdenjo#55](https://github.com/CandisIO/graphql-sse/issues/55) * **client:** Respect retry attempts when server goes away after connecting in single connection mode ([enisdenjo#59](https://github.com/CandisIO/graphql-sse/issues/59)) ([e895c5b](e895c5b)), closes [enisdenjo#55](https://github.com/CandisIO/graphql-sse/issues/55) * **client:** Retry if connection is closed while having active streams ([83a0178](83a0178)), closes [enisdenjo#28](https://github.com/CandisIO/graphql-sse/issues/28) * **client:** Retry network errors even if they occur during event emission ([489b1b0](489b1b0)), closes [enisdenjo#27](https://github.com/CandisIO/graphql-sse/issues/27) * **client:** Should not call complete after subscription error ([d8b7634](d8b7634)) * **client:** TypeScript generic for ensuring proper arguments when using "single connection mode" ([be2ae7d](be2ae7d)) * **client:** Use closures instead of bindings (with `this`) ([8ecdf3c](8ecdf3c)) * Define graphql execution results ([89da803](89da803)) * **handler:** Always include the `data` field in stream messages ([enisdenjo#71](https://github.com/CandisIO/graphql-sse/issues/71)) ([4643c9a](4643c9a)) * **handler:** Correct typings and support for http2 ([08d6ca3](08d6ca3)), closes [enisdenjo#38](https://github.com/CandisIO/graphql-sse/issues/38) * **handler:** Detect `ExecutionArgs` in `onSubscribe` return value ([a16b921](a16b921)), closes [enisdenjo#58](https://github.com/CandisIO/graphql-sse/issues/58) * **handler:** Support generics for requests and responses ([9ab10c0](9ab10c0)) * **handler:** Use 3rd `body` argument only if is object or string ([2062579](2062579)) * Prefer `X-GraphQL-Event-Stream-Token` header name for clarity ([9aaa0a9](9aaa0a9)) * remove package.json workspaces entry in release ([c6dc093](c6dc093)) * Request parameters `query` field can only be a string ([16c9600](16c9600)), closes [enisdenjo#65](https://github.com/CandisIO/graphql-sse/issues/65) * **server:** Operation result can be async generator or iterable ([24b6078](24b6078)) * **use/express,use/fastify:** Resolve body if previously parsed ([6573e94](6573e94)) * **use/express:** make sure that we not send something through previously closed stream ([e4b8c5f](e4b8c5f)) * **use/fastify:** Include middleware headers ([134c1b0](134c1b0)), closes [enisdenjo#91](https://github.com/CandisIO/graphql-sse/issues/91) * **use/http,use/http2,use/express,use/fastify:** Check `writable` instead of `closed` before writing to response ([3c71f69](3c71f69)), closes [enisdenjo#69](https://github.com/CandisIO/graphql-sse/issues/69) * **use/http,use/http2,use/express,use/fastify:** Handle cases where response's `close` event is late ([enisdenjo#75](https://github.com/CandisIO/graphql-sse/issues/75)) ([4457cba](4457cba)), closes [enisdenjo#69](https://github.com/CandisIO/graphql-sse/issues/69) * **use/koa:** Use parsed body from request ([enisdenjo#87](https://github.com/CandisIO/graphql-sse/issues/87)) ([b290b90](b290b90)) ### Features * Client ([enisdenjo#3](https://github.com/CandisIO/graphql-sse/issues/3)) ([754487d](754487d)) * **client:** Accept `referrer` and `referrerPolicy` fetch options ([enisdenjo#32](https://github.com/CandisIO/graphql-sse/issues/32)) ([dbaa90a](dbaa90a)) * **client:** Add `credentials` property for requests ([79d0266](79d0266)) * **client:** Add `lazyCloseTimeout` as a close timeout after last operation completes ([16e5e31](16e5e31)), closes [enisdenjo#17](https://github.com/CandisIO/graphql-sse/issues/17) * **client:** Async iterator for subscriptions ([enisdenjo#66](https://github.com/CandisIO/graphql-sse/issues/66)) ([fb8bf11](fb8bf11)) * **client:** Event listeners for both operation modes ([enisdenjo#84](https://github.com/CandisIO/graphql-sse/issues/84)) ([6274f44](6274f44)) * **client:** Inspect incoming messages through `ClientOptions.onMessage` ([496e74b](496e74b)), closes [enisdenjo#20](https://github.com/CandisIO/graphql-sse/issues/20) * **handler:** Export handler options type for each integration ([2a2e517](2a2e517)) * **handler:** Server and environment agnostic handler ([enisdenjo#37](https://github.com/CandisIO/graphql-sse/issues/37)) ([22cf03d](22cf03d)) * **handler:** Use Koa ([enisdenjo#80](https://github.com/CandisIO/graphql-sse/issues/80)) ([283b453](283b453)), closes [enisdenjo#78](https://github.com/CandisIO/graphql-sse/issues/78) * Server request handler ([enisdenjo#2](https://github.com/CandisIO/graphql-sse/issues/2)) ([8381796](8381796)) * **use/koa:** expose full Koa context to options ([enisdenjo#86](https://github.com/CandisIO/graphql-sse/issues/86)) ([b37a6f9](b37a6f9)) ### Performance Improvements * **client:** Avoid recreating result variables when reading the response stream ([16f6a6c](16f6a6c)) ### BREAKING CHANGES * **handler:** The handler is now server agnostic and can run _anywhere_ - Core of `graphql-sse` is now server agnostic and as such offers a handler that implements a generic request/response model - Handler does not await for whole operation to complete anymore. Only the processing part (parsing, validating and executing) - GraphQL context is now typed - Hook arguments have been changed, they're not providing the Node native req/res anymore - they instead provide the generic request/response - `onSubscribe` hook can now return an execution result too (useful for caching for example) - Throwing in `onNext` and `onComplete` hooks will bubble the error to the returned iterator ### Migration Even though the core of graphql-sse is now completely server agnostic, there are adapters to ease the integration with existing solutions. Migrating is actually not a headache! Beware that the adapters **don't** handle internal errors, it's your responsibility to take care of that and behave accordingly. #### [`http`](https://nodejs.org/api/http.html) ```diff import http from 'http'; - import { createHandler } from 'graphql-sse'; + import { createHandler } from 'graphql-sse/lib/use/http'; // Create the GraphQL over SSE handler const handler = createHandler({ schema }); // Create an HTTP server using the handler on `/graphql/stream` const server = http.createServer((req, res) => { if (req.url.startsWith('/graphql/stream')) { return handler(req, res); } res.writeHead(404).end(); }); server.listen(4000); console.log('Listening to port 4000'); ``` #### [`http2`](https://nodejs.org/api/http2.html) ```diff import fs from 'fs'; import http2 from 'http2'; - import { createHandler } from 'graphql-sse'; + import { createHandler } from 'graphql-sse/lib/use/http2'; // Create the GraphQL over SSE handler const handler = createHandler({ schema }); // Create an HTTP server using the handler on `/graphql/stream` const server = http.createServer((req, res) => { if (req.url.startsWith('/graphql/stream')) { return handler(req, res); } res.writeHead(404).end(); }); server.listen(4000); console.log('Listening to port 4000'); ``` #### [`express`](https://expressjs.com/) ```diff import express from 'express'; // yarn add express - import { createHandler } from 'graphql-sse'; + import { createHandler } from 'graphql-sse/lib/use/express'; // Create the GraphQL over SSE handler const handler = createHandler({ schema }); // Create an express app const app = express(); // Serve all methods on `/graphql/stream` app.use('/graphql/stream', handler); server.listen(4000); console.log('Listening to port 4000'); ``` #### [`fastify`](https://www.fastify.io/) ```diff import Fastify from 'fastify'; // yarn add fastify - import { createHandler } from 'graphql-sse'; + import { createHandler } from 'graphql-sse/lib/use/fastify'; // Create the GraphQL over SSE handler const handler = createHandler({ schema }); // Create a fastify app const fastify = Fastify(); // Serve all methods on `/graphql/stream` fastify.all('/graphql/stream', handler); fastify.listen({ port: 4000 }); console.log('Listening to port 4000'); ```
Expected Behaviour
The client should only retry up to the specified retry number.
Actual Behaviour
The client retries to infinite and beyond with the error
Connection closed while having active streams.Debug Information
I´m only able to reproduce this problem on my application that I can´t share but the issue lays here https://github.com/enisdenjo/graphql-sse/blob/master/src/client.ts#L476
Before getting the results we clear the number of retries but if the client fails to get the results back (which is the problem in my case) it will keep retrying over and over.
I think the number of the retries should only be cleared after successfully getting the results back.
The text was updated successfully, but these errors were encountered: