Skip to content

Bug: error in effect cleanup function no longer catchable #20946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
eps1lon opened this issue Mar 6, 2021 · 2 comments
Closed

Bug: error in effect cleanup function no longer catchable #20946

eps1lon opened this issue Mar 6, 2021 · 2 comments

Comments

@eps1lon
Copy link
Collaborator

eps1lon commented Mar 6, 2021

React version: 17.0.1
TL;DR: Fixed with skipUnmountedBoundaries = true

Steps To Reproduce

  1. Throw in an effect cleanup
  2. Try to catch it when wrapped in act()

Link to code example:
React 16
React 17

I also tested it locally and jest and couldn't catch the error as well.

The current behavior

The error is uncatchable and does not include the component stack ("The above error occurred").

The expected behavior

The error is catchable and does include the component stack i.e. React 16 behavior.

Context

I understand that the cleanup is now async but it's still flushed in act() so I would expect that an error thrown is catchable.

Originally reported in testing-library/react-testing-library#874

@eps1lon eps1lon added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Mar 6, 2021
@eps1lon eps1lon added Component: Reconciler Type: Bug and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Mar 7, 2021
@eps1lon
Copy link
Collaborator Author

eps1lon commented Mar 7, 2021

Added some tests to ReactTestUtilsAct and realized this already covered by skipUnmountedBoundaries

Tests for react-dom/test-utils are added in master...eps1lon:fix/act-cleanup-propagation but the same behavior is already covered in the noop renderer

@eps1lon
Copy link
Collaborator Author

eps1lon commented Feb 23, 2022

Fixed with #23322
Confirmation (notice the caught error during unmount): https://codesandbox.io/s/react-18-throw-in-effect-destory-forked-ce0j2u?file=/src/index.js

@eps1lon eps1lon closed this as completed Feb 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant