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

Subscriptions fire multiple times #101

Closed
nikolasburk opened this Issue Jan 10, 2018 · 22 comments

Comments

Projects
None yet
@nikolasburk
Member

nikolasburk commented Jan 10, 2018

I have an issue where the subscriptions are firing multiple times per event when they should only fire once. The number of times the subscriptions fire increases the more requests the server receives.

The issue can be reproduced with this project: https://github.com/howtographql/react-apollo/tree/gc-1.0

@nikolasburk

This comment has been minimized.

Member

nikolasburk commented Jan 22, 2018

Any news on this @schickling?

@tfiwm

This comment has been minimized.

Contributor

tfiwm commented Jan 23, 2018

For me it doesn't get fired :(

@schickling

This comment has been minimized.

Member

schickling commented Jan 25, 2018

@tfiwm can you share a link to your repo so we can reproduce your issue?

@marktani

This comment has been minimized.

Contributor

marktani commented Jan 25, 2018

Might be related to this, @tfiwm: prisma/prisma#1734

@hoodsy

This comment has been minimized.

hoodsy commented Jan 26, 2018

I'm experiencing this as well:

Response from http://localhost:4466/homeroom-api/dev:
{
  "message": {
    "node": {
      "text": "twice?",
      "sender": {
        "id": "cjcq8hd7e00130143cj9sg8mz",
        "name": "Logan Bernard",
        "__typename": "User"
      },
      "__typename": "Message"
    },
    "__typename": "MessageSubscriptionPayload"
  }
}
Response from http://localhost:4466/homeroom-api/dev:
{
  "message": {
    "node": {
      "text": "twice?",
      "sender": {
        "id": "cjcq8hd7e00130143cj9sg8mz",
        "name": "Logan Bernard",
        "__typename": "User"
      },
      "__typename": "Message"
    },
    "__typename": "MessageSubscriptionPayload"
  }
}
Response from http://localhost:4466/homeroom-api/dev:
{
  "message": {
    "node": {
      "text": "twice?",
      "sender": {
        "id": "cjcq8hd7e00130143cj9sg8mz",
        "name": "Logan Bernard",
        "__typename": "User"
      },
      "__typename": "Message"
    },
    "__typename": "MessageSubscriptionPayload"
  }
}
@marktani

This comment has been minimized.

Contributor

marktani commented Jan 28, 2018

@tfiwm found out that this is easily reproduced when refreshing the browser. Then a new connection is created, but old connections are also set up again.

@kbrandwijk

This comment has been minimized.

Contributor

kbrandwijk commented Jan 28, 2018

Is this reproducable in the playground? Or just combined with the apollo-client?

@marktani

This comment has been minimized.

Contributor

marktani commented Jan 28, 2018

It's reproducible with this "subscription client": https://gist.github.com/marktani/5df524523693c88be425bfb623ca8b8a

@marktani

This comment has been minimized.

Contributor

marktani commented Jan 28, 2018

Note that this observation might be a different phenomenon than what @nikolasburk originally reported.

It might be two separate problems, client and server side.

@oxy88

This comment has been minimized.

oxy88 commented Jan 29, 2018

https://github.com/oxy88/prisma_subscription
In my case,

  1. [Prisma only] Subscription fires once
  2. [Prisma with Apollo] Subscriptions fire multiple times
  3. [Turn off Apollo] Subscriptions fire multiple times
  4. [Turn off Apollo and restart Prisma] Subscription fires once
@kbrandwijk

This comment has been minimized.

Contributor

kbrandwijk commented Jan 29, 2018

@oxy88 Your steps description seems to indicate that yoga is not involved here?

@hoodsy

This comment has been minimized.

hoodsy commented Feb 2, 2018

Anyone have an update here? I'm not sure if this is graphql-yoga related or not.

I currently have a work around to ignore duplicate subscription messages, but it's less than ideal 😅

@marktani

This comment has been minimized.

Contributor

marktani commented Feb 2, 2018

We are currently looking into this with a fix coming up soon 🙂

@timsuchanek

This comment has been minimized.

Member

timsuchanek commented Feb 5, 2018

I did a PR to graphql-tools, as soon as it's merged, the problem is fixed. apollographql/graphql-tools#609

@marktani

This comment has been minimized.

Contributor

marktani commented Feb 5, 2018

The fix was merged and is available in version graphql-tools@2.20.0.

Thanks everyone for feedback and help to pinpoint this issue 🙌

@darmie

This comment has been minimized.

darmie commented Mar 22, 2018

This problem still happens while using the managed Graphcool service

@pungggi

This comment has been minimized.

pungggi commented Aug 10, 2018

@darmie maybe worth opening a new issue?

@doums

This comment has been minimized.

doums commented Sep 8, 2018

This problem still happens while using Prisma

@tedb19

This comment has been minimized.

tedb19 commented Nov 23, 2018

any word on this? Still happens when using prisma

@MyroshnykRoman

This comment has been minimized.

MyroshnykRoman commented Dec 12, 2018

I have an issue where the subscriptions are firing multiple times per event when they should only fire once. The number of times the subscriptions fire increases the more requests the server receives.

The issue can be reproduced with this project: https://github.com/howtographql/react-apollo/tree/gc-1.0

@nikolasburk
Hi Nikolas,

I was doing your tutorial with react and apollo on howtographql.com, and I seam to figure out why subscriptions fire multiple times.
It seams to have nothing to do with Prisma or Yoga server, but with component of react-apollo

The number of firing doesn't depend on number of requests on server but rather on number of renderings of Query component in LinkList, each time receiving new data from subscription, LinkList component rerender Query and makes new subscription for the same data, and then get it multiple times per request.

I solved this issue with dirty fix by wrapping subscription functions with flags, that prevent Query from subscribing multiple times.

Here is your LinkList code with my fixes.

`import React, { Component } from 'react'
import Link from './Link'
import { Query } from 'react-apollo'
import gql from 'graphql-tag'

export const FEED_QUERY = gql{ feed { links { id createdAt url description postedBy { id name } votes { id user { id } } } } }

const NEW_LINKS_SUBSCRIPTION = gqlsubscription { newLink { node { id url description createdAt postedBy { id name } votes { id user { id } } } } }

const NEW_VOTES_SUBSCRIPTION = gqlsubscription { newVote { node { id link { id url description createdAt postedBy { id name } votes { id user { id } } } user { id } } } }
class LinkList extends Component {
state = {
subscribedToNewLinks:false,
subscribedToNewVotes:false,
}
_updateCacheAfterVote = (store, createVote, linkId) => {
const data = store.readQuery({ query: FEED_QUERY })

    const votedLink = data.feed.links.find(link => link.id === linkId)
    votedLink.votes = createVote.link.votes

    store.writeQuery({ query: FEED_QUERY, data })
}

_subscribeToNewLinks = subscribeToMore => {
    subscribeToMore({
        document: NEW_LINKS_SUBSCRIPTION,
        updateQuery: (prev, { subscriptionData }) => {
            if (!subscriptionData.data) return prev
            const newLink = subscriptionData.data.newLink.node

            return Object.assign({}, prev, {
                feed: {
                    links: [newLink, ...prev.feed.links],
                    count: prev.feed.links.length + 1,
                    __typename: prev.feed.__typename
                }
            })
        }
    })
    this.setState({subscribedToNewLinks:true})
}

_subscribeToNewVotes = subscribeToMore => {
    subscribeToMore({
        document: NEW_VOTES_SUBSCRIPTION
    })
    this.setState({subscribedToNewVotes:true})
}

render() {

    return (
        <Query query={FEED_QUERY}>
            {({loading, error, data, subscribeToMore }) => {
                if (loading) return <div>Fetching</div>
                if (error) return <div>Error</div>
                if(!this.state.subscribedToNewLinks){
                    this._subscribeToNewLinks(subscribeToMore)
                }
                if(!this.state.subscribedToNewVotes){
                    this._subscribeToNewVotes(subscribeToMore)
                }
                const linksToRender = data.feed.links

                return (
                    <div>
                        {linksToRender.map((link, index) => (
                        <Link
                            key={link.id}
                            link={link}
                            index={index}
                            updateStoreAfterVote={this._updateCacheAfterVote}/>
                        ))}
                    </div>
                )
            }}
        </Query>
    )
}

}

export default LinkList`

I think that problem is that subscriptions should be registered on mount and unregistered on unmount of the component.

Hope that helps,

Best regards,
Myroshnyk Roman

PS. I opened an issue on react-apollo apollographql/react-apollo#2656

@developdeez

This comment has been minimized.

developdeez commented Dec 13, 2018

Was able to do it by passing subscribeToMore into a child component and subscribing in componentDidMount

@MyroshnykRoman

This comment has been minimized.

MyroshnykRoman commented Dec 13, 2018

Was able to do it by passing subscribeToMore into a child component and subscribing in componentDidMount

I think that's valid approach, but you should be careful to unregister subscription on componentWillUnmount in case the whole component will be rerendered upon link change or some other scenarios,
It feels for me, all subscriptions are saved somewhere in singleton of react-apollo, and leaving subscription uregistered may lead to multiple mutations of store on one emitted subscription.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment