apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.36k stars 2.66k forks source link

Meteor/Apollo/React/SSR Memory leak #8101

Closed Gorbus closed 3 years ago

Gorbus commented 3 years ago

Intended outcome: Having a meteor / apollo / react / ssr app running without memory leak

Actual outcome: The app works without any issue without SSR, when I add the SSR with the following code, it has a clear memory leak (Restarting the app regularly to empty it to avoid peak level where the server can't bear anymore): image

I (re)added the ssr code on the 1st, we can clearly see the leak and that it isn't here without the SSR. The only code difference is, the ssr.jsx file here under, and the fact I use hydrate instead of render in the main.jsx

How to reproduce the issue: This is the SSR code:

import { onPageLoad } from "meteor/server-render";
import React from "react";
import { renderToString } from "react-dom/server";
import { getMarkupFromTree } from "@apollo/client/react/ssr";
import App from "../imports/both/App";
import apolloClient from "../imports/both/apolloClient";
import { ServerStyleSheet } from "styled-components";
import {
  LoadableCaptureProvider,
  preloadAllLoadables,
} from "meteor/npdev:react-loadable";

function getClientData(client) {
  const cache = JSON.stringify(client.cache.extract());
  return `<script>window.__APOLLO_STATE__=${cache.replace(
    /</g,
    "\\u003c"
  )}</script>`;
}

preloadAllLoadables().then(() => {
  onPageLoad(async (sink) => {
    const sheet = new ServerStyleSheet();
    const client = apolloClient;
    const helmetContext = {};
    const loadableHandle = {};

    const tree = sheet.collectStyles(
      <LoadableCaptureProvider handle={loadableHandle}>
        <App
          client={client}
          location={sink.request.url}
          context={helmetContext}
        />
      </LoadableCaptureProvider>
    );

    const render = await getMarkupFromTree({
      tree,
      context: {},
      renderFunction: renderToString,
    });

    const style = sheet.getStyleTags();
    const { helmet } = helmetContext;
    const clientData = getClientData(client);
    sink.appendToHead(style);
    sheet.seal();
    sink.appendToHead(helmet.meta.toString());
    sink.appendToHead(helmet.title.toString());
    sink.appendToHead(clientData);
    sink.appendToBody(loadableHandle.toScriptTag());
    sink.renderIntoElementById("app", render);
  });
});

and here is the hydratation:

import React from "react";
import App from "../imports/both/App";
import apolloClient from "../imports/both/apolloClient";
import { hydrate } from "react-dom";
import { preloadLoadables } from "meteor/npdev:react-loadable";
import { onPageLoad } from "meteor/server-render";

// Hereunder the render without SSR
//  Meteor.startup(() => {
//   render(<App client={apolloClient} />, document.getElementById("app"));
// });

preloadLoadables().then(() => {
  onPageLoad((sink) =>
    hydrate(<App client={apolloClient} />, document.getElementById("app"))
  );
});

Versions

  "dependencies": {
    "@apollo/client": "^3.3.16",
    "@babel/runtime": "^7.14.0",
    "apollo-server-express": "^2.24.0",
    "bcrypt": "^5.0.1",
    "cross-fetch": "^3.1.4",
    "graphql": "^15.5.0",
    "lodash.debounce": "^4.0.8",
    "lodash.merge": "^4.6.2",
    "query-string": "^6.14.1",
    "r": "0.0.5",
    "react": "^17.0.2",
    "react-dom": "^17.0.2",
    "react-helmet-async": "^1.0.9",
    "react-lazy-load-image-component": "^1.5.1",
    "react-magnifier": "^3.0.4",
    "react-router-dom": "^5.2.0",
    "redis": "^3.1.2",
    "sitemap": "^6.4.0",
    "styled-components": "^5.2.3"
  },

Some notes:

App works perfectly, fast, everything works... until the server starts to reach it's memory limit and then crashes...

benjamn commented 3 years ago

@Gorbus Unless I'm misunderstanding your setup, it looks like you're not creating a fresh ApolloClient instance for each SSR request, but reusing the same client for all requests. Creating a fresh client per request should not only solve the memory leaks (in the sense that any remaining leaks definitely qualify as bugs), but will prevent risky situations where user-specific cached data could leak from one request to another.

There are other ways to make sure the client gets appropriately cleared out or reset between requests, but these approaches tend to assume you don't have any overlapping asynchronous requests, which is (I hope) clearly unrealistic in an async execution environment like Node.js. To get that kind of system to work, you'd still need to maintain a pool of clients, creating new clients only when all existing clients are already in use, and resetting each client before returning it to the shared pool.

By contrast, simply creating a fresh client for each request will save you some unnecessary headaches.

Gorbus commented 3 years ago

Thanks for your answer @benjamn. I got to admit you got me confused with your reply, I guess I don't have a full understanding of how to handle the apolloClient so I am not sure what to reply to your post except showing you the code from where the client is created. Could you tell me if that's good or it should be handled differently ?

"../imports/both/apolloClient";

import { Meteor } from "meteor/meteor";
import { ApolloClient, InMemoryCache, createHttpLink } from "@apollo/client";
import fetch from "cross-fetch";

const fieldPolicy = {
  typePolicies: {
    Query: {
      fields: {
        searchProducts: {
          keyArgs: false,
          merge(existing = [], incoming) {
            return incoming;
          },
        },
      },
    },
  },
};

const cache = Meteor.isClient
  ? new InMemoryCache(fieldPolicy).restore(window.__APOLLO_STATE__)
  : new InMemoryCache();

export default apolloClient = new ApolloClient({
  ssrMode: Meteor.isServer,
  link: createHttpLink({ uri: Meteor.absoluteUrl("/graphql"), fetch }),
  cache,
  ssrForceFetchDelay: 100,
});

Also I thought the client was created in this part of the code, so on each PageLoad:

preloadAllLoadables().then(() => {
  onPageLoad(async (sink) => {
    const sheet = new ServerStyleSheet();
    const client = apolloClient;