From 19d38edc0d75caffd21e3657223c146860d6b145 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 26 Jan 2024 16:01:56 +0000 Subject: [PATCH 1/8] feat(remix): Add `wrapHandleErrorWithSentry` --- .../create-remix-app-v2/app/entry.server.tsx | 13 +++++++++- packages/remix/src/index.server.ts | 7 +++++- packages/remix/src/utils/instrumentServer.ts | 24 +++++++++++++++++-- packages/remix/src/utils/vendor/types.ts | 2 +- .../test/integration/app_v2/entry.server.tsx | 6 ++++- .../test/integration/test/server/ssr.test.ts | 6 +++++ 6 files changed, 52 insertions(+), 6 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/create-remix-app-v2/app/entry.server.tsx b/dev-packages/e2e-tests/test-applications/create-remix-app-v2/app/entry.server.tsx index 0f4ea48a99e9..c49e814246a8 100644 --- a/dev-packages/e2e-tests/test-applications/create-remix-app-v2/app/entry.server.tsx +++ b/dev-packages/e2e-tests/test-applications/create-remix-app-v2/app/entry.server.tsx @@ -26,7 +26,18 @@ installGlobals(); const ABORT_DELAY = 5_000; -export const handleError = Sentry.wrapRemixHandleError; +Sentry.init({ + environment: 'qa', // dynamic sampling bias to keep transactions + dsn: process.env.E2E_TEST_DSN, + // Performance Monitoring + tracesSampleRate: 1.0, // Capture 100% of the transactions, reduce in production! +}); + +const handleErrorImpl = () => { + Sentry.setTag('remix-test-tag', 'remix-test-value'); +}; + +export const handleError = Sentry.wrapHandleErrorWithSentry(handleErrorImpl); export default function handleRequest( request: Request, diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts index 28a2c1b37e0d..adf4d463428e 100644 --- a/packages/remix/src/index.server.ts +++ b/packages/remix/src/index.server.ts @@ -103,7 +103,12 @@ export { // Keeping the `*` exports for backwards compatibility and types export * from '@sentry/node'; -export { captureRemixServerException, wrapRemixHandleError } from './utils/instrumentServer'; +export { + captureRemixServerException, + wrapRemixHandleError, + sentryHandleError, + wrapHandleErrorWithSentry, +} from './utils/instrumentServer'; export { ErrorBoundary, withErrorBoundary } from '@sentry/react'; export { withSentry } from './client/performance'; export { captureRemixErrorBoundaryError } from './client/errors'; diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 593163a96c24..0cd319fddb05 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -89,9 +89,9 @@ async function extractResponseError(response: Response): Promise { * * Should be used in `entry.server` like: * - * export const handleError = Sentry.wrapRemixHandleError + * export const handleError = Sentry.sentryHandleError */ -export function wrapRemixHandleError(err: unknown, { request }: DataFunctionArgs): void { +export function sentryHandleError(err: unknown, { request }: DataFunctionArgs): void { // We are skipping thrown responses here as they are handled by // `captureRemixServerException` at loader / action level // We don't want to capture them twice. @@ -107,6 +107,26 @@ export function wrapRemixHandleError(err: unknown, { request }: DataFunctionArgs }); } +// To be deprecated in favor of `sentryHandleError` +export const wrapRemixHandleError = sentryHandleError; + +/** + * Sentry wrapper for Remix's `handleError` function. + * Remix Docs: https://remix.run/docs/en/main/file-conventions/entry.server#handleerror + */ +export function wrapHandleErrorWithSentry( + origHandleError: (err: unknown, args: unknown) => void, +): (err: unknown, args: unknown) => void { + return function (this: unknown, err: unknown, args: unknown): void { + // This is expected to be void but just in case it changes in the future. + const res = origHandleError.call(this, err, args); + + sentryHandleError(err, args as DataFunctionArgs); + + return res; + }; +} + /** * Captures an exception happened in the Remix server. * diff --git a/packages/remix/src/utils/vendor/types.ts b/packages/remix/src/utils/vendor/types.ts index e3a3fa42fbea..19e30b1a78e1 100644 --- a/packages/remix/src/utils/vendor/types.ts +++ b/packages/remix/src/utils/vendor/types.ts @@ -61,7 +61,7 @@ export type RemixRequestState = { export type RemixRequest = Request & Record & { - agent: Agent | ((parsedURL: URL) => Agent) | undefined; + agent?: Agent | ((parsedURL: URL) => Agent) | undefined; }; export type AppLoadContext = Record & { __sentry_express_wrapped__?: boolean }; diff --git a/packages/remix/test/integration/app_v2/entry.server.tsx b/packages/remix/test/integration/app_v2/entry.server.tsx index 6d539702e955..04d5ef52f6c2 100644 --- a/packages/remix/test/integration/app_v2/entry.server.tsx +++ b/packages/remix/test/integration/app_v2/entry.server.tsx @@ -13,7 +13,11 @@ import type { EntryContext } from '@remix-run/node'; import { RemixServer } from '@remix-run/react'; import { renderToString } from 'react-dom/server'; -export const handleError = Sentry.wrapRemixHandleError; +const handleErrorImpl = () => { + Sentry.setTag('remix-test-tag', 'remix-test-value'); +}; + +export const handleError = Sentry.wrapHandleErrorWithSentry(handleErrorImpl); export default function handleRequest( request: Request, diff --git a/packages/remix/test/integration/test/server/ssr.test.ts b/packages/remix/test/integration/test/server/ssr.test.ts index 63b0fdf7d4cd..ad2ad7a87c8e 100644 --- a/packages/remix/test/integration/test/server/ssr.test.ts +++ b/packages/remix/test/integration/test/server/ssr.test.ts @@ -19,6 +19,12 @@ describe('Server Side Rendering', () => { }, }, }, + tags: useV2 + ? { + // Testing that the wrapped `handleError` correctly adds tags + 'remix-test-tag': 'remix-test-value', + } + : {}, }); assertSentryEvent(event[2], { From 7512e123fbdb2b592eda9da8f700d449de9abf3a Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 26 Jan 2024 16:36:10 +0000 Subject: [PATCH 2/8] Expect `request` inside args. --- packages/remix/src/utils/instrumentServer.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 0cd319fddb05..caf59ce1c1c0 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -115,9 +115,9 @@ export const wrapRemixHandleError = sentryHandleError; * Remix Docs: https://remix.run/docs/en/main/file-conventions/entry.server#handleerror */ export function wrapHandleErrorWithSentry( - origHandleError: (err: unknown, args: unknown) => void, -): (err: unknown, args: unknown) => void { - return function (this: unknown, err: unknown, args: unknown): void { + origHandleError: (err: unknown, args: { request: unknown }) => void, +): (err: unknown, args: { request: unknown }) => void { + return function (this: unknown, err: unknown, args: { request: unknown }): void { // This is expected to be void but just in case it changes in the future. const res = origHandleError.call(this, err, args); From 34020ab1aa69d8c4c09c097b483660b5ba888603 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 31 Jan 2024 15:35:56 +0000 Subject: [PATCH 3/8] Add deprecation flag to `wrapRemixHandleError` alias --- packages/remix/src/utils/instrumentServer.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index caf59ce1c1c0..11e5f9b64c1e 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -107,7 +107,9 @@ export function sentryHandleError(err: unknown, { request }: DataFunctionArgs): }); } -// To be deprecated in favor of `sentryHandleError` +/** + * @deprecated Use `sentryHandleError` instead. + */ export const wrapRemixHandleError = sentryHandleError; /** From 34a0ebdf307bb259ac301e353f1ae4f75f38174d Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 1 Feb 2024 11:59:11 +0000 Subject: [PATCH 4/8] Update MIGRATION.md --- MIGRATION.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/MIGRATION.md b/MIGRATION.md index b46b4327bc04..6f373cf67c78 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -1365,6 +1365,10 @@ Instead of an `transactionContext` being passed to the `tracesSampler` callback, will be removed in v8. Note that the `attributes` are only the attributes at span creation time, and some attributes may only be set later during the span lifecycle (and thus not be available during sampling). +## Deprecate `wrapRemixHandleError` in Remix SDK (since v7.100.0) + +This release deprecates `wrapRemixHandleError` in favor of using `sentryHandleError` from`@sentry/remix`. + ## Deprecate using `getClient()` to check if the SDK was initialized In v8, `getClient()` will stop returning `undefined` if `Sentry.init()` was not called. For cases where this may be used From d76e7505a1bed5afed5a607a8a41f74ad9305455 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 1 Feb 2024 12:15:22 +0000 Subject: [PATCH 5/8] Fix linter --- packages/remix/src/index.server.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/remix/src/index.server.ts b/packages/remix/src/index.server.ts index adf4d463428e..fdb9878206f6 100644 --- a/packages/remix/src/index.server.ts +++ b/packages/remix/src/index.server.ts @@ -105,6 +105,7 @@ export * from '@sentry/node'; export { captureRemixServerException, + // eslint-disable-next-line deprecation/deprecation wrapRemixHandleError, sentryHandleError, wrapHandleErrorWithSentry, From a3a15cf23f90203038ce1d974491567c8b65e76a Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 16 Feb 2024 16:36:43 +0000 Subject: [PATCH 6/8] Apply suggestions from code review Co-authored-by: Abhijeet Prasad --- MIGRATION.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MIGRATION.md b/MIGRATION.md index 6f373cf67c78..a04cce110659 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -1367,7 +1367,7 @@ only be set later during the span lifecycle (and thus not be available during sa ## Deprecate `wrapRemixHandleError` in Remix SDK (since v7.100.0) -This release deprecates `wrapRemixHandleError` in favor of using `sentryHandleError` from`@sentry/remix`. +This release deprecates `wrapRemixHandleError` in favor of using `sentryHandleError` from `@sentry/remix`. ## Deprecate using `getClient()` to check if the SDK was initialized From 8b2c16b798e8ffc01d509dd1788921ea26c103f5 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Fri, 16 Feb 2024 16:41:27 +0000 Subject: [PATCH 7/8] Add example to migration docs. --- MIGRATION.md | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index a04cce110659..504ddae193eb 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -1228,8 +1228,6 @@ export class HeaderComponent { } ``` ---- - # Deprecations in 7.x You can use the **Experimental** [@sentry/migr8](https://www.npmjs.com/package/@sentry/migr8) to automatically update @@ -1367,7 +1365,16 @@ only be set later during the span lifecycle (and thus not be available during sa ## Deprecate `wrapRemixHandleError` in Remix SDK (since v7.100.0) -This release deprecates `wrapRemixHandleError` in favor of using `sentryHandleError` from `@sentry/remix`. +This release deprecates `wrapRemixHandleError` in favor of using `sentryHandleError` from `@sentry/remix`. It can be +used as below: + +````typescript +// entry.server.ts + +export const handleError = Sentry.wrapHandleErrorWithSentry(() => { + // Custom handleError implementation +}); +``` ## Deprecate using `getClient()` to check if the SDK was initialized @@ -1465,7 +1472,7 @@ typescript: ```ts const replay = getClient().getIntegrationByName('Replay'); -``` +```` ## Deprecate `Hub` From 34f57f5a4e2b4c26808a454c6a484c3515f6ce16 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Thu, 22 Feb 2024 10:52:29 +0000 Subject: [PATCH 8/8] Fix backticks. --- MIGRATION.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MIGRATION.md b/MIGRATION.md index 504ddae193eb..be07cbdb1a5e 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -1368,7 +1368,7 @@ only be set later during the span lifecycle (and thus not be available during sa This release deprecates `wrapRemixHandleError` in favor of using `sentryHandleError` from `@sentry/remix`. It can be used as below: -````typescript +```typescript // entry.server.ts export const handleError = Sentry.wrapHandleErrorWithSentry(() => { @@ -1472,7 +1472,7 @@ typescript: ```ts const replay = getClient().getIntegrationByName('Replay'); -```` +``` ## Deprecate `Hub`