Skip to content

Commit a5dd638

Browse files
authored
fix: sentry memory leak by patching @sentry/remix to stop cloning request (#2389)
* fix: sentry memory leak by disabling includeLocalVariables * Enhance heap snapshot consistency and labeling To facilitate more accurate and consistent heap memory snapshots, a new function forceConsistentGC was added before taking a snapshot. This ensures the garbage collector (GC) runs multiple times, stabilizing the heap state for more reliable analysis. This is particularly helpful when debugging memory-related issues. Updates to the memory-leak-detector script now allow labeling of snapshot runs using the --label flag. This helps in distinguishing different runs for easier tracking and comparison of memory usage across test sessions. Additionally, the --expose-gc flag ensures that the GC can be manually triggered during test runs, leading to more consistent memory states and potentially uncovering hidden memory leaks. * Refactor forceConsistentGC for improved readability The function forceConsistentGC was refactored to enhance code readability and consistency. The main improvements include: - Updated syntax for consistent string quotation and spacing. - Simplified garbage collection by removing specific major/minor GC calls, as the distinction isn't necessary. - Implemented minor changes to arrow function formatting for consistency. These changes neither impact the program logic nor the function behavior but help maintain code quality standards and readability. * Fix memory leak by removing request.clone() usage Identified that the memory leak in the project was linked to the usage of request.clone() within the `@sentry/remix` package's callRouteAction handler. Although initially suspected as a Sentry issue, the problem appears to arise from the handling of request.clone() in Remix version 2.1.0. By removing the call to request.clone(), the memory leak has been resolved. - Introduced garbage collection execution before snapshot to manage memory allocation effectively. - Improved error handling and timeout mechanisms in the memory leak detector to enhance its resilience during runtime. - Expanded testing for both GET and POST requests to monitor and validate potential memory leaks better. The POST requests involve sending large payloads to stress-test the system. - The modification particularly focuses on enhancing robust memory tracking and providing detailed progress reporting during request phases. * patch @sentry/remix to prevent memory leaks * Fix pnpm lock * undo some unrelated changes
1 parent 3ec95cd commit a5dd638

File tree

10 files changed

+1148
-75
lines changed

10 files changed

+1148
-75
lines changed

apps/webapp/.gitignore

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,5 @@ build-storybook.log
1818
storybook-static
1919

2020
/prisma/seed.js
21-
/prisma/populate.js
21+
/prisma/populate.js
22+
.memory-snapshots

apps/webapp/app/routes/admin.api.v1.gc.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@ import { type DataFunctionArgs } from "@remix-run/node";
22
import { PerformanceObserver } from "node:perf_hooks";
33
import { runInNewContext } from "node:vm";
44
import v8 from "v8";
5-
import { requireUser } from "~/services/session.server";
5+
import { prisma } from "~/db.server";
6+
import { authenticateApiRequestWithPersonalAccessToken } from "~/services/personalAccessToken.server";
67

78
async function waitTillGcFinishes() {
89
let resolver: (value: PerformanceEntry) => void;
@@ -35,9 +36,19 @@ async function waitTillGcFinishes() {
3536
}
3637

3738
export async function loader({ request }: DataFunctionArgs) {
38-
const user = await requireUser(request);
39+
const authenticationResult = await authenticateApiRequestWithPersonalAccessToken(request);
3940

40-
if (!user.admin) {
41+
if (!authenticationResult) {
42+
throw new Response("You must be an admin to perform this action", { status: 403 });
43+
}
44+
45+
const user = await prisma.user.findFirst({
46+
where: {
47+
id: authenticationResult.userId,
48+
},
49+
});
50+
51+
if (!user?.admin) {
4152
throw new Response("You must be an admin to perform this action", { status: 403 });
4253
}
4354

apps/webapp/app/routes/admin.api.v1.snapshot.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ import os from "os";
44
import path from "path";
55
import { PassThrough } from "stream";
66
import v8 from "v8";
7-
import { requireUser } from "~/services/session.server";
7+
import { prisma } from "~/db.server";
8+
import { authenticateApiRequestWithPersonalAccessToken } from "~/services/personalAccessToken.server";
89

910
// Format date as yyyy-MM-dd HH_mm_ss_SSS
1011
function formatDate(date: Date) {
@@ -24,9 +25,19 @@ function formatDate(date: Date) {
2425
}
2526

2627
export async function loader({ request }: DataFunctionArgs) {
27-
const user = await requireUser(request);
28+
const authenticationResult = await authenticateApiRequestWithPersonalAccessToken(request);
2829

29-
if (!user.admin) {
30+
if (!authenticationResult) {
31+
throw new Response("You must be an admin to perform this action", { status: 403 });
32+
}
33+
34+
const user = await prisma.user.findFirst({
35+
where: {
36+
id: authenticationResult.userId,
37+
},
38+
});
39+
40+
if (!user?.admin) {
3041
throw new Response("You must be an admin to perform this action", { status: 403 });
3142
}
3243

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import { ActionFunctionArgs } from "@remix-run/server-runtime";
2+
3+
export async function action({ request }: ActionFunctionArgs) {
4+
if (process.env.NODE_ENV === "production") {
5+
return new Response("Not found", { status: 404 });
6+
}
7+
8+
return new Response(
9+
JSON.stringify({
10+
data: {
11+
id: "123",
12+
type: "mock",
13+
attributes: {
14+
name: "Mock",
15+
},
16+
},
17+
}),
18+
{ status: 200 }
19+
);
20+
}

0 commit comments

Comments
 (0)