From d63bee5f57c64bc162e0098d866ebecac08e75e5 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 29 Aug 2023 13:06:43 +0200 Subject: [PATCH 1/5] fix(remix): Mark errors caught from Remix instrumentation as unhandled --- packages/remix/src/client/errors.tsx | 2 +- packages/remix/src/utils/instrumentServer.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/remix/src/client/errors.tsx b/packages/remix/src/client/errors.tsx index 9c9fd5c4b449..ec2bf44a79e1 100644 --- a/packages/remix/src/client/errors.tsx +++ b/packages/remix/src/client/errors.tsx @@ -43,7 +43,7 @@ export function captureRemixErrorBoundaryError(error: unknown): void { scope.addEventProcessor(event => { addExceptionMechanism(event, { type: 'instrument', - handled: true, + handled: false, data: eventData, }); return event; diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts index 6fc326aab104..780b5e9df121 100644 --- a/packages/remix/src/utils/instrumentServer.ts +++ b/packages/remix/src/utils/instrumentServer.ts @@ -104,7 +104,7 @@ export async function captureRemixServerException(err: unknown, name: string, re scope.addEventProcessor(event => { addExceptionMechanism(event, { type: 'instrument', - handled: true, + handled: false, data: { function: name, }, From 2a1c3c5505decd72a34d63cca8df492cde609dfb Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 29 Aug 2023 14:22:45 +0200 Subject: [PATCH 2/5] adjust tests --- packages/remix/test/integration/test/server/loader.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/remix/test/integration/test/server/loader.test.ts b/packages/remix/test/integration/test/server/loader.test.ts index ccaa93b05e36..c3be005f0c1b 100644 --- a/packages/remix/test/integration/test/server/loader.test.ts +++ b/packages/remix/test/integration/test/server/loader.test.ts @@ -41,7 +41,7 @@ describe.each(['builtin', 'express'])('Remix API Loaders with adapter = %s', ada data: { function: useV2 ? 'remix.server' : 'loader', }, - handled: true, + handled: false, type: 'instrument', }, }, @@ -140,7 +140,7 @@ describe.each(['builtin', 'express'])('Remix API Loaders with adapter = %s', ada data: { function: useV2 ? 'remix.server' : 'loader', }, - handled: true, + handled: false, type: 'instrument', }, }, From 34cd7af1c15b4856a2ee57d20c83dd1d5f92e795 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 29 Aug 2023 14:40:18 +0200 Subject: [PATCH 3/5] fix missing integration test --- .../remix/test/integration/test/client/errorboundary.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/remix/test/integration/test/client/errorboundary.test.ts b/packages/remix/test/integration/test/client/errorboundary.test.ts index 9c496e3e3040..78f02fde7076 100644 --- a/packages/remix/test/integration/test/client/errorboundary.test.ts +++ b/packages/remix/test/integration/test/client/errorboundary.test.ts @@ -11,7 +11,7 @@ test('should capture React component errors.', async ({ page }) => { const [pageloadEnvelope, errorEnvelope] = envelopes; - expect(pageloadEnvelope.contexts?.trace.op).toBe('pageload'); + expect(pageloadEnvelope.contexts?.trace?.op).toBe('pageload'); expect(pageloadEnvelope.tags?.['routing.instrumentation']).toBe('remix-router'); expect(pageloadEnvelope.type).toBe('transaction'); expect(pageloadEnvelope.transaction).toBe( @@ -34,7 +34,7 @@ test('should capture React component errors.', async ({ page }) => { type: 'Error', value: 'Sentry React Component Error', stacktrace: { frames: expect.any(Array) }, - mechanism: { type: useV2 ? 'instrument' : 'generic', handled: true }, + mechanism: { type: useV2 ? 'instrument' : 'generic', handled: false }, }, ]); }); From 993b1e54a80d7ade7eab74cecf73930e940df333 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Tue, 29 Aug 2023 15:45:50 +0200 Subject: [PATCH 4/5] moar test fixes --- .../integration/test/client/errorboundary.test.ts | 2 +- .../test/integration/test/server/action.test.ts | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/remix/test/integration/test/client/errorboundary.test.ts b/packages/remix/test/integration/test/client/errorboundary.test.ts index 78f02fde7076..a03ab36b8b9f 100644 --- a/packages/remix/test/integration/test/client/errorboundary.test.ts +++ b/packages/remix/test/integration/test/client/errorboundary.test.ts @@ -34,7 +34,7 @@ test('should capture React component errors.', async ({ page }) => { type: 'Error', value: 'Sentry React Component Error', stacktrace: { frames: expect.any(Array) }, - mechanism: { type: useV2 ? 'instrument' : 'generic', handled: false }, + mechanism: { type: useV2 ? 'instrument' : 'generic', handled: true }, }, ]); }); diff --git a/packages/remix/test/integration/test/server/action.test.ts b/packages/remix/test/integration/test/server/action.test.ts index edf94f20e253..af48c99777ce 100644 --- a/packages/remix/test/integration/test/server/action.test.ts +++ b/packages/remix/test/integration/test/server/action.test.ts @@ -83,7 +83,7 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada data: { function: useV2 ? 'remix.server' : 'action', }, - handled: true, + handled: false, type: 'instrument', }, }, @@ -203,7 +203,7 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada data: { function: useV2 ? 'remix.server' : 'loader', }, - handled: true, + handled: false, type: 'instrument', }, }, @@ -256,7 +256,7 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada data: { function: useV2 ? 'ErrorResponse' : 'action', }, - handled: true, + handled: false, type: 'instrument', }, }, @@ -311,7 +311,7 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada data: { function: useV2 ? 'ErrorResponse' : 'action', }, - handled: true, + handled: false, type: 'instrument', }, }, @@ -364,7 +364,7 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada data: { function: useV2 ? 'ErrorResponse' : 'action', }, - handled: true, + handled: false, type: 'instrument', }, }, @@ -419,7 +419,7 @@ describe.each(['builtin', 'express'])('Remix API Actions with adapter = %s', ada data: { function: useV2 ? 'ErrorResponse' : 'action', }, - handled: true, + handled: false, type: 'instrument', }, }, From 5a235babdb87377abed36259f86c86c57e78dbb3 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 30 Aug 2023 14:50:24 +0200 Subject: [PATCH 5/5] fix test but this depends on the react change --- .../remix/test/integration/test/client/errorboundary.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/remix/test/integration/test/client/errorboundary.test.ts b/packages/remix/test/integration/test/client/errorboundary.test.ts index a03ab36b8b9f..56b18aa72d12 100644 --- a/packages/remix/test/integration/test/client/errorboundary.test.ts +++ b/packages/remix/test/integration/test/client/errorboundary.test.ts @@ -27,6 +27,7 @@ test('should capture React component errors.', async ({ page }) => { type: 'React ErrorBoundary Error', value: 'Sentry React Component Error', stacktrace: { frames: expect.any(Array) }, + mechanism: { type: 'chained', handled: false }, }, ] : []), @@ -34,7 +35,9 @@ test('should capture React component errors.', async ({ page }) => { type: 'Error', value: 'Sentry React Component Error', stacktrace: { frames: expect.any(Array) }, - mechanism: { type: useV2 ? 'instrument' : 'generic', handled: true }, + // In v2 this error will be marked unhandled, in v1 its handled because of LinkedErrors + // This should be fine though because the error boundary's error is marked unhandled + mechanism: { type: useV2 ? 'instrument' : 'generic', handled: !useV2 }, }, ]); });