Browse Source

fix(replay): Include the origin in the Replay CurrentUrl component (#51748)

<!-- Describe your PR here. -->

---------

Co-authored-by: Gabe Villalobos <GabeVillalobos@users.noreply.github.com>
Ryan Albrecht 1 year ago
parent
commit
96f20dd982

+ 3 - 2
static/app/components/replays/replayCurrentUrl.tsx

@@ -7,16 +7,17 @@ import getCurrentUrl from 'sentry/utils/replays/getCurrentUrl';
 
 function ReplayCurrentUrl() {
   const {currentTime, replay} = useReplayContext();
+  const replayRecord = replay?.getReplay();
   const frames = replay?.getNavigationFrames();
 
   const url = useMemo(() => {
     try {
-      return getCurrentUrl(frames, currentTime);
+      return getCurrentUrl(replayRecord, frames, currentTime);
     } catch (err) {
       Sentry.captureException(err);
       return '';
     }
-  }, [frames, currentTime]);
+  }, [replayRecord, frames, currentTime]);
 
   if (!replay || !url) {
     return (

+ 7 - 7
static/app/utils/replays/getCurrentUrl.spec.tsx

@@ -32,26 +32,26 @@ describe('getCurrentUrl', () => {
   it('should return the origin of the first url from the url array if the offset is early', () => {
     const frames = [PAGELOAD_FRAME, NAV_FRAME];
     const offsetMS = 0;
-    const url = getCurrentUrl(frames, offsetMS);
+    const url = getCurrentUrl(TestStubs.ReplayRecord(), frames, offsetMS);
 
-    expect(url).toBe('/');
+    expect(url).toBe('http://localhost:3000/');
   });
 
   it('should return the first navigation url when the offset is after that', () => {
     const frames = [PAGELOAD_FRAME, NAV_FRAME];
     const offsetMS = Number(NAVIGATION_DATE) - Number(START_DATE) + 10;
-    const url = getCurrentUrl(frames, offsetMS);
+    const url = getCurrentUrl(TestStubs.ReplayRecord(), frames, offsetMS);
 
     expect(url).toBe(
-      '/report/1655300817078_https%3A%2F%2Fmaxcdn.bootstrapcdn.com%2Fbootstrap%2F3.3.7%2Fjs%2Fbootstrap.min.js'
+      'http://localhost:3000/report/1655300817078_https%3A%2F%2Fmaxcdn.bootstrapcdn.com%2Fbootstrap%2F3.3.7%2Fjs%2Fbootstrap.min.js'
     );
   });
 
-  it('should use the domain that is included in the crumb, if the crumb is a valid url', () => {
+  it('should use the domain that is included in the ReplayRecord, not the one in the frame', () => {
     const frames = [NEW_DOMAIN_FRAME];
     const offsetMS = Number(NEW_DOMAIN_DATE) - Number(START_DATE) + 10;
-    const url = getCurrentUrl(frames, offsetMS);
+    const url = getCurrentUrl(TestStubs.ReplayRecord(), frames, offsetMS);
 
-    expect(url).toBe('/report/jquery.min.js');
+    expect(url).toBe('http://localhost:3000/report/jquery.min.js');
   });
 });

+ 8 - 2
static/app/utils/replays/getCurrentUrl.tsx

@@ -6,9 +6,12 @@ import type {
   NavigationFrame,
   SpanFrame,
 } from 'sentry/utils/replays/types';
+import parseUrl from 'sentry/utils/url/parseUrl';
 import stripOrigin from 'sentry/utils/url/stripOrigin';
+import type {ReplayRecord} from 'sentry/views/replays/types';
 
 function getCurrentUrl(
+  replayRecord: undefined | ReplayRecord,
   frames: undefined | (BreadcrumbFrame | SpanFrame)[],
   currentOffsetMS: number
 ) {
@@ -21,8 +24,11 @@ function getCurrentUrl(
     return '';
   }
 
+  const initialUrl = replayRecord?.urls[0] ?? '';
+  const origin = initialUrl ? parseUrl(initialUrl)?.origin || initialUrl : '';
+
   if ('category' in mostRecentFrame && mostRecentFrame.category === 'replay.init') {
-    return stripOrigin(mostRecentFrame.message ?? '');
+    return origin + stripOrigin(mostRecentFrame.message ?? '');
   }
 
   if (
@@ -36,7 +42,7 @@ function getCurrentUrl(
   ) {
     // navigation.push will have the pathname while the other `navigate.*`
     // operations will have a full url.
-    return stripOrigin((mostRecentFrame as NavigationFrame).description);
+    return origin + stripOrigin((mostRecentFrame as NavigationFrame).description);
   }
 
   throw new Error('Unknown frame type in getCurrentUrl');

+ 11 - 0
static/app/utils/url/parseUrl.spec.tsx

@@ -0,0 +1,11 @@
+import parseUrl from 'sentry/utils/url/parseUrl';
+
+describe('parseUrl', () => {
+  it('should return a URL object when a valid string is passed', () => {
+    expect(parseUrl('https://example.com')).toStrictEqual(expect.any(URL));
+  });
+
+  it('should return undefined, not throw, when an invalid string is passed', () => {
+    expect(parseUrl('foo')).toBeUndefined();
+  });
+});

+ 7 - 0
static/app/utils/url/parseUrl.tsx

@@ -0,0 +1,7 @@
+export default function parseUrl(url: string) {
+  try {
+    return new URL(url);
+  } catch {
+    return undefined;
+  }
+}

+ 1 - 7
static/app/utils/url/stripOrigin.tsx

@@ -1,10 +1,4 @@
-function parseUrl(url: string) {
-  try {
-    return new URL(url);
-  } catch {
-    return undefined;
-  }
-}
+import parseUrl from 'sentry/utils/url/parseUrl';
 
 /**
  * Accept a url like: