[fei5402.2.removeheapdump] Remove heapdump support (#1013)

## Summary: This removes the `heapdump` peer dependency and support for heap dumps. We haven't used this in quite a while and it tends to cause Node upgrade issues. If we should need it to debug something in the future, we can always add it back. Issue: FEI-5402 ## Test plan: `yarn install` `yarn test` `yarn typecheck` I did these steps under Node 20 and Node 16. Author: somewhatabstract Reviewers: jeresig Required Reviewers: Approved By: jeresig Checks: ✅ Test (macos-latest, 20.x), ✅ CodeQL, ❌ codecov/project, ✅ Lint, typecheck, and coverage check (ubuntu-latest, 20.x), ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 20.x), ✅ gerald, ✅ Analyze (javascript), ⏭️ dependabot Pull Request URL: #1013
Khan · Feb 12, 2024 · 4fc7dbb · 4fc7dbb
1 parent b3c527b
commit 4fc7dbb
Show file tree

Hide file tree

Showing 8 changed files with 9 additions and 235 deletions.
diff --git a/.changeset/rare-zoos-move.md b/.changeset/rare-zoos-move.md
@@ -0,0 +1,6 @@
+---
+"@khanacademy/wonder-stuff-render-server": major
+"@khanacademy/wonder-stuff-server": major
+---
+
+Removed support for the heapdump peer dependency and heap dump generation.
diff --git a/package.json b/package.json
@@ -58,7 +58,6 @@
     "express-async-handler": "^1.2.0",
     "express-winston": "^4.2.0",
     "fast-glob": "^3.3.0",
-    "heapdump": "^0.3.15",
     "jest": "^29.7.0",
     "jest-extended": "^4.0.2",
     "jest-when": "^3.6.0",
@@ -76,7 +75,6 @@
     "winston": "^3.10.0"
   },
   "resolutions": {
-    "//": "We need to use node-gyp 10 to support systems that use Python 3.12",
     "//": "we need to pin wide-align/string-width because v5 & up are ESM only",
     "wide-align/string-width": "^4.2.0",
     "node-gyp": "^10.0.0"
@@ -100,4 +98,4 @@
     "nochangeset": "yarn changeset add --empty",
     "add:devdepbysha": "bash -c 'yarn add -W --dev \"git+https://[email protected]/Khan/$0.git#$1\"'"
   }
-}
+}
diff --git a/packages/wonder-stuff-render-server/src/__tests__/run-server.test.ts b/packages/wonder-stuff-render-server/src/__tests__/run-server.test.ts
@@ -15,20 +15,6 @@ jest.mock("../handlers/make-render-handler");
 jest.mock("../get-request-authentication");
 
 describe("#runServer", () => {
-    const OLD_KA_ALLOW_HEAPDUMPS = process.env.KA_ALLOW_HEAPDUMPS;
-
-    beforeEach(() => {
-        delete process.env.KA_ALLOW_HEAPDUMPS;
-    });
-
-    afterEach(() => {
-        if (OLD_KA_ALLOW_HEAPDUMPS == null) {
-            delete process.env.KA_ALLOW_HEAPDUMPS;
-        } else {
-            process.env.KA_ALLOW_HEAPDUMPS = OLD_KA_ALLOW_HEAPDUMPS;
-        }
-    });
-
     describe("when NODE_ENV does not match the runtime mode", () => {
         const OLD_NODE_ENV = process.env.NODE_ENV;
         const OLD_KA_IS_DEV_SERVER = process.env.KA_IS_DEV_SERVER;
@@ -273,7 +259,6 @@ describe("#runServer", () => {
                 port: 42,
                 host: "127.0.0.1",
                 mode: "test",
-                allowHeapDumps: false,
                 integrations: {
                     profiler: true,
                 },
@@ -286,43 +271,4 @@ describe("#runServer", () => {
             pretendApp,
         );
     });
-
-    it("should start the server with allowHeapDumps as true when KA_ALLOW_HEAPDUMP is 1", async () => {
-        // Arrange
-        process.env.KA_ALLOW_HEAPDUMP = "1";
-        const fakeRenderEnvironment: any = {render: jest.fn()};
-        jest.spyOn(WSServer, "getRuntimeMode").mockReturnValue(
-            WSServer.Runtime.Test,
-        );
-        jest.spyOn(WSServer, "getAppEngineInfo").mockReturnValue({} as any);
-        const pretendApp = {
-            use: jest.fn().mockReturnThis(),
-            get: jest.fn().mockReturnThis(),
-            set: jest.fn(),
-        } as any;
-        jest.spyOn(Express, "default").mockReturnValue(pretendApp);
-        const startGatewaySpy = jest.spyOn(WSServer, "startServer");
-
-        // Act
-        await runServer({
-            name: "MY_TEST",
-            port: 42,
-            host: "127.0.0.1",
-            renderEnvironment: fakeRenderEnvironment,
-        });
-
-        // Assert
-        expect(startGatewaySpy).toHaveBeenCalledWith(
-            {
-                name: "MY_TEST",
-                port: 42,
-                host: "127.0.0.1",
-                mode: "test",
-                allowHeapDumps: true,
-                integrations: {},
-                logLevel: "debug",
-            },
-            pretendApp,
-        );
-    });
 });
diff --git a/packages/wonder-stuff-render-server/src/run-server.ts b/packages/wonder-stuff-render-server/src/run-server.ts
@@ -66,7 +66,6 @@ export const runServer = async (
             },
             requestAuthentication:
                 await getRequestAuthentication(authentication),
-            allowHeapDumps: process.env.KA_ALLOW_HEAPDUMP === "1",
         },
         app,
     );

diff --git a/packages/wonder-stuff-server/src/__tests__/start-server.test.ts b/packages/wonder-stuff-server/src/__tests__/start-server.test.ts
@@ -6,7 +6,6 @@ import {Runtime} from "../types";
 import {startServer} from "../start-server";
 import type {ServerOptions} from "../types";
 
-jest.mock("heapdump");
 jest.mock("../create-logger");
 jest.mock("../root-logger");
 jest.mock("../middleware/wrap-with-middleware");
@@ -50,139 +49,6 @@ describe("#start-server", () => {
         expect(setRootLoggerSpy).toHaveBeenCalledWith(logger);
     });
 
-    it.each(Array.from(Object.values(Runtime)))(
-        "should import heapdumps if allowHeapDumps is true",
-        async (mode: any) => {
-            // Arrange
-            const logger: any = {
-                debug: jest.fn(),
-            };
-            const options: ServerOptions = {
-                ...DefaultOptions,
-                allowHeapDumps: true,
-            };
-            const fakeServer = {
-                address: jest.fn(),
-                on: jest.fn(),
-                keepAliveTimeout: 0,
-            } as const;
-            const pretendApp: any = {
-                listen: jest.fn().mockReturnValue(fakeServer),
-                use: jest.fn().mockReturnThis(),
-            };
-            jest.spyOn(CreateLogger, "createLogger").mockReturnValue(logger);
-            jest.spyOn(
-                WrapWithMiddleware,
-                "wrapWithMiddleware",
-            ).mockResolvedValue(pretendApp);
-
-            // Act
-            await startServer(options, pretendApp);
-
-            // Assert
-            expect(logger.debug).toHaveBeenCalledWith(
-                expect.stringContaining(
-                    'Heapdumps enabled. To create a heap snapshot at any time, run "kill -USR2 ',
-                ),
-            );
-        },
-    );
-
-    it("should import heapdumps if not production", async () => {
-        // Arrange
-        const logger: any = {
-            debug: jest.fn(),
-        };
-        const options: ServerOptions = {
-            ...DefaultOptions,
-            mode: Runtime.Development,
-        } as const;
-        const fakeServer = {
-            address: jest.fn(),
-            on: jest.fn(),
-            keepAliveTimeout: 0,
-        } as const;
-        const pretendApp: any = {
-            listen: jest.fn().mockReturnValue(fakeServer),
-            use: jest.fn().mockReturnThis(),
-        };
-        jest.spyOn(CreateLogger, "createLogger").mockReturnValue(logger);
-        jest.spyOn(WrapWithMiddleware, "wrapWithMiddleware").mockResolvedValue(
-            pretendApp,
-        );
-
-        // Act
-        await startServer(options, pretendApp);
-
-        // Assert
-        expect(logger.debug).toHaveBeenCalledWith(
-            expect.stringContaining(
-                'Heapdumps enabled. To create a heap snapshot at any time, run "kill -USR2 ',
-            ),
-        );
-    });
-
-    it("should not import heapdumps if in production", async () => {
-        // Arrange
-        const logger: any = {
-            debug: jest.fn(),
-        };
-        const options: ServerOptions = {
-            ...DefaultOptions,
-            mode: Runtime.Production,
-        } as const;
-        const fakeServer = {
-            address: jest.fn(),
-            on: jest.fn(),
-            keepAliveTimeout: 0,
-        } as const;
-        const pretendApp: any = {
-            listen: jest.fn().mockReturnValue(fakeServer),
-            use: jest.fn().mockReturnThis(),
-        };
-        jest.spyOn(CreateLogger, "createLogger").mockReturnValue(logger);
-        jest.spyOn(WrapWithMiddleware, "wrapWithMiddleware").mockResolvedValue(
-            pretendApp,
-        );
-
-        // Act
-        await startServer(options, pretendApp);
-
-        // Assert
-        expect(logger.debug).not.toHaveBeenCalled();
-    });
-
-    it("should not import heapdumps if explicitly told not to", async () => {
-        // Arrange
-        const logger: any = {
-            debug: jest.fn(),
-        };
-        const options: ServerOptions = {
-            ...DefaultOptions,
-            mode: Runtime.Development,
-            allowHeapDumps: false,
-        } as const;
-        const fakeServer = {
-            address: jest.fn(),
-            on: jest.fn(),
-            keepAliveTimeout: 0,
-        } as const;
-        const pretendApp: any = {
-            listen: jest.fn().mockReturnValue(fakeServer),
-            use: jest.fn().mockReturnThis(),
-        };
-        jest.spyOn(CreateLogger, "createLogger").mockReturnValue(logger);
-        jest.spyOn(WrapWithMiddleware, "wrapWithMiddleware").mockResolvedValue(
-            pretendApp,
-        );
-
-        // Act
-        await startServer(options, pretendApp);
-
-        // Assert
-        expect(logger.debug).not.toHaveBeenCalled();
-    });
-
     it("should wrap the app with middleware", async () => {
         // Arrange
         const logger: any = {

diff --git a/packages/wonder-stuff-server/src/start-server.ts b/packages/wonder-stuff-server/src/start-server.ts
@@ -25,7 +25,6 @@ export async function startServer(
         name,
         mode,
         keepAliveTimeout,
-        allowHeapDumps,
         requestAuthentication,
         integrations,
         logLevel,
@@ -60,31 +59,6 @@ export async function startServer(
     });
     setRootLogger(logger);
 
-    /**
-     * In development mode, we include the heapdump module if it exists.
-     * With this installed, `kill -USR2 <pid>` can be used to create a
-     * heapsnapshot file of the gateway's memory.
-     *
-     * We ignore this from coverage because we don't care enough to test it.
-     */
-    /* istanbul ignore next */
-    if (
-        allowHeapDumps === true ||
-        (mode !== Runtime.Production && allowHeapDumps !== false)
-    ) {
-        try {
-            /* eslint-disable import/no-unassigned-import */
-            require("heapdump");
-            /* eslint-enable import/no-unassigned-import */
-            logger.debug(
-                `Heapdumps enabled. To create a heap snapshot at any time, run "kill -USR2 ${process.pid}".`,
-            );
-        } catch (e: any) {
-            // heapdump is an optional peer dependency, so if it is absent,
-            // that is perfectly fine.
-        }
-    }
-
     // Set up Google Cloud debugging integrations.
     await setupGoogleCloudIntegrations(mode, integrations);
 

diff --git a/packages/wonder-stuff-server/src/types.ts b/packages/wonder-stuff-server/src/types.ts
@@ -109,15 +109,6 @@ export type ServerOptions = {
      * Defaults to 90000.
      */
     readonly keepAliveTimeout?: number;
-    /**
-     * When `true`, the "heapdump" package will be loaded, allowing for
-     * heap dumps to be generated on demand using `kill -USR2 <pid>` where
-     * `<pid>` is the process ID of the server.
-     *
-     * Defaults to `false` in production mode, and `true` in all other modes.
-     * Set explicitly to `false` to disable heap dumps in all modes.
-     */
-    readonly allowHeapDumps?: boolean;
     /**
      * Configuration information for authenticating requests.
      */

diff --git a/yarn.lock b/yarn.lock
@@ -5394,13 +5394,6 @@ [email protected]:
   resolved "https://registry.yarnpkg.com/he/-/he-1.2.0.tgz#84ae65fa7eafb165fddb61566ae14baf05664f0f"
   integrity sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw==
 
-heapdump@^0.3.15:
-  version "0.3.15"
-  resolved "https://registry.yarnpkg.com/heapdump/-/heapdump-0.3.15.tgz#631a8a2585588ea64778d8ec80a64c6c025f6a08"
-  integrity sha512-n8aSFscI9r3gfhOcAECAtXFaQ1uy4QSke6bnaL+iymYZ/dWs9cqDqHM+rALfsHUwukUbxsdlECZ0pKmJdQ/4OA==
-  dependencies:
-    nan "^2.13.2"
-
 hex2dec@^1.0.1:
   version "1.1.2"
   resolved "https://registry.yarnpkg.com/hex2dec/-/hex2dec-1.1.2.tgz#8e1ce4bef36a74f7d5723c3fb3090c2860077338"
@@ -7170,7 +7163,7 @@ multimatch@^4.0.0:
     arrify "^2.0.1"
     minimatch "^3.0.4"
 
-nan@^2.13.2, nan@^2.14.0:
+nan@^2.14.0:
   version "2.17.0"
   resolved "https://registry.yarnpkg.com/nan/-/nan-2.17.0.tgz#c0150a2368a182f033e9aa5195ec76ea41a199cb"
   integrity sha512-2ZTgtl0nJsO0KQCjEpxcIr5D+Yv90plTitZt9JBfQvVJDS5seMl3FOvsh3+9CoYWXf/1l5OaZzzF6nDm4cagaQ==
@@ -9605,6 +9598,7 @@ [email protected]:
   integrity sha512-ILEIE97kDZvF9Wb9f6h5aXK4swSlKGUcOEGiIYb2OOu/IrDU9iwj0fD//SsA6E5ibwJxpEvhullJY4Sl4GcpAw==
 
 "wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0:
+  name wrap-ansi-cjs
   version "7.0.0"
   resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
   integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
-Original file line number
+Diff line change
@@ Expand Up / @@ -66,7 +66,6 @@ export const runServer = async ( @@
                 },
                 requestAuthentication:
                     await getRequestAuthentication(authentication),
-                allowHeapDumps: process.env.KA_ALLOW_HEAPDUMP === "1",
             },
             app,
         );
@@ Expand Down @@