feat(browser): Add ContextLines integration for html-embedded JS st…

…ack frames (#8670) Adds a new browser integration - `ContextLines`. It can be used to add source code lines to and around stack frames that point towards JS in html files (e.g. in `<script>` tags or `onclick` handlers). The integration **does not** apply these context lines to frames pointing to actual script files as these cannot be accessed within the browser.
getsentry · Jul 31, 2023 · ab3515c · ab3515c
1 parent e7afa27
commit ab3515c
Show file tree

Hide file tree

Showing 7 changed files with 326 additions and 0 deletions.
diff --git a/packages/browser-integration-tests/suites/integrations/ContextLines/init.js b/packages/browser-integration-tests/suites/integrations/ContextLines/init.js
@@ -0,0 +1,9 @@
+import * as Sentry from '@sentry/browser';
+import { ContextLines } from '@sentry/integrations';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  integrations: [new ContextLines()],
+});
diff --git a/packages/browser-integration-tests/suites/integrations/ContextLines/subject.js b/packages/browser-integration-tests/suites/integrations/ContextLines/subject.js
@@ -0,0 +1,3 @@
+document.getElementById('script-error-btn').addEventListener('click', () => {
+  throw new Error('Error without context lines');
+});
diff --git a/packages/browser-integration-tests/suites/integrations/ContextLines/template.html b/packages/browser-integration-tests/suites/integrations/ContextLines/template.html
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+  </head>
+  <body>
+      <button id="inline-error-btn" onclick="throw new Error('Error with context lines')">Click me</button>
+      <button id="script-error-btn">Click me too</button>
+  </body>
+  <footer>
+    Some text...
+  </foot>
+</html>
diff --git a/packages/browser-integration-tests/suites/integrations/ContextLines/test.ts b/packages/browser-integration-tests/suites/integrations/ContextLines/test.ts
@@ -0,0 +1,70 @@
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../utils/fixtures';
+import { envelopeRequestParser, waitForErrorRequestOnUrl } from '../../../utils/helpers';
+
+sentryTest(
+  'should add source context lines around stack frames from errors in Html inline JS',
+  async ({ getLocalTestPath, page, browserName }) => {
+    if (browserName === 'webkit') {
+      // The error we're throwing in this test is thrown as "Script error." in Webkit.
+      // We filter "Script error." out by default in `InboundFilters`.
+      // I don't think there's much value to disable InboundFilters defaults for this test,
+      // given that most of our users won't do that either.
+      // Let's skip it instead for Webkit.
+      sentryTest.skip();
+    }
+
+    const url = await getLocalTestPath({ testDir: __dirname });
+
+    const eventReqPromise = waitForErrorRequestOnUrl(page, url);
+
+    const clickPromise = page.click('#inline-error-btn');
+
+    const [req] = await Promise.all([eventReqPromise, clickPromise]);
+
+    const eventData = envelopeRequestParser(req);
+
+    expect(eventData.exception?.values).toHaveLength(1);
+
+    const exception = eventData.exception?.values?.[0];
+
+    expect(exception).toMatchObject({
+      stacktrace: {
+        frames: [
+          {
+            pre_context: ['    <meta charset="utf-8">', '  </head>', '  <body>'],
+            context_line:
+              '      <button id="inline-error-btn" onclick="throw new Error(\'Error with context lines\')">Click me</button>',
+            post_context: [
+              '      <button id="script-error-btn">Click me too</button>',
+              expect.stringContaining('subject.bundle.js'), // this line varies in the test based on tarball/cdn bundle (+variants)
+              '  <footer>',
+            ],
+          },
+        ],
+      },
+    });
+  },
+);
+
+sentryTest('should not add source context lines to errors from script files', async ({ getLocalTestPath, page }) => {
+  const url = await getLocalTestPath({ testDir: __dirname });
+
+  const eventReqPromise = waitForErrorRequestOnUrl(page, url);
+
+  const clickPromise = page.click('#script-error-btn');
+
+  const [req] = await Promise.all([eventReqPromise, clickPromise]);
+
+  const eventData = envelopeRequestParser(req);
+
+  const exception = eventData.exception?.values?.[0];
+  const frames = exception?.stacktrace?.frames;
+  expect(frames).toHaveLength(1);
+  frames?.forEach(f => {
+    expect(f).not.toHaveProperty('pre_context');
+    expect(f).not.toHaveProperty('context_line');
+    expect(f).not.toHaveProperty('post_context');
+  });
+});
diff --git a/packages/integrations/src/contextlines.ts b/packages/integrations/src/contextlines.ts
@@ -0,0 +1,115 @@
+import type { Event, EventProcessor, Integration, StackFrame } from '@sentry/types';
+import { GLOBAL_OBJ, stripUrlQueryAndFragment } from '@sentry/utils';
+
+const WINDOW = GLOBAL_OBJ as typeof GLOBAL_OBJ & Window;
+
+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;
+}
+
+/**
+ * Collects source context lines around the lines of stackframes pointing to JS embedded in
+ * the current page's HTML.
+ *
+ * This integration DOES NOT work for stack frames pointing to JS files that are loaded by the browser.
+ * For frames pointing to files, context lines are added during ingestion and symbolication
+ * by attempting to download the JS files to the Sentry backend.
+ *
+ * Use this integration if you have inline JS code in HTML pages that can't be accessed
+ * by our backend (e.g. due to a login-protected page).
+ */
+export class ContextLines implements Integration {
+  /**
+   * @inheritDoc
+   */
+  public static id: string = 'ContextLines';
+
+  /**
+   * @inheritDoc
+   */
+  public name: string = ContextLines.id;
+
+  public constructor(private readonly _options: ContextLinesOptions = {}) {}
+
+  /**
+   * @inheritDoc
+   */
+  public setupOnce(addGlobalEventProcessor: (callback: EventProcessor) => void): void {
+    addGlobalEventProcessor(event => this.addSourceContext(event));
+  }
+
+  /** Processes an event and adds context lines */
+  public addSourceContext(event: Event): Event {
+    const doc = WINDOW.document;
+    const htmlFilename = WINDOW.location && stripUrlQueryAndFragment(WINDOW.location.href);
+    if (!doc || !htmlFilename) {
+      return event;
+    }
+
+    const exceptions = event.exception && event.exception.values;
+    if (!exceptions || !exceptions.length) {
+      return event;
+    }
+
+    const html = doc.documentElement.innerHTML;
+
+    const htmlLines = ['<!DOCTYPE html>', '<html>', ...html.split('\n'), '</html>'];
+    if (!htmlLines.length) {
+      return event;
+    }
+
+    exceptions.forEach(exception => {
+      const stacktrace = exception.stacktrace;
+      if (stacktrace && stacktrace.frames) {
+        stacktrace.frames = stacktrace.frames.map(frame =>
+          applySourceContextToFrame(frame, htmlLines, htmlFilename, this._options.frameContextLines || 7),
+        );
+      }
+    });
+
+    return event;
+  }
+}
+
+/**
+ * Only exported for testing
+ */
+export function applySourceContextToFrame(
+  frame: StackFrame,
+  htmlLines: string[],
+  htmlFilename: string,
+  contextRange: number,
+): StackFrame {
+  if (frame.filename !== htmlFilename || !frame.lineno || !htmlLines.length) {
+    return frame;
+  }
+
+  const sourroundingRange = Math.floor(contextRange / 2);
+  const contextLineIndex = frame.lineno - 1;
+  const preStartIndex = Math.max(contextLineIndex - sourroundingRange, 0);
+  const postEndIndex = Math.min(contextLineIndex + sourroundingRange, htmlLines.length - 1);
+
+  const preLines = htmlLines.slice(preStartIndex, contextLineIndex);
+  const contextLine = htmlLines[contextLineIndex];
+  const postLines = htmlLines.slice(contextLineIndex + 1, postEndIndex + 1);
+
+  if (preLines.length) {
+    frame.pre_context = preLines;
+  }
+
+  if (contextLine) {
+    frame.context_line = contextLine;
+  }
+
+  if (postLines.length) {
+    frame.post_context = postLines || undefined;
+  }
+
+  return frame;
+}
diff --git a/packages/integrations/src/index.ts b/packages/integrations/src/index.ts
@@ -9,3 +9,4 @@ export { RewriteFrames } from './rewriteframes';
 export { SessionTiming } from './sessiontiming';
 export { Transaction } from './transaction';
 export { HttpClient } from './httpclient';
+export { ContextLines } from './contextlines';
diff --git a/packages/integrations/test/contextlines.test.ts b/packages/integrations/test/contextlines.test.ts
@@ -0,0 +1,115 @@
+import type { StackFrame } from '@sentry/types';
+
+import { applySourceContextToFrame } from '../src/contextlines';
+
+const lines = ['line1', 'line2', 'line3', 'line4', 'line5', 'line6', 'line7', 'line8', 'line9'];
+describe('ContextLines', () => {
+  describe('applySourceContextToFrame', () => {
+    it.each([
+      [
+        5,
+        {
+          pre_context: ['line2', 'line3', 'line4'],
+          context_line: 'line5',
+          post_context: ['line6', 'line7', 'line8'],
+        },
+      ],
+      [
+        1,
+        {
+          context_line: 'line1',
+          post_context: ['line2', 'line3', 'line4'],
+        },
+      ],
+      [
+        2,
+        {
+          pre_context: ['line1'],
+          context_line: 'line2',
+          post_context: ['line3', 'line4', 'line5'],
+        },
+      ],
+      [
+        9,
+        {
+          pre_context: ['line6', 'line7', 'line8'],
+          context_line: 'line9',
+        },
+      ],
+      [
+        11,
+        {
+          pre_context: ['line8', 'line9'],
+        },
+      ],
+    ])(
+      'correctly applies pre, post contexts and context lines for an inline stack frame (lineno %s)',
+      (lineno, contextLines) => {
+        const frame: StackFrame = {
+          lineno,
+          filename: 'https://mydomain.com/index.html',
+        };
+
+        expect(applySourceContextToFrame(frame, lines, 'https://mydomain.com/index.html', 7)).toStrictEqual({
+          filename: 'https://mydomain.com/index.html',
+          lineno,
+          ...contextLines,
+        });
+      },
+    );
+
+    it('only applies the context line if the range is 1', () => {
+      const frame: StackFrame = {
+        lineno: 5,
+        filename: 'https://mydomain.com/index.html',
+      };
+
+      expect(applySourceContextToFrame(frame, lines, 'https://mydomain.com/index.html', 1)).toStrictEqual({
+        filename: 'https://mydomain.com/index.html',
+        lineno: 5,
+        context_line: 'line5',
+      });
+    });
+
+    it("no-ops if the frame's line number is out of bounds for the found lines", () => {
+      const frame: StackFrame = {
+        lineno: 20,
+        filename: 'https://mydomain.com/index.html',
+      };
+
+      expect(applySourceContextToFrame(frame, lines, 'https://mydomain.com/index.html', 7)).toStrictEqual(frame);
+    });
+
+    it("no-ops if the frame's filename is not the html file's name", () => {
+      const frame: StackFrame = {
+        filename: '/someScript.js',
+      };
+
+      expect(applySourceContextToFrame(frame, lines, 'https://mydomain.com/index.html', 7)).toStrictEqual(frame);
+    });
+
+    it("no-ops if the frame doesn't have a line number", () => {
+      const frame: StackFrame = {
+        filename: '/index.html',
+      };
+
+      expect(applySourceContextToFrame(frame, lines, 'https://mydomain.com/index.html', 0)).toStrictEqual(frame);
+    });
+
+    it("no-ops if the frame doesn't have a filename", () => {
+      const frame: StackFrame = {
+        lineno: 9,
+      };
+
+      expect(applySourceContextToFrame(frame, lines, 'https://mydomain.com/index.html', 0)).toStrictEqual(frame);
+    });
+
+    it('no-ops if there are no html lines available', () => {
+      const frame: StackFrame = {
+        lineno: 9,
+        filename: '/index.html',
+      };
+      expect(applySourceContextToFrame(frame, [], 'https://mydomain.com/index.html', 0)).toStrictEqual(frame);
+    });
+  });
+});