Skip to content
This repository has been archived by the owner on Nov 11, 2023. It is now read-only.

Avoid Error Updates on Unmounted Component #136

Closed
ajhenry opened this issue Jul 5, 2019 · 4 comments
Closed

Avoid Error Updates on Unmounted Component #136

ajhenry opened this issue Jul 5, 2019 · 4 comments
Assignees

Comments

@ajhenry
Copy link
Contributor

ajhenry commented Jul 5, 2019

Describe the bug
The Get component has an abort handler that will cancel any state updates after the component umounts. However, since the fetch call is wrapped in a try catch block, it will bypass the abort check and update state resulting in the classic Warning: Can't perform a React state update on an unmounted component

This is because the abort check is after the catch block intercepts the error thrown by fetch

restful-react/src/Get.tsx

Lines 256 to 262 in 529bdb3

const response = await fetch(request, { signal: this.signal });
const { data, responseError } = await processResponse(response);
// avoid state updates when component has been unmounted
if (this.signal.aborted) {
return;
}

I was able to fix this issue by putting the abort check in the catch block as well

https://github.com/AJHenry/restful-react/blob/8627e00f58cfc8c69d050c0d9a811717aa6e0533/src/Get.tsx#L381-L385

To Reproduce
Steps to reproduce the behavior:

  1. Mount the Get component
  2. Call an API that will result in an error such as 401, 404, etc
  3. Unmount the component before the call finishes and the error is thrown

Expected behavior
The state should not be updated when the component is unmounted, regardless of whether there is an error or not

Additional context
I would open a PR for this issue myself but I am unable to reliably reproduce the issue in a unit test (as setState on unmounted components are fairly hard to test)

@TejasQ
Copy link
Contributor

TejasQ commented Jul 6, 2019

Thanks for the issue, @ajhenry! I'd suggest opening a PR regardless and we can collaborate towards a good working test together. What do you think?

@ajhenry
Copy link
Contributor Author

ajhenry commented Jul 8, 2019

@TejasQ Sounds good, I can open a PR later today

ajhenry added a commit to ajhenry/restful-react that referenced this issue Jul 8, 2019
fabien0102 pushed a commit that referenced this issue Jul 29, 2019
@stereobooster
Copy link
Contributor

@fabien0102 @ajhenry is this resolved issue? Can we close the ticket?

@ajhenry
Copy link
Contributor Author

ajhenry commented Sep 18, 2019

@stereobooster this should be fixed as #137 was merged in awhile back

@ajhenry ajhenry closed this as completed Sep 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants