From e29b2bbf36f554f073d3886febb9676de7ad364c Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Fri, 8 Mar 2024 15:09:25 +0100 Subject: [PATCH 1/2] fix(react): Set `handled` value in ErrorBoundary depending on fallback (#10989) Previously, we made a change to always mark such errors as unhandled (#8914). However, as pointed out and discussed in #10985, this isn't always the best default value. I propose we set handled/unhandled depending on the presence of the `fallback` component. If users specify a fallback, I think it's safe to mark the error handled, given that users show something else instead of the errored component. --- packages/react/src/errorboundary.tsx | 4 +- packages/react/test/errorboundary.test.tsx | 50 ++++++++++++++++++++-- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/packages/react/src/errorboundary.tsx b/packages/react/src/errorboundary.tsx index a4920cb37b2b..178737a45cf4 100644 --- a/packages/react/src/errorboundary.tsx +++ b/packages/react/src/errorboundary.tsx @@ -147,7 +147,9 @@ class ErrorBoundary extends React.Component { captureContext: { contexts: { react: { componentStack: expect.any(String) } }, }, - mechanism: { handled: false }, + mechanism: { handled: true }, }); expect(mockOnError.mock.calls[0][0]).toEqual(mockCaptureException.mock.calls[0][0]); @@ -300,7 +300,7 @@ describe('ErrorBoundary', () => { captureContext: { contexts: { react: { componentStack: expect.any(String) } }, }, - mechanism: { handled: false }, + mechanism: { handled: true }, }); // Check if error.cause -> react component stack @@ -339,7 +339,7 @@ describe('ErrorBoundary', () => { captureContext: { contexts: { react: { componentStack: expect.any(String) } }, }, - mechanism: { handled: false }, + mechanism: { handled: true }, }); expect(mockOnError.mock.calls[0][0]).toEqual(mockCaptureException.mock.calls[0][0]); @@ -383,7 +383,7 @@ describe('ErrorBoundary', () => { captureContext: { contexts: { react: { componentStack: expect.any(String) } }, }, - mechanism: { handled: false }, + mechanism: { handled: true }, }); expect(mockOnError.mock.calls[0][0]).toEqual(mockCaptureException.mock.calls[0][0]); @@ -515,6 +515,48 @@ describe('ErrorBoundary', () => { expect(mockOnReset).toHaveBeenCalledTimes(1); expect(mockOnReset).toHaveBeenCalledWith(expect.any(Error), expect.any(String), expect.any(String)); }); + + it('sets `handled: true` when a fallback is provided', async () => { + render( +