From 2d042a274467f8cc99bbb068b0008d7138555268 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Sat, 25 May 2024 17:55:20 -0400
Subject: [PATCH 01/23] ref(node): use readline for context lines

---
 .../node/src/integrations/contextlines.ts     | 218 +++++++++++++-----
 .../test/integrations/contextlines.test.ts    |   2 +-
 2 files changed, 157 insertions(+), 63 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index 7f4b78653b6a..caafbb29b9a2 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -1,20 +1,91 @@
-import { promises } from 'node:fs';
+import { createReadStream } from 'node:fs';
+import * as readline from 'node:readline';
 import { defineIntegration } from '@sentry/core';
 import type { Event, IntegrationFn, StackFrame } from '@sentry/types';
-import { LRUMap, addContextToFrame } from '@sentry/utils';
+import { logger, LRUMap, snipLine } from '@sentry/utils';
+
+import { DEBUG_BUILD } from '../debug-build';
 
-const FILE_CONTENT_CACHE = new LRUMap<string, string[] | null>(100);
 const DEFAULT_LINES_OF_CONTEXT = 7;
 const INTEGRATION_NAME = 'ContextLines';
-
-const readFileAsync = promises.readFile;
+const LRUFileContentCache = new LRUMap<string, Record<number, string>>(10);
 
 /**
- * Resets the file cache. Exists for testing purposes.
- * @hidden
+ * Exists for testing purposes.
  */
 export function resetFileContentCache(): void {
-  FILE_CONTENT_CACHE.clear();
+  LRUFileContentCache.clear();
+}
+
+function makeLineReaderRanges(lines: number[], linecontext: number): ReadlineRange[] {
+  if (!lines.length) {
+    return [];
+  }
+
+  const out: ReadlineRange[] = [];
+  let i = 0;
+  let current = makeContextRange(lines[i], linecontext);
+
+  // eslint-disable-next-line no-constant-condition
+  while (true) {
+    if (i === lines.length - 1) {
+      out.push(current);
+      break;
+    }
+
+    // We need to create contiguous ranges in cases where context lines overlap so that
+    // the final set of ranges is an increasing sequence of lines without overlaps.
+    const next = lines[i + 1];
+    if (next <= current[1]) {
+      current[1] = next + linecontext + 1;
+    } else {
+      out.push(current);
+      current = makeContextRange(next, linecontext);
+    }
+
+    i++;
+  }
+
+  return out;
+}
+
+// Stack trace comes in, parse it and extract stack filename + lines
+// in case we receive lines from multiple files, the final output
+// should contain files sorted by stack order importance - top first
+// and should contain.
+async function getContextLinesFromFile(
+  path: string,
+  ranges: ReadlineRange[],
+  output: Record<number, string>,
+): Promise<void> {
+  const fileStream = readline.createInterface({
+    input: createReadStream(path),
+  });
+
+  // Line numbers are 1 indexed
+  let lineNumber = 1;
+  let currentRangeIndex = 0;
+  let rangeStart = ranges[currentRangeIndex][0];
+  let rangeEnd = ranges[currentRangeIndex][1];
+
+  for await (const line of fileStream) {
+    lineNumber++;
+    if (lineNumber < rangeStart) {
+      continue;
+    }
+
+    output[lineNumber] = line;
+    // or if there are other ranges to process. If so, update the range
+    // and continue processing the file, else break from the loop.
+    if (lineNumber >= rangeEnd) {
+      if (currentRangeIndex === ranges.length - 1) {
+        break;
+      }
+      currentRangeIndex++;
+      rangeStart = ranges[currentRangeIndex][0];
+      rangeEnd = ranges[currentRangeIndex][1];
+    }
+  }
 }
 
 interface ContextLinesOptions {
@@ -27,7 +98,8 @@ interface ContextLinesOptions {
   frameContextLines?: number;
 }
 
-/** Exported only for tests, as a type-safe variant. */
+/** Exported on
+ * ly for tests, as a type-safe variant. */
 export const _contextLinesIntegration = ((options: ContextLinesOptions = {}) => {
   const contextLines = options.frameContextLines !== undefined ? options.frameContextLines : DEFAULT_LINES_OF_CONTEXT;
 
@@ -47,8 +119,7 @@ export const contextLinesIntegration = defineIntegration(_contextLinesIntegratio
 async function addSourceContext(event: Event, contextLines: number): Promise<Event> {
   // keep a lookup map of which files we've already enqueued to read,
   // so we don't enqueue the same file multiple times which would cause multiple i/o reads
-  const enqueuedReadSourceFileTasks: Record<string, number> = {};
-  const readSourceFileTasks: Promise<string[] | null>[] = [];
+  const filesToLines: Record<string, number[]> = {};
 
   if (contextLines > 0 && event.exception?.values) {
     for (const exception of event.exception.values) {
@@ -56,33 +127,54 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
         continue;
       }
 
-      // We want to iterate in reverse order as calling cache.get will bump the file in our LRU cache.
-      // This ends up prioritizes source context for frames at the top of the stack instead of the bottom.
+      // Maps preserve insertion order, so we iterate in reverse, starting at the
+      // outermost frame and closer to where the exception has occurred (poor mans priority)
       for (let i = exception.stacktrace.frames.length - 1; i >= 0; i--) {
         const frame = exception.stacktrace.frames[i];
-        // Call cache.get to bump the file to the top of the cache and ensure we have not already
-        // enqueued a read operation for this filename
-        if (frame.filename && !enqueuedReadSourceFileTasks[frame.filename] && !FILE_CONTENT_CACHE.get(frame.filename)) {
-          readSourceFileTasks.push(_readSourceFile(frame.filename));
-          enqueuedReadSourceFileTasks[frame.filename] = 1;
+
+        // Collecting context lines for minified code is useless.
+        if (frame.filename?.endsWith('.min.js')) {
+          continue;
+        }
+
+        if (frame.filename && typeof frame.lineno === 'number') {
+          if (!filesToLines[frame.filename]) filesToLines[frame.filename] = [];
+          filesToLines[frame.filename].push(frame.lineno);
         }
       }
     }
   }
 
-  // check if files to read > 0, if so, await all of them to be read before adding source contexts.
-  // Normally, Promise.all here could be short circuited if one of the promises rejects, but we
-  // are guarding from that by wrapping the i/o read operation in a try/catch.
-  if (readSourceFileTasks.length > 0) {
-    await Promise.all(readSourceFileTasks);
+  const files = Object.keys(filesToLines);
+  if (files.length == 0) {
+    return event;
+  }
+
+  const readlinePromises: Promise<void>[] = [];
+  for (const file of files) {
+    // Sort ranges so that they are sorted by line increasing order and match how the file is read.
+    filesToLines[file].sort((a, b) => a - b);
+    const ranges = makeLineReaderRanges(filesToLines[file], contextLines);
+
+    let cache = LRUFileContentCache.get(file);
+    if (!cache) {
+      cache = {};
+      LRUFileContentCache.set(file, cache);
+    }
+    readlinePromises.push(getContextLinesFromFile(file, ranges, cache));
   }
 
+  await Promise.all(readlinePromises).catch(() => {
+    // We don't want to error if we can't read the file.
+    DEBUG_BUILD && logger.log('Failed to read one or more source files and resolve context lines');
+  });
+
   // Perform the same loop as above, but this time we can assume all files are in the cache
   // and attempt to add source context to frames.
   if (contextLines > 0 && event.exception?.values) {
     for (const exception of event.exception.values) {
-      if (exception.stacktrace && exception.stacktrace.frames) {
-        await addSourceContextToFrames(exception.stacktrace.frames, contextLines);
+      if (exception.stacktrace && exception.stacktrace.frames && exception.stacktrace.frames.length > 0) {
+        addSourceContextToFrames(exception.stacktrace.frames, LRUFileContentCache);
       }
     }
   }
@@ -91,56 +183,58 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
 }
 
 /** Adds context lines to frames */
-function addSourceContextToFrames(frames: StackFrame[], contextLines: number): void {
+function addSourceContextToFrames(frames: StackFrame[], cache: LRUMap<string, Record<number, string>>): void {
   for (const frame of frames) {
     // Only add context if we have a filename and it hasn't already been added
-    if (frame.filename && frame.context_line === undefined) {
-      const sourceFileLines = FILE_CONTENT_CACHE.get(frame.filename);
-
-      if (sourceFileLines) {
-        try {
-          addContextToFrame(sourceFileLines, frame, contextLines);
-        } catch (e) {
-          // anomaly, being defensive in case
-          // unlikely to ever happen in practice but can definitely happen in theory
-        }
+    if (frame.filename && frame.context_line === undefined && typeof frame.lineno === 'number') {
+      const contents = cache.get(frame.filename);
+      if (contents) {
+        addContextToFrame(frame.lineno, frame, contents);
       }
     }
   }
 }
 
 /**
- * Reads file contents and caches them in a global LRU cache.
- * If reading fails, mark the file as null in the cache so we don't try again.
- *
- * @param filename filepath to read content from.
+ * Resolves context lines before and after the given line number and appends them to the frame;
  */
-async function _readSourceFile(filename: string): Promise<string[] | null> {
-  const cachedFile = FILE_CONTENT_CACHE.get(filename);
-
-  // We have already attempted to read this file and failed, do not try again
-  if (cachedFile === null) {
-    return null;
+export function addContextToFrame(
+  lineno: number,
+  frame: StackFrame,
+  contents: Record<number, string> | undefined,
+): void {
+  // When there is no line number in the frame, attaching context is nonsensical and will even break grouping.
+  // We already check for lineno before calling this, but since StackFrame lineno ism optional, we check it again.
+  if (frame.lineno === undefined || contents === undefined) {
+    return;
   }
 
-  // We have a cache hit, return it
-  if (cachedFile !== undefined) {
-    return cachedFile;
+  frame.pre_context = [];
+  for (let i = makeRangeStart(lineno, DEFAULT_LINES_OF_CONTEXT); i < lineno; i++) {
+    if (contents[i]) {
+      frame.pre_context.push(snipLine(contents[i], 0));
+    }
   }
 
-  // Guard from throwing if readFile fails, this enables us to use Promise.all and
-  // not have it short circuiting if one of the promises rejects + since context lines are added
-  // on a best effort basis, we want to throw here anyways.
-
-  // If we made it to here, it means that our file is not cache nor marked as failed, so attempt to read it
-  let content: string[] | null = null;
-  try {
-    const rawFileContents = await readFileAsync(filename, 'utf-8');
-    content = rawFileContents.split('\n');
-  } catch (_) {
-    // if we fail, we will mark the file as null in the cache and short circuit next time we try to read it
+  frame.context_line = snipLine(contents[lineno] || '', frame.colno || 0);
+
+  frame.post_context = [];
+  for (let i = lineno + 1; i < makeRangeEnd(lineno, DEFAULT_LINES_OF_CONTEXT); i++) {
+    if (contents[i]) {
+      frame.post_context.push(snipLine(contents[i], 0));
+    }
   }
+}
 
-  FILE_CONTENT_CACHE.set(filename, content);
-  return content;
+// Helper functions for generating line context ranges. They take a line number and the number of lines of context to
+// include before and after the line and generate an inclusive range of indices.
+type ReadlineRange = [start: number, end: number];
+function makeRangeStart(line: number, linecontext: number): number {
+  return Math.max(1, line - linecontext);
+}
+function makeRangeEnd(line: number, linecontext: number): number {
+  return line + linecontext + 1;
+}
+function makeContextRange(line: number, linecontext: number): [start: number, end: number] {
+  return [makeRangeStart(line, linecontext), makeRangeEnd(line, linecontext)];
 }
diff --git a/packages/node/test/integrations/contextlines.test.ts b/packages/node/test/integrations/contextlines.test.ts
index 3c6394f9ac52..8aee4aaac499 100644
--- a/packages/node/test/integrations/contextlines.test.ts
+++ b/packages/node/test/integrations/contextlines.test.ts
@@ -114,7 +114,7 @@ describe('ContextLines', () => {
       const frames = parseStackFrames(defaultStackParser, new Error('test'));
 
       await addContext(frames);
-      expect(readFileSpy).toHaveBeenCalledTimes(0);
+      expect(readFileSpy).not.toHaveBeenCalled();
     });
   });
 

From 1fc254449d7a5e98ca7e40f2aef6f23914f3108a Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Sat, 25 May 2024 20:33:33 -0400
Subject: [PATCH 02/23] test(node): contextline tests

---
 .../node/src/integrations/contextlines.ts     | 140 +++++++++++-------
 .../test/integrations/contextlines.test.ts    | 122 ++++++++-------
 2 files changed, 162 insertions(+), 100 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index caafbb29b9a2..4e19bbcc52c5 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -1,5 +1,5 @@
 import { createReadStream } from 'node:fs';
-import * as readline from 'node:readline';
+import { createInterface } from 'node:readline';
 import { defineIntegration } from '@sentry/core';
 import type { Event, IntegrationFn, StackFrame } from '@sentry/types';
 import { logger, LRUMap, snipLine } from '@sentry/utils';
@@ -8,7 +8,9 @@ import { DEBUG_BUILD } from '../debug-build';
 
 const DEFAULT_LINES_OF_CONTEXT = 7;
 const INTEGRATION_NAME = 'ContextLines';
-const LRUFileContentCache = new LRUMap<string, Record<number, string>>(10);
+
+// Exported for tests
+export const LRUFileContentCache = new LRUMap<string, Record<number, string>>(10);
 
 /**
  * Exists for testing purposes.
@@ -17,15 +19,21 @@ export function resetFileContentCache(): void {
   LRUFileContentCache.clear();
 }
 
+/**
+ * Creates contiguous ranges of lines to read from a file. In the case where context lines overlap,
+ * the ranges are merged to create a single range that includes both sets of lines.
+ * @param lines
+ * @param linecontext
+ * @returns
+ */
 function makeLineReaderRanges(lines: number[], linecontext: number): ReadlineRange[] {
   if (!lines.length) {
     return [];
   }
 
-  const out: ReadlineRange[] = [];
   let i = 0;
   let current = makeContextRange(lines[i], linecontext);
-
+  const out: ReadlineRange[] = [];
   // eslint-disable-next-line no-constant-condition
   while (true) {
     if (i === lines.length - 1) {
@@ -49,43 +57,50 @@ function makeLineReaderRanges(lines: number[], linecontext: number): ReadlineRan
   return out;
 }
 
-// Stack trace comes in, parse it and extract stack filename + lines
-// in case we receive lines from multiple files, the final output
-// should contain files sorted by stack order importance - top first
-// and should contain.
-async function getContextLinesFromFile(
-  path: string,
-  ranges: ReadlineRange[],
-  output: Record<number, string>,
-): Promise<void> {
-  const fileStream = readline.createInterface({
-    input: createReadStream(path),
-  });
-
-  // Line numbers are 1 indexed
-  let lineNumber = 1;
-  let currentRangeIndex = 0;
-  let rangeStart = ranges[currentRangeIndex][0];
-  let rangeEnd = ranges[currentRangeIndex][1];
-
-  for await (const line of fileStream) {
-    lineNumber++;
-    if (lineNumber < rangeStart) {
-      continue;
-    }
-
-    output[lineNumber] = line;
-    // or if there are other ranges to process. If so, update the range
-    // and continue processing the file, else break from the loop.
-    if (lineNumber >= rangeEnd) {
-      if (currentRangeIndex === ranges.length - 1) {
-        break;
+/**
+ * Extracts lines from a file and stores them in a cache.
+ */
+function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output: Record<number, string>): Promise<void> {
+  return new Promise((resolve, _reject) => {
+    const fileStream = createInterface({
+      input: createReadStream(path),
+    });
+
+    // Init at zero and increment at the start of the loop because lines are 1 indexed.
+    let lineNumber = 0;
+    let currentRangeIndex = 0;
+    let rangeStart = ranges[currentRangeIndex][0];
+    let rangeEnd = ranges[currentRangeIndex][1];
+
+    fileStream.on('line', line => {
+      lineNumber++;
+      if (lineNumber < rangeStart) return;
+
+      // Mutates the cache value directly
+      output[lineNumber] = line;
+      // or if there are other ranges to process. If so, update the range
+      // and continue processing the file, else break from the loop.
+      if (lineNumber >= rangeEnd) {
+        if (currentRangeIndex === ranges.length - 1) {
+          // We need to close the file stream and remove listeners, else it wont close.
+          fileStream.close();
+          fileStream.removeAllListeners();
+          return;
+        }
+        currentRangeIndex++;
+        rangeStart = ranges[currentRangeIndex][0];
+        rangeEnd = ranges[currentRangeIndex][1];
       }
-      currentRangeIndex++;
-      rangeStart = ranges[currentRangeIndex][0];
-      rangeEnd = ranges[currentRangeIndex][1];
-    }
-  }
+    });
+
+    fileStream.on('close', resolve);
+    // We use this inside Promise.all, so we need to resolve the promise even if there is an error
+    // to prevent Promise.all from short circuiting the rest.
+    fileStream.on('error', e => {
+      DEBUG_BUILD && logger.error(`Failed to read file: ${path}. Error: ${e}`);
+      resolve();
+    });
+  });
 }
 
 interface ContextLinesOptions {
@@ -133,6 +148,7 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
         const frame = exception.stacktrace.frames[i];
 
         // Collecting context lines for minified code is useless.
+        // @TODO omit builtin modules
         if (frame.filename?.endsWith('.min.js')) {
           continue;
         }
@@ -174,7 +190,7 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
   if (contextLines > 0 && event.exception?.values) {
     for (const exception of event.exception.values) {
       if (exception.stacktrace && exception.stacktrace.frames && exception.stacktrace.frames.length > 0) {
-        addSourceContextToFrames(exception.stacktrace.frames, LRUFileContentCache);
+        addSourceContextToFrames(exception.stacktrace.frames, contextLines, LRUFileContentCache);
       }
     }
   }
@@ -183,24 +199,31 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
 }
 
 /** Adds context lines to frames */
-function addSourceContextToFrames(frames: StackFrame[], cache: LRUMap<string, Record<number, string>>): void {
+function addSourceContextToFrames(frames: StackFrame[], contextLines: number, cache: LRUMap<string, Record<number, string>>): void {
   for (const frame of frames) {
     // Only add context if we have a filename and it hasn't already been added
     if (frame.filename && frame.context_line === undefined && typeof frame.lineno === 'number') {
       const contents = cache.get(frame.filename);
       if (contents) {
-        addContextToFrame(frame.lineno, frame, contents);
+        addContextToFrame(frame.lineno, frame, contextLines, contents);
       }
     }
   }
 }
 
+function clearLineContext(frame: StackFrame): void {
+  delete frame.pre_context;
+  delete frame.context_line;
+  delete frame.post_context;
+}
+
 /**
  * Resolves context lines before and after the given line number and appends them to the frame;
  */
 export function addContextToFrame(
   lineno: number,
   frame: StackFrame,
+  contextLines: number,
   contents: Record<number, string> | undefined,
 ): void {
   // When there is no line number in the frame, attaching context is nonsensical and will even break grouping.
@@ -210,19 +233,36 @@ export function addContextToFrame(
   }
 
   frame.pre_context = [];
-  for (let i = makeRangeStart(lineno, DEFAULT_LINES_OF_CONTEXT); i < lineno; i++) {
-    if (contents[i]) {
-      frame.pre_context.push(snipLine(contents[i], 0));
+  for (let i = makeRangeStart(lineno, contextLines); i < lineno; i++) {
+    // Make sure to never send partial context lines
+    if (contents[i] === undefined) {
+      clearLineContext(frame);
+      DEBUG_BUILD && logger.error(`Could not find line ${i} in file ${frame.filename}`);
+      console.log('Pre, could not find line', i, 'in file', frame.filename);
+      return;
     }
+
+    frame.pre_context.push(snipLine(contents[i], 0));
   }
 
-  frame.context_line = snipLine(contents[lineno] || '', frame.colno || 0);
+  if (contents[lineno] === undefined) {
+    clearLineContext(frame);
+    DEBUG_BUILD && logger.error(`Could not find line ${lineno} in file ${frame.filename}`);
+    console.log('Lineno, could not find line', lineno, 'in file', frame.filename);
+    return;
+  }
+  frame.context_line = snipLine(contents[lineno], frame.colno || 0);
 
   frame.post_context = [];
-  for (let i = lineno + 1; i < makeRangeEnd(lineno, DEFAULT_LINES_OF_CONTEXT); i++) {
-    if (contents[i]) {
-      frame.post_context.push(snipLine(contents[i], 0));
+  for (let i = lineno + 1; i < makeRangeEnd(lineno, contextLines); i++) {
+    if (contents[i] === undefined) {
+      clearLineContext(frame);
+      DEBUG_BUILD && logger.error(`Could not find line ${lineno} in file ${frame.filename}`);
+      console.log('Post, could not find line', i, 'in file', frame.filename)
+      return;
     }
+
+    frame.post_context.push(snipLine(contents[i], 0));
   }
 }
 
diff --git a/packages/node/test/integrations/contextlines.test.ts b/packages/node/test/integrations/contextlines.test.ts
index 8aee4aaac499..be889aea489f 100644
--- a/packages/node/test/integrations/contextlines.test.ts
+++ b/packages/node/test/integrations/contextlines.test.ts
@@ -1,24 +1,15 @@
-import { promises } from 'node:fs';
+import * as fs from 'node:fs';
 import type { StackFrame } from '@sentry/types';
 import { parseStackFrames } from '@sentry/utils';
 
-import { _contextLinesIntegration, resetFileContentCache } from '../../src/integrations/contextlines';
+import {
+  _contextLinesIntegration,
+  resetFileContentCache,
+} from '../../src/integrations/contextlines';
 import { defaultStackParser } from '../../src/sdk/api';
 import { getError } from '../helpers/error';
 
-jest.mock('node:fs', () => {
-  const actual = jest.requireActual('node:fs');
-  return {
-    ...actual,
-    promises: {
-      ...actual.promises,
-      readFile: jest.fn(actual.promises),
-    },
-  };
-});
-
 describe('ContextLines', () => {
-  const readFileSpy = promises.readFile as unknown as jest.SpyInstance;
   let contextLines: ReturnType<typeof _contextLinesIntegration>;
 
   async function addContext(frames: StackFrame[]): Promise<void> {
@@ -39,20 +30,22 @@ describe('ContextLines', () => {
       expect.assertions(1);
 
       const frames = parseStackFrames(defaultStackParser, new Error('test'));
+      const readStreamSpy = jest.spyOn(fs, 'createReadStream');
 
-      await addContext(Array.from(frames));
+      await addContext(frames);
 
-      const numCalls = readFileSpy.mock.calls.length;
+      const numCalls = readStreamSpy.mock.calls.length;
       await addContext(frames);
 
       // Calls to `readFile` shouldn't increase if there isn't a new error to
       // parse whose stacktrace contains a file we haven't yet seen
-      expect(readFileSpy).toHaveBeenCalledTimes(numCalls);
+      expect(readStreamSpy).toHaveBeenCalledTimes(numCalls * 2);
     });
 
     test('parseStack with ESM module names', async () => {
       expect.assertions(1);
 
+      const readStreamSpy = jest.spyOn(fs, 'createReadStream');
       const framesWithFilePath: StackFrame[] = [
         {
           colno: 1,
@@ -63,25 +56,64 @@ describe('ContextLines', () => {
       ];
 
       await addContext(framesWithFilePath);
-      expect(readFileSpy).toHaveBeenCalledTimes(1);
+      expect(readStreamSpy).toHaveBeenCalledTimes(1);
     });
 
     test('parseStack with adding different file', async () => {
       expect.assertions(1);
       const frames = parseStackFrames(defaultStackParser, new Error('test'));
+      const readStreamSpy = jest.spyOn(fs, 'createReadStream');
 
       await addContext(frames);
 
-      const numCalls = readFileSpy.mock.calls.length;
+      const numCalls = readStreamSpy.mock.calls.length;
       const parsedFrames = parseStackFrames(defaultStackParser, getError());
       await addContext(parsedFrames);
 
-      const newErrorCalls = readFileSpy.mock.calls.length;
+      const newErrorCalls = readStreamSpy.mock.calls.length;
       expect(newErrorCalls).toBeGreaterThan(numCalls);
     });
 
+    test('parseStack with overlapping errors', async () => {
+      function inner() {
+        return new Error('inner');
+      }
+      function outer() {
+        return inner();
+      }
+
+      const overlappingContextWithFirstError = parseStackFrames(defaultStackParser, outer());
+
+      await addContext(overlappingContextWithFirstError);
+
+      const innerFrame = overlappingContextWithFirstError[overlappingContextWithFirstError.length - 1];
+      const outerFrame = overlappingContextWithFirstError[overlappingContextWithFirstError.length - 2];
+
+      expect(innerFrame.context_line).toBe("        return new Error('inner');");
+      expect(innerFrame.pre_context).toHaveLength(7)
+      expect(innerFrame.post_context).toHaveLength(7)
+
+      expect(outerFrame.context_line).toBe('        return inner();');
+      expect(outerFrame.pre_context).toHaveLength(7)
+      expect(outerFrame.post_context).toHaveLength(7)
+    });
+
+    test('parseStack with error on first line errors', async () => {
+      const overlappingContextWithFirstError = parseStackFrames(defaultStackParser, getError());
+
+      await addContext(overlappingContextWithFirstError);
+
+      const errorFrame = overlappingContextWithFirstError[overlappingContextWithFirstError.length - 1];
+      console.log(errorFrame)
+
+      expect(errorFrame.context_line).toBe("  return new Error('mock error');");
+      expect(errorFrame.pre_context).toHaveLength(7)
+      expect(errorFrame.post_context).toHaveLength(7)
+    });
+
     test('parseStack with duplicate files', async () => {
       expect.assertions(1);
+      const readStreamSpy = jest.spyOn(fs, 'createReadStream');
       const framesWithDuplicateFiles: StackFrame[] = [
         {
           colno: 1,
@@ -104,44 +136,34 @@ describe('ContextLines', () => {
       ];
 
       await addContext(framesWithDuplicateFiles);
-      expect(readFileSpy).toHaveBeenCalledTimes(1);
+      expect(readStreamSpy).toHaveBeenCalledTimes(1);
+    });
+
+    test('stack errors without lineno', async () => {
+      expect.assertions(1);
+      const readStreamSpy = jest.spyOn(fs, 'createReadStream');
+      const framesWithDuplicateFiles: StackFrame[] = [
+        {
+          colno: 1,
+          filename: '/var/task/index.js',
+          lineno: undefined,
+          function: 'fxn1',
+        },
+      ];
+
+      await addContext(framesWithDuplicateFiles);
+      expect(readStreamSpy).not.toHaveBeenCalled();
     });
 
     test('parseStack with no context', async () => {
+      expect.assertions(1);
       contextLines = _contextLinesIntegration({ frameContextLines: 0 });
+      const readStreamSpy = jest.spyOn(fs, 'createReadStream');
 
-      expect.assertions(1);
       const frames = parseStackFrames(defaultStackParser, new Error('test'));
 
       await addContext(frames);
-      expect(readFileSpy).not.toHaveBeenCalled();
-    });
-  });
-
-  test('does not attempt to readfile multiple times if it fails', async () => {
-    expect.assertions(1);
-
-    readFileSpy.mockImplementation(() => {
-      throw new Error("ENOENT: no such file or directory, open '/does/not/exist.js'");
+      expect(readStreamSpy).not.toHaveBeenCalled();
     });
-
-    await addContext([
-      {
-        colno: 1,
-        filename: '/does/not/exist.js',
-        lineno: 1,
-        function: 'fxn1',
-      },
-    ]);
-    await addContext([
-      {
-        colno: 1,
-        filename: '/does/not/exist.js',
-        lineno: 1,
-        function: 'fxn1',
-      },
-    ]);
-
-    expect(readFileSpy).toHaveBeenCalledTimes(1);
   });
 });

From b0df967e917f845f3a12a1b49b76395e811ca1d7 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Sat, 25 May 2024 22:05:34 -0400
Subject: [PATCH 03/23] test(node): contextline tests

---
 .../node/src/integrations/contextlines.ts     | 66 ++++++++++---------
 .../test/integrations/contextlines.test.ts    | 13 ++--
 2 files changed, 43 insertions(+), 36 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index 4e19bbcc52c5..90ead43eef50 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -6,22 +6,20 @@ import { logger, LRUMap, snipLine } from '@sentry/utils';
 
 import { DEBUG_BUILD } from '../debug-build';
 
+const LRU_FILE_CONTENTS_CACHE = new LRUMap<string, Record<number, string>>(10);
 const DEFAULT_LINES_OF_CONTEXT = 7;
 const INTEGRATION_NAME = 'ContextLines';
 
-// Exported for tests
-export const LRUFileContentCache = new LRUMap<string, Record<number, string>>(10);
-
 /**
- * Exists for testing purposes.
+ * Exported for testing purposes.
  */
 export function resetFileContentCache(): void {
-  LRUFileContentCache.clear();
+  LRU_FILE_CONTENTS_CACHE.clear();
 }
 
 /**
  * Creates contiguous ranges of lines to read from a file. In the case where context lines overlap,
- * the ranges are merged to create a single range that includes both sets of lines.
+ * the ranges are merged to create a single range.
  * @param lines
  * @param linecontext
  * @returns
@@ -41,11 +39,10 @@ function makeLineReaderRanges(lines: number[], linecontext: number): ReadlineRan
       break;
     }
 
-    // We need to create contiguous ranges in cases where context lines overlap so that
-    // the final set of ranges is an increasing sequence of lines without overlaps.
+    // If the next line falls into the current range, extend the current range to lineno + linecontext.
     const next = lines[i + 1];
     if (next <= current[1]) {
-      current[1] = next + linecontext + 1;
+      current[1] = next + linecontext;
     } else {
       out.push(current);
       current = makeContextRange(next, linecontext);
@@ -62,6 +59,8 @@ function makeLineReaderRanges(lines: number[], linecontext: number): ReadlineRan
  */
 function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output: Record<number, string>): Promise<void> {
   return new Promise((resolve, _reject) => {
+    // It is important *not* to have any async code between createInterface and the 'line' event listener
+    // as it will cause the 'line' event to be emitted before the listener is attached.
     const fileStream = createInterface({
       input: createReadStream(path),
     });
@@ -76,13 +75,12 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
       lineNumber++;
       if (lineNumber < rangeStart) return;
 
-      // Mutates the cache value directly
+      // !Warning: Mutates the cache value
       output[lineNumber] = line;
-      // or if there are other ranges to process. If so, update the range
-      // and continue processing the file, else break from the loop.
+
       if (lineNumber >= rangeEnd) {
         if (currentRangeIndex === ranges.length - 1) {
-          // We need to close the file stream and remove listeners, else it wont close.
+          // We need to close the file stream and remove listeners, else the reader will continue to run our listener;
           fileStream.close();
           fileStream.removeAllListeners();
           return;
@@ -138,7 +136,7 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
 
   if (contextLines > 0 && event.exception?.values) {
     for (const exception of event.exception.values) {
-      if (!exception.stacktrace?.frames) {
+      if (!exception.stacktrace?.frames?.length) {
         continue;
       }
 
@@ -147,9 +145,10 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
       for (let i = exception.stacktrace.frames.length - 1; i >= 0; i--) {
         const frame = exception.stacktrace.frames[i];
 
-        // Collecting context lines for minified code is useless.
-        // @TODO omit builtin modules
-        if (frame.filename?.endsWith('.min.js')) {
+        // Cases where collecting context lines is either not useful or possible
+        // - .min.js files are and not useful since they dont point to the original source
+        // - node: prefixed modules are part of the runtime and cannot be resolved to a file
+        if (frame.filename?.endsWith('.min.js') || frame.filename?.startsWith('node:')) {
           continue;
         }
 
@@ -172,16 +171,16 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
     filesToLines[file].sort((a, b) => a - b);
     const ranges = makeLineReaderRanges(filesToLines[file], contextLines);
 
-    let cache = LRUFileContentCache.get(file);
+    let cache = LRU_FILE_CONTENTS_CACHE.get(file);
     if (!cache) {
       cache = {};
-      LRUFileContentCache.set(file, cache);
+      LRU_FILE_CONTENTS_CACHE.set(file, cache);
     }
     readlinePromises.push(getContextLinesFromFile(file, ranges, cache));
   }
 
+  // The promise rejections are caught in order to prevent them from short circuiting Promise.all
   await Promise.all(readlinePromises).catch(() => {
-    // We don't want to error if we can't read the file.
     DEBUG_BUILD && logger.log('Failed to read one or more source files and resolve context lines');
   });
 
@@ -190,7 +189,7 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
   if (contextLines > 0 && event.exception?.values) {
     for (const exception of event.exception.values) {
       if (exception.stacktrace && exception.stacktrace.frames && exception.stacktrace.frames.length > 0) {
-        addSourceContextToFrames(exception.stacktrace.frames, contextLines, LRUFileContentCache);
+        addSourceContextToFrames(exception.stacktrace.frames, contextLines, LRU_FILE_CONTENTS_CACHE);
       }
     }
   }
@@ -211,6 +210,10 @@ function addSourceContextToFrames(frames: StackFrame[], contextLines: number, ca
   }
 }
 
+/**
+ * Clears the context lines from a frame, used to reset a frame to its original state
+ * if we fail to resolve all context lines for it.
+ */
 function clearLineContext(frame: StackFrame): void {
   delete frame.pre_context;
   delete frame.context_line;
@@ -229,39 +232,40 @@ export function addContextToFrame(
   // When there is no line number in the frame, attaching context is nonsensical and will even break grouping.
   // We already check for lineno before calling this, but since StackFrame lineno ism optional, we check it again.
   if (frame.lineno === undefined || contents === undefined) {
+    DEBUG_BUILD && logger.error('Cannot resolve context for frame with no lineno or file contents');
     return;
   }
 
   frame.pre_context = [];
   for (let i = makeRangeStart(lineno, contextLines); i < lineno; i++) {
-    // Make sure to never send partial context lines
+    // We always expect the start context as line numbers cannot be negative. If we dont find a line, then
+    // something went wrong somewhere. Clear the context and return without adding any linecontext.
     if (contents[i] === undefined) {
       clearLineContext(frame);
       DEBUG_BUILD && logger.error(`Could not find line ${i} in file ${frame.filename}`);
-      console.log('Pre, could not find line', i, 'in file', frame.filename);
       return;
     }
 
     frame.pre_context.push(snipLine(contents[i], 0));
   }
 
+  // We should always have the context line. If we dont, something went wrong, so we clear the context and return
+  // without adding any linecontext.
   if (contents[lineno] === undefined) {
     clearLineContext(frame);
     DEBUG_BUILD && logger.error(`Could not find line ${lineno} in file ${frame.filename}`);
-    console.log('Lineno, could not find line', lineno, 'in file', frame.filename);
     return;
   }
   frame.context_line = snipLine(contents[lineno], frame.colno || 0);
 
+  const end = makeRangeEnd(lineno, contextLines);
   frame.post_context = [];
-  for (let i = lineno + 1; i < makeRangeEnd(lineno, contextLines); i++) {
+  for (let i = lineno + 1; i <= end; i++) {
+    // Since we dont track when the file ends, we cant clear the context if we dont find a line as it could
+    // just be that we reached the end of the file.
     if (contents[i] === undefined) {
-      clearLineContext(frame);
-      DEBUG_BUILD && logger.error(`Could not find line ${lineno} in file ${frame.filename}`);
-      console.log('Post, could not find line', i, 'in file', frame.filename)
-      return;
+      break;
     }
-
     frame.post_context.push(snipLine(contents[i], 0));
   }
 }
@@ -273,7 +277,7 @@ function makeRangeStart(line: number, linecontext: number): number {
   return Math.max(1, line - linecontext);
 }
 function makeRangeEnd(line: number, linecontext: number): number {
-  return line + linecontext + 1;
+  return line + linecontext;
 }
 function makeContextRange(line: number, linecontext: number): [start: number, end: number] {
   return [makeRangeStart(line, linecontext), makeRangeEnd(line, linecontext)];
diff --git a/packages/node/test/integrations/contextlines.test.ts b/packages/node/test/integrations/contextlines.test.ts
index be889aea489f..90f5233e8d0e 100644
--- a/packages/node/test/integrations/contextlines.test.ts
+++ b/packages/node/test/integrations/contextlines.test.ts
@@ -39,7 +39,7 @@ describe('ContextLines', () => {
 
       // Calls to `readFile` shouldn't increase if there isn't a new error to
       // parse whose stacktrace contains a file we haven't yet seen
-      expect(readStreamSpy).toHaveBeenCalledTimes(numCalls * 2);
+      expect(readStreamSpy).toHaveBeenCalledTimes(numCalls);
     });
 
     test('parseStack with ESM module names', async () => {
@@ -103,12 +103,15 @@ describe('ContextLines', () => {
 
       await addContext(overlappingContextWithFirstError);
 
-      const errorFrame = overlappingContextWithFirstError[overlappingContextWithFirstError.length - 1];
-      console.log(errorFrame)
+      const errorFrame = overlappingContextWithFirstError.find(f => f.filename?.endsWith('error.ts'))
+
+      if (!errorFrame) {
+        throw new Error('Could not find error frame');
+      }
 
       expect(errorFrame.context_line).toBe("  return new Error('mock error');");
-      expect(errorFrame.pre_context).toHaveLength(7)
-      expect(errorFrame.post_context).toHaveLength(7)
+      expect(errorFrame.pre_context).toHaveLength(2)
+      expect(errorFrame.post_context).toHaveLength(1)
     });
 
     test('parseStack with duplicate files', async () => {

From 3a785bcab0f5c161cf3dc13a4e413df95fbcdf63 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Tue, 28 May 2024 11:03:47 -0400
Subject: [PATCH 04/23] ref(node): contextlines cache attempt

---
 .../node/src/integrations/contextlines.ts     | 117 ++++++++++++------
 .../test/integrations/contextlines.test.ts    |   2 +-
 2 files changed, 77 insertions(+), 42 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index 90ead43eef50..cc62b3a4c6f5 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -10,6 +10,16 @@ const LRU_FILE_CONTENTS_CACHE = new LRUMap<string, Record<number, string>>(10);
 const DEFAULT_LINES_OF_CONTEXT = 7;
 const INTEGRATION_NAME = 'ContextLines';
 
+interface ContextLinesOptions {
+  /**
+   * Sets the number of context lines for each frame when loading a file.
+   * Defaults to 7.
+   *
+   * Set to 0 to disable loading and inclusion of source files.
+   **/
+  frameContextLines?: number;
+}
+
 /**
  * Exported for testing purposes.
  */
@@ -17,12 +27,36 @@ export function resetFileContentCache(): void {
   LRU_FILE_CONTENTS_CACHE.clear();
 }
 
+/**
+ * Determines if context lines should be skipped for a file.
+ * - .min.js files are and not useful since they dont point to the original source
+ * - node: prefixed modules are part of the runtime and cannot be resolved to a file
+ */
+function shouldSkipContextLinesForFile(path: string): boolean {
+  return path.endsWith('.min.js') || path.startsWith('node:');
+}
+/**
+ * Checks if we have all the contents that we need in the cache.
+ */
+function rangeExistsInContentCache(file: string, range: ReadlineRange): boolean {
+  const contents = LRU_FILE_CONTENTS_CACHE.get(file);
+
+  if (!contents) {
+    return false;
+  }
+
+  for (let i = range[0]; i <= range[1]; i++) {
+    if (!contents[i]) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
 /**
  * Creates contiguous ranges of lines to read from a file. In the case where context lines overlap,
  * the ranges are merged to create a single range.
- * @param lines
- * @param linecontext
- * @returns
  */
 function makeLineReaderRanges(lines: number[], linecontext: number): ReadlineRange[] {
   if (!lines.length) {
@@ -60,7 +94,8 @@ function makeLineReaderRanges(lines: number[], linecontext: number): ReadlineRan
 function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output: Record<number, string>): Promise<void> {
   return new Promise((resolve, _reject) => {
     // It is important *not* to have any async code between createInterface and the 'line' event listener
-    // as it will cause the 'line' event to be emitted before the listener is attached.
+    // as it will cause the 'line' event to
+    // be emitted before the listener is attached.
     const fileStream = createInterface({
       input: createReadStream(path),
     });
@@ -101,34 +136,6 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
   });
 }
 
-interface ContextLinesOptions {
-  /**
-   * Sets the number of context lines for each frame when loading a file.
-   * Defaults to 7.
-   *
-   * Set to 0 to disable loading and inclusion of source files.
-   **/
-  frameContextLines?: number;
-}
-
-/** Exported on
- * ly for tests, as a type-safe variant. */
-export const _contextLinesIntegration = ((options: ContextLinesOptions = {}) => {
-  const contextLines = options.frameContextLines !== undefined ? options.frameContextLines : DEFAULT_LINES_OF_CONTEXT;
-
-  return {
-    name: INTEGRATION_NAME,
-    processEvent(event) {
-      return addSourceContext(event, contextLines);
-    },
-  };
-}) satisfies IntegrationFn;
-
-/**
- * Capture the lines before and after the frame's context.
- */
-export const contextLinesIntegration = defineIntegration(_contextLinesIntegration);
-
 async function addSourceContext(event: Event, contextLines: number): Promise<Event> {
   // keep a lookup map of which files we've already enqueued to read,
   // so we don't enqueue the same file multiple times which would cause multiple i/o reads
@@ -145,17 +152,16 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
       for (let i = exception.stacktrace.frames.length - 1; i >= 0; i--) {
         const frame = exception.stacktrace.frames[i];
 
-        // Cases where collecting context lines is either not useful or possible
-        // - .min.js files are and not useful since they dont point to the original source
-        // - node: prefixed modules are part of the runtime and cannot be resolved to a file
-        if (frame.filename?.endsWith('.min.js') || frame.filename?.startsWith('node:')) {
+        if (
+          typeof frame.filename !== 'string' ||
+          typeof frame.lineno !== 'number' ||
+          shouldSkipContextLinesForFile(frame.filename)
+        ) {
           continue;
         }
 
-        if (frame.filename && typeof frame.lineno === 'number') {
-          if (!filesToLines[frame.filename]) filesToLines[frame.filename] = [];
-          filesToLines[frame.filename].push(frame.lineno);
-        }
+        if (!filesToLines[frame.filename]) filesToLines[frame.filename] = [];
+        filesToLines[frame.filename].push(frame.lineno);
       }
     }
   }
@@ -171,6 +177,11 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
     filesToLines[file].sort((a, b) => a - b);
     const ranges = makeLineReaderRanges(filesToLines[file], contextLines);
 
+    // If the contents are already in the cache, then we dont need to read the file.
+    if (ranges.every(r => rangeExistsInContentCache(file, r))) {
+      continue;
+    }
+
     let cache = LRU_FILE_CONTENTS_CACHE.get(file);
     if (!cache) {
       cache = {};
@@ -198,7 +209,11 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
 }
 
 /** Adds context lines to frames */
-function addSourceContextToFrames(frames: StackFrame[], contextLines: number, cache: LRUMap<string, Record<number, string>>): void {
+function addSourceContextToFrames(
+  frames: StackFrame[],
+  contextLines: number,
+  cache: LRUMap<string, Record<number, string>>,
+): void {
   for (const frame of frames) {
     // Only add context if we have a filename and it hasn't already been added
     if (frame.filename && frame.context_line === undefined && typeof frame.lineno === 'number') {
@@ -273,12 +288,32 @@ export function addContextToFrame(
 // Helper functions for generating line context ranges. They take a line number and the number of lines of context to
 // include before and after the line and generate an inclusive range of indices.
 type ReadlineRange = [start: number, end: number];
+// Compute inclusive end context range
 function makeRangeStart(line: number, linecontext: number): number {
   return Math.max(1, line - linecontext);
 }
+// Compute inclusive start context range
 function makeRangeEnd(line: number, linecontext: number): number {
   return line + linecontext;
 }
+// Determine start and end indices for context range (inclusive);
 function makeContextRange(line: number, linecontext: number): [start: number, end: number] {
   return [makeRangeStart(line, linecontext), makeRangeEnd(line, linecontext)];
 }
+
+/** Exported only for tests, as a type-safe variant. */
+export const _contextLinesIntegration = ((options: ContextLinesOptions = {}) => {
+  const contextLines = options.frameContextLines !== undefined ? options.frameContextLines : DEFAULT_LINES_OF_CONTEXT;
+
+  return {
+    name: INTEGRATION_NAME,
+    processEvent(event) {
+      return addSourceContext(event, contextLines);
+    },
+  };
+}) satisfies IntegrationFn;
+
+/**
+ * Capture the lines before and after the frame's context.
+ */
+export const contextLinesIntegration = defineIntegration(_contextLinesIntegration);
diff --git a/packages/node/test/integrations/contextlines.test.ts b/packages/node/test/integrations/contextlines.test.ts
index 90f5233e8d0e..0147b0fb4cf0 100644
--- a/packages/node/test/integrations/contextlines.test.ts
+++ b/packages/node/test/integrations/contextlines.test.ts
@@ -26,7 +26,7 @@ describe('ContextLines', () => {
   });
 
   describe('lru file cache', () => {
-    test('parseStack with same file', async () => {
+    test.only('parseStack with same file', async () => {
       expect.assertions(1);
 
       const frames = parseStackFrames(defaultStackParser, new Error('test'));

From 28f576ff73cae8babcc958443014e13b28814050 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Sun, 9 Jun 2024 19:27:21 -0400
Subject: [PATCH 05/23] ref(node) exclude other contextlines extensions

---
 packages/node/src/integrations/contextlines.ts | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index cc62b3a4c6f5..80f0aec0f26e 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -29,21 +29,20 @@ export function resetFileContentCache(): void {
 
 /**
  * Determines if context lines should be skipped for a file.
- * - .min.js files are and not useful since they dont point to the original source
+ * - .min.(mjs|cjs|js) files are and not useful since they dont point to the original source
  * - node: prefixed modules are part of the runtime and cannot be resolved to a file
+ * - data: skip json, wasm and inline js https://nodejs.org/api/esm.html#data-imports
  */
+const SKIP_CONTEXTLINES_REGEXP = /^(node|data):|\.min\.(mjs|cjs|js$)/;
 function shouldSkipContextLinesForFile(path: string): boolean {
-  return path.endsWith('.min.js') || path.startsWith('node:');
+  return SKIP_CONTEXTLINES_REGEXP.test(path);
 }
 /**
  * Checks if we have all the contents that we need in the cache.
  */
 function rangeExistsInContentCache(file: string, range: ReadlineRange): boolean {
   const contents = LRU_FILE_CONTENTS_CACHE.get(file);
-
-  if (!contents) {
-    return false;
-  }
+  if (!contents) return false;
 
   for (let i = range[0]; i <= range[1]; i++) {
     if (!contents[i]) {

From 2cfda9b41ccef22848ad2b6df510ca97b17eb0e1 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Sun, 9 Jun 2024 21:05:55 -0400
Subject: [PATCH 06/23] test(node) fix contextlines test

---
 packages/node/src/integrations/contextlines.ts       | 2 +-
 packages/node/test/integrations/contextlines.test.ts | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index 80f0aec0f26e..78b67037974b 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -45,7 +45,7 @@ function rangeExistsInContentCache(file: string, range: ReadlineRange): boolean
   if (!contents) return false;
 
   for (let i = range[0]; i <= range[1]; i++) {
-    if (!contents[i]) {
+    if (contents[i] === undefined) {
       return false;
     }
   }
diff --git a/packages/node/test/integrations/contextlines.test.ts b/packages/node/test/integrations/contextlines.test.ts
index 0147b0fb4cf0..b8c52721fc44 100644
--- a/packages/node/test/integrations/contextlines.test.ts
+++ b/packages/node/test/integrations/contextlines.test.ts
@@ -33,7 +33,6 @@ describe('ContextLines', () => {
       const readStreamSpy = jest.spyOn(fs, 'createReadStream');
 
       await addContext(frames);
-
       const numCalls = readStreamSpy.mock.calls.length;
       await addContext(frames);
 

From db2df8dfe0de3084bccc23fa49de0bc5eec42518 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Sun, 9 Jun 2024 21:08:40 -0400
Subject: [PATCH 07/23] fix scoped test

---
 .../test/integrations/contextlines.test.ts    | 21 ++++++++-----------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/packages/node/test/integrations/contextlines.test.ts b/packages/node/test/integrations/contextlines.test.ts
index b8c52721fc44..2ea18f6ea49b 100644
--- a/packages/node/test/integrations/contextlines.test.ts
+++ b/packages/node/test/integrations/contextlines.test.ts
@@ -2,10 +2,7 @@ import * as fs from 'node:fs';
 import type { StackFrame } from '@sentry/types';
 import { parseStackFrames } from '@sentry/utils';
 
-import {
-  _contextLinesIntegration,
-  resetFileContentCache,
-} from '../../src/integrations/contextlines';
+import { _contextLinesIntegration, resetFileContentCache } from '../../src/integrations/contextlines';
 import { defaultStackParser } from '../../src/sdk/api';
 import { getError } from '../helpers/error';
 
@@ -26,7 +23,7 @@ describe('ContextLines', () => {
   });
 
   describe('lru file cache', () => {
-    test.only('parseStack with same file', async () => {
+    test('parseStack with same file', async () => {
       expect.assertions(1);
 
       const frames = parseStackFrames(defaultStackParser, new Error('test'));
@@ -89,12 +86,12 @@ describe('ContextLines', () => {
       const outerFrame = overlappingContextWithFirstError[overlappingContextWithFirstError.length - 2];
 
       expect(innerFrame.context_line).toBe("        return new Error('inner');");
-      expect(innerFrame.pre_context).toHaveLength(7)
-      expect(innerFrame.post_context).toHaveLength(7)
+      expect(innerFrame.pre_context).toHaveLength(7);
+      expect(innerFrame.post_context).toHaveLength(7);
 
       expect(outerFrame.context_line).toBe('        return inner();');
-      expect(outerFrame.pre_context).toHaveLength(7)
-      expect(outerFrame.post_context).toHaveLength(7)
+      expect(outerFrame.pre_context).toHaveLength(7);
+      expect(outerFrame.post_context).toHaveLength(7);
     });
 
     test('parseStack with error on first line errors', async () => {
@@ -102,15 +99,15 @@ describe('ContextLines', () => {
 
       await addContext(overlappingContextWithFirstError);
 
-      const errorFrame = overlappingContextWithFirstError.find(f => f.filename?.endsWith('error.ts'))
+      const errorFrame = overlappingContextWithFirstError.find(f => f.filename?.endsWith('error.ts'));
 
       if (!errorFrame) {
         throw new Error('Could not find error frame');
       }
 
       expect(errorFrame.context_line).toBe("  return new Error('mock error');");
-      expect(errorFrame.pre_context).toHaveLength(2)
-      expect(errorFrame.post_context).toHaveLength(1)
+      expect(errorFrame.pre_context).toHaveLength(2);
+      expect(errorFrame.post_context).toHaveLength(1);
     });
 
     test('parseStack with duplicate files', async () => {

From 2be28a51b1c4e0f573a8b1d280fe542020cd6d0f Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Mon, 10 Jun 2024 09:57:41 -0400
Subject: [PATCH 08/23] lint

---
 packages/node/src/integrations/contextlines.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index 78b67037974b..efd7229843a4 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -2,7 +2,7 @@ import { createReadStream } from 'node:fs';
 import { createInterface } from 'node:readline';
 import { defineIntegration } from '@sentry/core';
 import type { Event, IntegrationFn, StackFrame } from '@sentry/types';
-import { logger, LRUMap, snipLine } from '@sentry/utils';
+import { LRUMap, logger, snipLine } from '@sentry/utils';
 
 import { DEBUG_BUILD } from '../debug-build';
 

From 93e81732274e1eb08235fbaecb045c9397195ed0 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Mon, 10 Jun 2024 09:58:42 -0400
Subject: [PATCH 09/23] fix regexp danger warning

---
 packages/node/src/integrations/contextlines.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index efd7229843a4..e20e6ebcca38 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -33,7 +33,7 @@ export function resetFileContentCache(): void {
  * - node: prefixed modules are part of the runtime and cannot be resolved to a file
  * - data: skip json, wasm and inline js https://nodejs.org/api/esm.html#data-imports
  */
-const SKIP_CONTEXTLINES_REGEXP = /^(node|data):|\.min\.(mjs|cjs|js$)/;
+const SKIP_CONTEXTLINES_REGEXP = /(^(node|data):)|(\.min\.(mjs|cjs|js$))/;
 function shouldSkipContextLinesForFile(path: string): boolean {
   return SKIP_CONTEXTLINES_REGEXP.test(path);
 }

From 584dad2dc0087519d40821c7d52d2d78aca44d73 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Mon, 10 Jun 2024 19:04:12 -0400
Subject: [PATCH 10/23] strict undefined check

---
 packages/node/src/integrations/contextlines.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index e20e6ebcca38..403cc4cebed8 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -42,7 +42,7 @@ function shouldSkipContextLinesForFile(path: string): boolean {
  */
 function rangeExistsInContentCache(file: string, range: ReadlineRange): boolean {
   const contents = LRU_FILE_CONTENTS_CACHE.get(file);
-  if (!contents) return false;
+  if (contents === undefined) return false;
 
   for (let i = range[0]; i <= range[1]; i++) {
     if (contents[i] === undefined) {
@@ -182,7 +182,7 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
     }
 
     let cache = LRU_FILE_CONTENTS_CACHE.get(file);
-    if (!cache) {
+    if (cache === undefined) {
       cache = {};
       LRU_FILE_CONTENTS_CACHE.set(file, cache);
     }

From 00fd3fc582a44b501bc178539e4eeb25b0519118 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Mon, 10 Jun 2024 19:36:43 -0400
Subject: [PATCH 11/23] reshuffle order

---
 .../node/src/integrations/contextlines.ts     | 18 ++++++++++--------
 .../test/integrations/contextlines.test.ts    | 19 +++++++++++++++++++
 2 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index 403cc4cebed8..59d3f7c03770 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -105,6 +105,16 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
     let rangeStart = ranges[currentRangeIndex][0];
     let rangeEnd = ranges[currentRangeIndex][1];
 
+    // We use this inside Promise.all, so we need to resolve the promise even if there is an error
+    // to prevent Promise.all from short circuiting the rest.
+    fileStream.on('error', e => {
+      DEBUG_BUILD && logger.error(`Failed to read file: ${path}. Error: ${e}`);
+      fileStream.close();
+      fileStream.removeAllListeners();
+      resolve();
+    });
+    fileStream.on('close', resolve);
+
     fileStream.on('line', line => {
       lineNumber++;
       if (lineNumber < rangeStart) return;
@@ -124,14 +134,6 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
         rangeEnd = ranges[currentRangeIndex][1];
       }
     });
-
-    fileStream.on('close', resolve);
-    // We use this inside Promise.all, so we need to resolve the promise even if there is an error
-    // to prevent Promise.all from short circuiting the rest.
-    fileStream.on('error', e => {
-      DEBUG_BUILD && logger.error(`Failed to read file: ${path}. Error: ${e}`);
-      resolve();
-    });
   });
 }
 
diff --git a/packages/node/test/integrations/contextlines.test.ts b/packages/node/test/integrations/contextlines.test.ts
index 2ea18f6ea49b..e28078c3481f 100644
--- a/packages/node/test/integrations/contextlines.test.ts
+++ b/packages/node/test/integrations/contextlines.test.ts
@@ -23,6 +23,25 @@ describe('ContextLines', () => {
   });
 
   describe('lru file cache', () => {
+    test('parseStack when file does not exist', async () => {
+      expect.assertions(4)
+      const frames: StackFrame[] = [
+        {
+          colno: 1,
+          filename: 'file:///var/task/nonexistent.js',
+          lineno: 1,
+          function: 'fxn1',
+        },
+      ];
+
+      const readStreamSpy = jest.spyOn(fs, 'createReadStream');
+      await addContext(frames);
+
+      expect(frames[0].pre_context).toBeUndefined();
+      expect(frames[0].post_context).toBeUndefined();
+      expect(frames[0].context_line).toBeUndefined();
+      expect(readStreamSpy).toHaveBeenCalledTimes(1);
+    })
     test('parseStack with same file', async () => {
       expect.assertions(1);
 

From 6588673754f27b6014340a3c55ed292e7b53a5d2 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Mon, 10 Jun 2024 19:46:30 -0400
Subject: [PATCH 12/23] test adding error handler on input stream

---
 .../node/src/integrations/contextlines.ts     | 26 ++++++++++++-------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index 59d3f7c03770..1680ea2b7804 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -95,8 +95,9 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
     // It is important *not* to have any async code between createInterface and the 'line' event listener
     // as it will cause the 'line' event to
     // be emitted before the listener is attached.
-    const fileStream = createInterface({
-      input: createReadStream(path),
+    const stream = createReadStream(path)
+    const lineReaded = createInterface({
+      input: stream,
     });
 
     // Init at zero and increment at the start of the loop because lines are 1 indexed.
@@ -107,15 +108,20 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
 
     // We use this inside Promise.all, so we need to resolve the promise even if there is an error
     // to prevent Promise.all from short circuiting the rest.
-    fileStream.on('error', e => {
+    function onStreamError(e: Error): void {
       DEBUG_BUILD && logger.error(`Failed to read file: ${path}. Error: ${e}`);
-      fileStream.close();
-      fileStream.removeAllListeners();
+      lineReaded.close();
+      lineReaded.removeAllListeners();
       resolve();
-    });
-    fileStream.on('close', resolve);
+    }
+
+    // We need to handle the error event to prevent the process from crashing in < Node 16
+    // https://github.com/nodejs/node/pull/31603
+    stream.on('error', onStreamError);
+    lineReaded.on('error', onStreamError);
+    lineReaded.on('close', resolve);
 
-    fileStream.on('line', line => {
+    lineReaded.on('line', line => {
       lineNumber++;
       if (lineNumber < rangeStart) return;
 
@@ -125,8 +131,8 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
       if (lineNumber >= rangeEnd) {
         if (currentRangeIndex === ranges.length - 1) {
           // We need to close the file stream and remove listeners, else the reader will continue to run our listener;
-          fileStream.close();
-          fileStream.removeAllListeners();
+          lineReaded.close();
+          lineReaded.removeAllListeners();
           return;
         }
         currentRangeIndex++;

From 96a617e5c90dda8cfa85154f113e39f93a4c92a2 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Mon, 10 Jun 2024 19:59:18 -0400
Subject: [PATCH 13/23] lint

---
 packages/node/src/integrations/contextlines.ts       | 8 +++++---
 packages/node/test/integrations/contextlines.test.ts | 4 ++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index 1680ea2b7804..feace7b07acc 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -95,7 +95,7 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
     // It is important *not* to have any async code between createInterface and the 'line' event listener
     // as it will cause the 'line' event to
     // be emitted before the listener is attached.
-    const stream = createReadStream(path)
+    const stream = createReadStream(path);
     const lineReaded = createInterface({
       input: stream,
     });
@@ -225,9 +225,11 @@ function addSourceContextToFrames(
     // Only add context if we have a filename and it hasn't already been added
     if (frame.filename && frame.context_line === undefined && typeof frame.lineno === 'number') {
       const contents = cache.get(frame.filename);
-      if (contents) {
-        addContextToFrame(frame.lineno, frame, contextLines, contents);
+      if (contents === undefined) {
+        continue;
       }
+
+      addContextToFrame(frame.lineno, frame, contextLines, contents);
     }
   }
 }
diff --git a/packages/node/test/integrations/contextlines.test.ts b/packages/node/test/integrations/contextlines.test.ts
index e28078c3481f..d3e1c4403ea3 100644
--- a/packages/node/test/integrations/contextlines.test.ts
+++ b/packages/node/test/integrations/contextlines.test.ts
@@ -24,7 +24,7 @@ describe('ContextLines', () => {
 
   describe('lru file cache', () => {
     test('parseStack when file does not exist', async () => {
-      expect.assertions(4)
+      expect.assertions(4);
       const frames: StackFrame[] = [
         {
           colno: 1,
@@ -41,7 +41,7 @@ describe('ContextLines', () => {
       expect(frames[0].post_context).toBeUndefined();
       expect(frames[0].context_line).toBeUndefined();
       expect(readStreamSpy).toHaveBeenCalledTimes(1);
-    })
+    });
     test('parseStack with same file', async () => {
       expect.assertions(1);
 

From b4a51f6ddecd3bfe32492f169d3a477308f59ac7 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Mon, 10 Jun 2024 20:11:04 -0400
Subject: [PATCH 14/23] fix duplicate reads on stream error

---
 packages/node/src/integrations/contextlines.ts | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index feace7b07acc..2ddd846bc664 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -6,6 +6,7 @@ import { LRUMap, logger, snipLine } from '@sentry/utils';
 
 import { DEBUG_BUILD } from '../debug-build';
 
+const FILE_CONTENTS_FS_READ_FAILED = new LRUMap<string, 1>(20);
 const LRU_FILE_CONTENTS_CACHE = new LRUMap<string, Record<number, string>>(10);
 const DEFAULT_LINES_OF_CONTEXT = 7;
 const INTEGRATION_NAME = 'ContextLines';
@@ -109,6 +110,8 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
     // We use this inside Promise.all, so we need to resolve the promise even if there is an error
     // to prevent Promise.all from short circuiting the rest.
     function onStreamError(e: Error): void {
+      // Mark file path as failed to read and prevent multiple read attempts.
+      FILE_CONTENTS_FS_READ_FAILED.set(path, 1);
       DEBUG_BUILD && logger.error(`Failed to read file: ${path}. Error: ${e}`);
       lineReaded.close();
       lineReaded.removeAllListeners();
@@ -180,11 +183,15 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
 
   const readlinePromises: Promise<void>[] = [];
   for (const file of files) {
+    // If we failed to read this before, dont try again.
+    if (FILE_CONTENTS_FS_READ_FAILED.get(file)) {
+      continue;
+    }
+
+    // Check if the contents are already in the cache and if we can avoid reading the file again.
     // Sort ranges so that they are sorted by line increasing order and match how the file is read.
     filesToLines[file].sort((a, b) => a - b);
     const ranges = makeLineReaderRanges(filesToLines[file], contextLines);
-
-    // If the contents are already in the cache, then we dont need to read the file.
     if (ranges.every(r => rangeExistsInContentCache(file, r))) {
       continue;
     }

From 421178ea623d02ae4ed89cbc7af37307507e1cb3 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Mon, 10 Jun 2024 20:29:39 -0400
Subject: [PATCH 15/23] fix duplicate reads on stream error

---
 packages/node/src/integrations/contextlines.ts | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index 2ddd846bc664..e02351242e94 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -146,6 +146,7 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
   });
 }
 
+// eslint-disable-next-line complexity
 async function addSourceContext(event: Event, contextLines: number): Promise<Event> {
   // keep a lookup map of which files we've already enqueued to read,
   // so we don't enqueue the same file multiple times which would cause multiple i/o reads
@@ -188,9 +189,9 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
       continue;
     }
 
-    // Check if the contents are already in the cache and if we can avoid reading the file again.
     // Sort ranges so that they are sorted by line increasing order and match how the file is read.
     filesToLines[file].sort((a, b) => a - b);
+    // Check if the contents are already in the cache and if we can avoid reading the file again.
     const ranges = makeLineReaderRanges(filesToLines[file], contextLines);
     if (ranges.every(r => rangeExistsInContentCache(file, r))) {
       continue;

From 306a11bc8784278f56d420d9aa626c9440be473e Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Mon, 10 Jun 2024 20:38:42 -0400
Subject: [PATCH 16/23] add emplace

---
 .../node/src/integrations/contextlines.ts     | 21 +++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index e02351242e94..f1f02ce44387 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -28,6 +28,20 @@ export function resetFileContentCache(): void {
   LRU_FILE_CONTENTS_CACHE.clear();
 }
 
+/**
+ * Get or init map value
+ */
+function emplace<T extends LRUMap<K, V>, K extends string, V>(map: T, key: K, contents: V): V {
+  const value = map.get(key);
+
+  if (value === undefined) {
+    map.set(key, contents);
+    return contents;
+  }
+
+  return value;
+}
+
 /**
  * Determines if context lines should be skipped for a file.
  * - .min.(mjs|cjs|js) files are and not useful since they dont point to the original source
@@ -146,7 +160,6 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
   });
 }
 
-// eslint-disable-next-line complexity
 async function addSourceContext(event: Event, contextLines: number): Promise<Event> {
   // keep a lookup map of which files we've already enqueued to read,
   // so we don't enqueue the same file multiple times which would cause multiple i/o reads
@@ -197,11 +210,7 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
       continue;
     }
 
-    let cache = LRU_FILE_CONTENTS_CACHE.get(file);
-    if (cache === undefined) {
-      cache = {};
-      LRU_FILE_CONTENTS_CACHE.set(file, cache);
-    }
+    const cache = emplace(LRU_FILE_CONTENTS_CACHE, file, {});
     readlinePromises.push(getContextLinesFromFile(file, ranges, cache));
   }
 

From 0dca6c50276475135a42dba50157016d88b1c08b Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Tue, 11 Jun 2024 07:52:02 -0400
Subject: [PATCH 17/23] fix(node) last line is not picked up as it does not
 include \n

---
 .../public-api/captureException/catched-error/test.ts       | 2 +-
 packages/node/src/integrations/contextlines.ts              | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/dev-packages/node-integration-tests/suites/public-api/captureException/catched-error/test.ts b/dev-packages/node-integration-tests/suites/public-api/captureException/catched-error/test.ts
index 6d04fee01291..1ea1bf3b8569 100644
--- a/dev-packages/node-integration-tests/suites/public-api/captureException/catched-error/test.ts
+++ b/dev-packages/node-integration-tests/suites/public-api/captureException/catched-error/test.ts
@@ -30,7 +30,7 @@ test('should work inside catch block', done => {
                       '',
                       'try {',
                     ],
-                    post_context: ['} catch (err) {', '  Sentry.captureException(err);', '}', ''],
+                    post_context: ['} catch (err) {', '  Sentry.captureException(err);', '}'],
                   }),
                 ]),
               },
diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index f1f02ce44387..03972f022da7 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -6,8 +6,8 @@ import { LRUMap, logger, snipLine } from '@sentry/utils';
 
 import { DEBUG_BUILD } from '../debug-build';
 
-const FILE_CONTENTS_FS_READ_FAILED = new LRUMap<string, 1>(20);
 const LRU_FILE_CONTENTS_CACHE = new LRUMap<string, Record<number, string>>(10);
+const LRU_FILE_CONTENTS_FS_READ_FAILED = new LRUMap<string, 1>(20);
 const DEFAULT_LINES_OF_CONTEXT = 7;
 const INTEGRATION_NAME = 'ContextLines';
 
@@ -125,7 +125,7 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
     // to prevent Promise.all from short circuiting the rest.
     function onStreamError(e: Error): void {
       // Mark file path as failed to read and prevent multiple read attempts.
-      FILE_CONTENTS_FS_READ_FAILED.set(path, 1);
+      LRU_FILE_CONTENTS_FS_READ_FAILED.set(path, 1);
       DEBUG_BUILD && logger.error(`Failed to read file: ${path}. Error: ${e}`);
       lineReaded.close();
       lineReaded.removeAllListeners();
@@ -198,7 +198,7 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
   const readlinePromises: Promise<void>[] = [];
   for (const file of files) {
     // If we failed to read this before, dont try again.
-    if (FILE_CONTENTS_FS_READ_FAILED.get(file)) {
+    if (LRU_FILE_CONTENTS_FS_READ_FAILED.get(file)) {
       continue;
     }
 

From dea113b14934672a2a4ed8896afdbad1daf5950f Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Wed, 12 Jun 2024 14:52:51 -0400
Subject: [PATCH 18/23] ref(contexlines) use ordered startsWith instead of
 regexp and store cached snipLine values

---
 packages/node/src/integrations/contextlines.ts | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index 03972f022da7..f037f4f9c1db 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -48,9 +48,15 @@ function emplace<T extends LRUMap<K, V>, K extends string, V>(map: T, key: K, co
  * - node: prefixed modules are part of the runtime and cannot be resolved to a file
  * - data: skip json, wasm and inline js https://nodejs.org/api/esm.html#data-imports
  */
-const SKIP_CONTEXTLINES_REGEXP = /(^(node|data):)|(\.min\.(mjs|cjs|js$))/;
 function shouldSkipContextLinesForFile(path: string): boolean {
-  return SKIP_CONTEXTLINES_REGEXP.test(path);
+  // Test the most common prefix and extension first. These are the ones we
+  // are most likely to see in user applications and are the ones we can break out of first.
+  if (path.startsWith('node:')) return true;
+  if (path.endsWith('.min.js')) return true;
+  if (path.endsWith('.min.cjs')) return true;
+  if (path.endsWith('.min.mjs')) return true;
+  if (path.startsWith('data:')) return true;
+  return false;
 }
 /**
  * Checks if we have all the contents that we need in the cache.
@@ -142,8 +148,8 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
       lineNumber++;
       if (lineNumber < rangeStart) return;
 
-      // !Warning: Mutates the cache value
-      output[lineNumber] = line;
+      // !Warning: This mutates the cache by storing the snipped line into the cache.
+      output[lineNumber] = snipLine(line, 0);
 
       if (lineNumber >= rangeEnd) {
         if (currentRangeIndex === ranges.length - 1) {
@@ -287,7 +293,7 @@ export function addContextToFrame(
       return;
     }
 
-    frame.pre_context.push(snipLine(contents[i], 0));
+    frame.pre_context.push(contents[i]);
   }
 
   // We should always have the context line. If we dont, something went wrong, so we clear the context and return

From 9af7dfc6904ee7ae03bb39f255b8fdfdf5e6ecac Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Wed, 12 Jun 2024 17:10:34 -0400
Subject: [PATCH 19/23] ref(node) remove all but cache set snipline occurences

---
 packages/node/src/integrations/contextlines.ts | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index f037f4f9c1db..96a68ace76b3 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -303,7 +303,8 @@ export function addContextToFrame(
     DEBUG_BUILD && logger.error(`Could not find line ${lineno} in file ${frame.filename}`);
     return;
   }
-  frame.context_line = snipLine(contents[lineno], frame.colno || 0);
+
+  frame.context_line = contents[lineno];
 
   const end = makeRangeEnd(lineno, contextLines);
   frame.post_context = [];
@@ -313,7 +314,7 @@ export function addContextToFrame(
     if (contents[i] === undefined) {
       break;
     }
-    frame.post_context.push(snipLine(contents[i], 0));
+    frame.post_context.push(contents[i]);
   }
 }
 

From e79dac25be3ac4a8deeb03a81846403caf1b9c70 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Mon, 17 Jun 2024 10:14:05 -0400
Subject: [PATCH 20/23] fix(contextlines) update type assertions

---
 .../node/src/integrations/contextlines.ts     | 45 +++++++++++--------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index 96a68ace76b3..f6ca14dffb00 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -84,7 +84,7 @@ function makeLineReaderRanges(lines: number[], linecontext: number): ReadlineRan
   }
 
   let i = 0;
-  let current = makeContextRange(lines[i], linecontext);
+  let current = makeContextRange(lines[i]!, linecontext);
   const out: ReadlineRange[] = [];
   // eslint-disable-next-line no-constant-condition
   while (true) {
@@ -94,7 +94,7 @@ function makeLineReaderRanges(lines: number[], linecontext: number): ReadlineRan
     }
 
     // If the next line falls into the current range, extend the current range to lineno + linecontext.
-    const next = lines[i + 1];
+    const next = lines[i + 1]!;
     if (next <= current[1]) {
       current[1] = next + linecontext;
     } else {
@@ -124,8 +124,8 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
     // Init at zero and increment at the start of the loop because lines are 1 indexed.
     let lineNumber = 0;
     let currentRangeIndex = 0;
-    let rangeStart = ranges[currentRangeIndex][0];
-    let rangeEnd = ranges[currentRangeIndex][1];
+    let rangeStart = ranges[currentRangeIndex]?.[0]!;
+    let rangeEnd = ranges[currentRangeIndex]?.[1]!;
 
     // We use this inside Promise.all, so we need to resolve the promise even if there is an error
     // to prevent Promise.all from short circuiting the rest.
@@ -159,8 +159,8 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
           return;
         }
         currentRangeIndex++;
-        rangeStart = ranges[currentRangeIndex][0];
-        rangeEnd = ranges[currentRangeIndex][1];
+        rangeStart = ranges[currentRangeIndex]?.[0]!;
+        rangeEnd = ranges[currentRangeIndex]?.[1]!;
       }
     });
   });
@@ -180,18 +180,20 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
       // Maps preserve insertion order, so we iterate in reverse, starting at the
       // outermost frame and closer to where the exception has occurred (poor mans priority)
       for (let i = exception.stacktrace.frames.length - 1; i >= 0; i--) {
-        const frame = exception.stacktrace.frames[i];
+        const frame: StackFrame | undefined = exception.stacktrace.frames[i];
+        const filename = frame?.filename;
 
         if (
-          typeof frame.filename !== 'string' ||
+          !frame ||
+          typeof filename !== 'string' ||
           typeof frame.lineno !== 'number' ||
-          shouldSkipContextLinesForFile(frame.filename)
+          shouldSkipContextLinesForFile(filename)
         ) {
           continue;
         }
 
-        if (!filesToLines[frame.filename]) filesToLines[frame.filename] = [];
-        filesToLines[frame.filename].push(frame.lineno);
+        if (!filesToLines[filename]) filesToLines[filename] = [];
+        filesToLines[filename]!.push(frame.lineno);
       }
     }
   }
@@ -203,15 +205,20 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
 
   const readlinePromises: Promise<void>[] = [];
   for (const file of files) {
-    // If we failed to read this before, dont try again.
+    // If we failed to read this before, dont try reading it again.
     if (LRU_FILE_CONTENTS_FS_READ_FAILED.get(file)) {
       continue;
     }
 
+    const filesToLineRanges = filesToLines[file];
+    if (!filesToLineRanges) {
+      continue;
+    }
+
     // Sort ranges so that they are sorted by line increasing order and match how the file is read.
-    filesToLines[file].sort((a, b) => a - b);
+    filesToLineRanges.sort((a, b) => a - b);
     // Check if the contents are already in the cache and if we can avoid reading the file again.
-    const ranges = makeLineReaderRanges(filesToLines[file], contextLines);
+    const ranges = makeLineReaderRanges(filesToLineRanges, contextLines);
     if (ranges.every(r => rangeExistsInContentCache(file, r))) {
       continue;
     }
@@ -287,13 +294,14 @@ export function addContextToFrame(
   for (let i = makeRangeStart(lineno, contextLines); i < lineno; i++) {
     // We always expect the start context as line numbers cannot be negative. If we dont find a line, then
     // something went wrong somewhere. Clear the context and return without adding any linecontext.
-    if (contents[i] === undefined) {
+    const line = contents[i];
+    if (line === undefined) {
       clearLineContext(frame);
       DEBUG_BUILD && logger.error(`Could not find line ${i} in file ${frame.filename}`);
       return;
     }
 
-    frame.pre_context.push(contents[i]);
+    frame.pre_context.push(line);
   }
 
   // We should always have the context line. If we dont, something went wrong, so we clear the context and return
@@ -311,10 +319,11 @@ export function addContextToFrame(
   for (let i = lineno + 1; i <= end; i++) {
     // Since we dont track when the file ends, we cant clear the context if we dont find a line as it could
     // just be that we reached the end of the file.
-    if (contents[i] === undefined) {
+    let line = contents[i];
+    if (line === undefined) {
       break;
     }
-    frame.post_context.push(contents[i]);
+    frame.post_context.push(line);
   }
 }
 

From ef1fa79829fc67701ef35cd790b2d5c1b7dd892c Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Mon, 17 Jun 2024 10:34:11 -0400
Subject: [PATCH 21/23] fix(contextlines) use runtime checks instead

---
 .../node/src/integrations/contextlines.ts     | 50 +++++++++++++++----
 1 file changed, 41 insertions(+), 9 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index f6ca14dffb00..a92e9e0c8687 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -84,7 +84,13 @@ function makeLineReaderRanges(lines: number[], linecontext: number): ReadlineRan
   }
 
   let i = 0;
-  let current = makeContextRange(lines[i]!, linecontext);
+  const line = lines[0];
+
+  if (typeof line !== 'number') {
+    return [];
+  }
+
+  let current = makeContextRange(line, linecontext);
   const out: ReadlineRange[] = [];
   // eslint-disable-next-line no-constant-condition
   while (true) {
@@ -94,7 +100,10 @@ function makeLineReaderRanges(lines: number[], linecontext: number): ReadlineRan
     }
 
     // If the next line falls into the current range, extend the current range to lineno + linecontext.
-    const next = lines[i + 1]!;
+    const next = lines[i + 1];
+    if (typeof next !== 'number') {
+      break;
+    }
     if (next <= current[1]) {
       current[1] = next + linecontext;
     } else {
@@ -124,8 +133,14 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
     // Init at zero and increment at the start of the loop because lines are 1 indexed.
     let lineNumber = 0;
     let currentRangeIndex = 0;
-    let rangeStart = ranges[currentRangeIndex]?.[0]!;
-    let rangeEnd = ranges[currentRangeIndex]?.[1]!;
+    const range = ranges[currentRangeIndex];
+    if (typeof range !== 'number') {
+      // We should never reach this point, but if we do, we should resolve the promise to prevent it from hanging.
+      resolve();
+      return;
+    }
+    let rangeStart = range[0];
+    let rangeEnd = range[1];
 
     // We use this inside Promise.all, so we need to resolve the promise even if there is an error
     // to prevent Promise.all from short circuiting the rest.
@@ -159,13 +174,27 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
           return;
         }
         currentRangeIndex++;
-        rangeStart = ranges[currentRangeIndex]?.[0]!;
-        rangeEnd = ranges[currentRangeIndex]?.[1]!;
+        const range = ranges[currentRangeIndex];
+        if (typeof range !== 'number') {
+          // This should never happen as it means we have a bug in the context.
+          lineReaded.close();
+          lineReaded.removeAllListeners();
+          return;
+        }
+        rangeStart = range[0];
+        rangeEnd = range[1];
       }
     });
   });
 }
 
+/**
+ * Adds surrounding (context) lines of the line that an exception occurred on to the event.
+ * This is done by reading the file line by line and extracting the lines. The extracted lines are stored in
+ * a cache to prevent multiple reads of the same file. Failures to read a file are similarly cached to prevent multiple
+ * failing reads from happening.
+ */
+/* eslint-disable complexity */
 async function addSourceContext(event: Event, contextLines: number): Promise<Event> {
   // keep a lookup map of which files we've already enqueued to read,
   // so we don't enqueue the same file multiple times which would cause multiple i/o reads
@@ -192,8 +221,10 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
           continue;
         }
 
-        if (!filesToLines[filename]) filesToLines[filename] = [];
-        filesToLines[filename]!.push(frame.lineno);
+        const filesToLinesOutput = filesToLines[filename];
+        if (!filesToLinesOutput) filesToLines[filename] = [];
+        // @ts-expect-error this is defined above
+        filesToLinesOutput.push(frame.lineno);
       }
     }
   }
@@ -244,6 +275,7 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
 
   return event;
 }
+/* eslint-enable complexity */
 
 /** Adds context lines to frames */
 function addSourceContextToFrames(
@@ -319,7 +351,7 @@ export function addContextToFrame(
   for (let i = lineno + 1; i <= end; i++) {
     // Since we dont track when the file ends, we cant clear the context if we dont find a line as it could
     // just be that we reached the end of the file.
-    let line = contents[i];
+    const line = contents[i];
     if (line === undefined) {
       break;
     }

From cc4305a763d23d44e21d04f08df1e26898846625 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Mon, 17 Jun 2024 10:50:56 -0400
Subject: [PATCH 22/23] ref(node) fix types in tests

---
 packages/node/src/integrations/contextlines.ts       |  2 +-
 packages/node/test/integrations/contextlines.test.ts | 10 +++++-----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index a92e9e0c8687..a01c7d0136ba 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -224,7 +224,7 @@ async function addSourceContext(event: Event, contextLines: number): Promise<Eve
         const filesToLinesOutput = filesToLines[filename];
         if (!filesToLinesOutput) filesToLines[filename] = [];
         // @ts-expect-error this is defined above
-        filesToLinesOutput.push(frame.lineno);
+        filesToLines[filename].push(frame.lineno);
       }
     }
   }
diff --git a/packages/node/test/integrations/contextlines.test.ts b/packages/node/test/integrations/contextlines.test.ts
index d3e1c4403ea3..14c2992eecad 100644
--- a/packages/node/test/integrations/contextlines.test.ts
+++ b/packages/node/test/integrations/contextlines.test.ts
@@ -37,9 +37,9 @@ describe('ContextLines', () => {
       const readStreamSpy = jest.spyOn(fs, 'createReadStream');
       await addContext(frames);
 
-      expect(frames[0].pre_context).toBeUndefined();
-      expect(frames[0].post_context).toBeUndefined();
-      expect(frames[0].context_line).toBeUndefined();
+      expect(frames[0]!.pre_context).toBeUndefined();
+      expect(frames[0]!.post_context).toBeUndefined();
+      expect(frames[0]!.context_line).toBeUndefined();
       expect(readStreamSpy).toHaveBeenCalledTimes(1);
     });
     test('parseStack with same file', async () => {
@@ -101,8 +101,8 @@ describe('ContextLines', () => {
 
       await addContext(overlappingContextWithFirstError);
 
-      const innerFrame = overlappingContextWithFirstError[overlappingContextWithFirstError.length - 1];
-      const outerFrame = overlappingContextWithFirstError[overlappingContextWithFirstError.length - 2];
+      const innerFrame = overlappingContextWithFirstError[overlappingContextWithFirstError.length - 1]!;
+      const outerFrame = overlappingContextWithFirstError[overlappingContextWithFirstError.length - 2]!;
 
       expect(innerFrame.context_line).toBe("        return new Error('inner');");
       expect(innerFrame.pre_context).toHaveLength(7);

From bd35821d90d117c8f59a129a0b1073d6cfdc3372 Mon Sep 17 00:00:00 2001
From: JonasBa <jonas@badalic.com>
Date: Mon, 17 Jun 2024 11:13:19 -0400
Subject: [PATCH 23/23] test(node) fix wrong type assertion

---
 packages/node/src/integrations/contextlines.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/node/src/integrations/contextlines.ts b/packages/node/src/integrations/contextlines.ts
index a01c7d0136ba..dd654fb3a153 100644
--- a/packages/node/src/integrations/contextlines.ts
+++ b/packages/node/src/integrations/contextlines.ts
@@ -134,7 +134,7 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
     let lineNumber = 0;
     let currentRangeIndex = 0;
     const range = ranges[currentRangeIndex];
-    if (typeof range !== 'number') {
+    if (range === undefined) {
       // We should never reach this point, but if we do, we should resolve the promise to prevent it from hanging.
       resolve();
       return;
@@ -175,7 +175,7 @@ function getContextLinesFromFile(path: string, ranges: ReadlineRange[], output:
         }
         currentRangeIndex++;
         const range = ranges[currentRangeIndex];
-        if (typeof range !== 'number') {
+        if (range === undefined) {
           // This should never happen as it means we have a bug in the context.
           lineReaded.close();
           lineReaded.removeAllListeners();