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

graphql-subscription dependency causing memory leak #749

Closed
urossmolnik opened this issue Nov 17, 2020 · 2 comments
Closed

graphql-subscription dependency causing memory leak #749

urossmolnik opened this issue Nov 17, 2020 · 2 comments
Labels
Question ❔ Not future request, proposal or bug issue Wontfix ❌ This will not be worked on

Comments

@urossmolnik
Copy link

urossmolnik commented Nov 17, 2020

Describe the Bug

graphql-subscriptions dependency has a Promise memory leak that was fixed with pull request: apollographql/graphql-subscriptions#209
Problem is, this pull request is over a year old and it seems it won't be merged in.

We were solving this issue by installing specific graphql-subscriptions version (github:urossmolnik/graphql-subscriptions#v1.3.0) and making sure package-lock file is properly "deduped" (npm dedupe) so version v1.1.0 of graphql-subscriptions package is not installed locally for type-graphql package.

This does not solve our problem anymore with npm 7 - something to do with SemVer for packages installed directly from Git.

To Reproduce

Start a subscriber to a subscription which rejects messages and start publishing messages. You'll notice memory keeps increasing. Memory profile will confirm.
Do the same thing using github:urossmolnik/graphql-subscriptions#v1.3.0 and you'll notice memory is not increasing.

Expected Behavior

Not have a memory leak :)
Not sure what the solution would be. Are we missing something or would removing graphql-subscription dependency be the only solution?

@urossmolnik urossmolnik changed the title graphql-subscription dependency causing memory issues graphql-subscription dependency causing memory leak Nov 17, 2020
@MichalLytek
Copy link
Owner

Problem is, this pull request is over a year old and it seems it won't be merged in.

So it's time for community to fork and maintain the updated version 😉

This does not solve our problem anymore with npm 7 - something to do with SemVer for packages installed directly from Git.

So go ask npm team about overriding dependencies like a yarn can do 😛

Not sure what the solution would be.

I have no idea too 🙄
Maybe in v2.0.0 I'll integrate custom subscriptions engine with compatible API for other pubsub addons (redis, etc.).

@MichalLytek MichalLytek added Need More Info 🤷‍♂️ Further information is requested Question ❔ Not future request, proposal or bug issue labels Nov 18, 2020
@MichalLytek
Copy link
Owner

Closing for a housekeeping purposes 🔒

@MichalLytek MichalLytek added Wontfix ❌ This will not be worked on and removed Need More Info 🤷‍♂️ Further information is requested labels Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question ❔ Not future request, proposal or bug issue Wontfix ❌ This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants