diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 210212af1108..64dea4cfb92b 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -236,18 +236,26 @@ function makeWrappedRootLoader(origLoader: DataFunction): DataFunction { }; } - // Note: `redirect` and `catch` responses do not have bodies to extract - if (isResponse(res) && !isRedirectResponse(res) && !isCatchResponse(res)) { - const data = await extractData(res); - - if (typeof data === 'object') { - return json( - { ...data, ...traceAndBaggage }, - { headers: res.headers, statusText: res.statusText, status: res.status }, - ); - } else { - __DEBUG_BUILD__ && logger.warn('Skipping injection of trace and baggage as the response body is not an object'); + if (isResponse(res)) { + // Note: `redirect` and `catch` responses do not have bodies to extract. + // We skip injection of trace and baggage in those cases. + // For `redirect`, a valid internal redirection target will have the trace and baggage injected. + if (isRedirectResponse(res) || isCatchResponse(res)) { + __DEBUG_BUILD__ && logger.warn('Skipping injection of trace and baggage as the response does not have a body'); return res; + } else { + const data = await extractData(res); + + if (typeof data === 'object') { + return json( + { ...data, ...traceAndBaggage }, + { headers: res.headers, statusText: res.statusText, status: res.status }, + ); + } else { + __DEBUG_BUILD__ && + logger.warn('Skipping injection of trace and baggage as the response body is not an object'); + return res; + } } } diff --git a/packages/remix/test/integration/app_v1/root.tsx b/packages/remix/test/integration/app_v1/root.tsx index 1e716237343c..ee9b6ceddb78 100644 --- a/packages/remix/test/integration/app_v1/root.tsx +++ b/packages/remix/test/integration/app_v1/root.tsx @@ -34,6 +34,10 @@ export const loader: LoaderFunction = async ({ request }) => { throw redirect('/?type=plain'); case 'returnRedirect': return redirect('/?type=plain'); + case 'throwRedirectToExternal': + throw redirect('https://example.com'); + case 'returnRedirectToExternal': + return redirect('https://example.com'); default: { return {}; } diff --git a/packages/remix/test/integration/app_v2/root.tsx b/packages/remix/test/integration/app_v2/root.tsx index faf075951d69..5af1f8cc7a1a 100644 --- a/packages/remix/test/integration/app_v2/root.tsx +++ b/packages/remix/test/integration/app_v2/root.tsx @@ -34,6 +34,10 @@ export const loader: LoaderFunction = async ({ request }) => { throw redirect('/?type=plain'); case 'returnRedirect': return redirect('/?type=plain'); + case 'throwRedirectToExternal': + throw redirect('https://example.com'); + case 'returnRedirectToExternal': + return redirect('https://example.com'); default: { return {}; } diff --git a/packages/remix/test/integration/test/client/root-loader.test.ts b/packages/remix/test/integration/test/client/root-loader.test.ts index 4bb5b41a82ce..c78ceb2aa411 100644 --- a/packages/remix/test/integration/test/client/root-loader.test.ts +++ b/packages/remix/test/integration/test/client/root-loader.test.ts @@ -125,6 +125,9 @@ test('should inject `sentry-trace` and `baggage` into root loader throwing a red }) => { await page.goto('/?type=throwRedirect'); + // We should be successfully redirected to the path. + expect(page.url()).toEqual(expect.stringContaining('/?type=plain')); + const { sentryTrace, sentryBaggage } = await extractTraceAndBaggageFromMeta(page); expect(sentryTrace).toEqual(expect.any(String)); @@ -138,11 +141,14 @@ test('should inject `sentry-trace` and `baggage` into root loader throwing a red }); }); -test('should inject `sentry-trace` and `baggage` into root loader returning a redirection to a plain object', async ({ +test('should inject `sentry-trace` and `baggage` into root loader returning a redirection to valid path.', async ({ page, }) => { await page.goto('/?type=returnRedirect'); + // We should be successfully redirected to the path. + expect(page.url()).toEqual(expect.stringContaining('/?type=plain')); + const { sentryTrace, sentryBaggage } = await extractTraceAndBaggageFromMeta(page); expect(sentryTrace).toEqual(expect.any(String)); @@ -155,3 +161,27 @@ test('should inject `sentry-trace` and `baggage` into root loader returning a re sentryBaggage: sentryBaggage, }); }); + +test('should return redirect to an external path with no baggage and trace injected.', async ({ page }) => { + await page.goto('/?type=returnRedirectToExternal'); + + // We should be successfully redirected to the external path. + expect(page.url()).toEqual(expect.stringContaining('https://example.com')); + + const { sentryTrace, sentryBaggage } = await extractTraceAndBaggageFromMeta(page); + + expect(sentryTrace).toBeUndefined(); + expect(sentryBaggage).toBeUndefined(); +}); + +test('should throw redirect to an external path with no baggage and trace injected.', async ({ page }) => { + await page.goto('/?type=throwRedirectToExternal'); + + // We should be successfully redirected to the external path. + expect(page.url()).toEqual(expect.stringContaining('https://example.com')); + + const { sentryTrace, sentryBaggage } = await extractTraceAndBaggageFromMeta(page); + + expect(sentryTrace).toBeUndefined(); + expect(sentryBaggage).toBeUndefined(); +});