Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[SIEM] Do not update state component when they did unmount #45847

Merged
merged 5 commits into from
Sep 17, 2019
Merged

[SIEM] Do not update state component when they did unmount #45847

merged 5 commits into from
Sep 17, 2019

Conversation

XavierM
Copy link
Contributor

@XavierM XavierM commented Sep 16, 2019

Summary

We had an issue that we were still updating our async component if they did unmount because of our async call.

How to reproduce the bug:

  1. Click on the SIEM app icon to force a full page refresh and start with clean application state
  2. Click the Hosts tab, and wait for all the widgets on the page to load
  3. Click on the Network tab, but do NOT wait for all the widgets to load before performing the next step
  4. While widgets on the Network tab are still loading, click on the Hosts tab
    Expected result
  • No errors logged in the development Console
    Actual result
  • The following error is logged to the Console:
    Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in a useEffect cleanup function.
    in Unknown (created by NetworkFilterComponent)
    in NetworkFilterComponent (created by Connect(NetworkFilterComponent))
    in Connect(NetworkFilterComponent)
    in Unknown (created by Connect(Component))
    in Connect(Component) (created by Query)
    in div (created by Container)
    in Container (created by Query)
    in Query (created by WithSource)
    in WithSource
    in Unknown (created by Connect(NetworkComponent))
    in Connect(NetworkComponent) (created by Route)
    in Route
    in Switch
    in Unknown (created by Route)

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@XavierM XavierM added Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.4.0 v7.5.0 labels Sep 16, 2019
@XavierM XavierM self-assigned this Sep 16, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem

spong
spong reviewed Sep 16, 2019
View reviewed changes
x-pack/legacy/plugins/siem/public/containers/hosts/first_last_seen/index.ts
updateLastSeen(null);
updateErrorMessage(err.toString());
}
const abortCtrl = new AbortController();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did not know about this -- very cool!

Copy link
Contributor

@FrankHassanabad FrankHassanabad Sep 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, did not know about this.

That's pretty neat and fetch takes signals too and does cancels.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it's used throughout the kibana code base so it looks to be polyfilled and a great thing to use.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

FrankHassanabad
FrankHassanabad approved these changes Sep 17, 2019
View reviewed changes
Copy link
Contributor

@FrankHassanabad FrankHassanabad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome changes and thanks for the Abort syntax below to use for fetches!

Love it ❤️ ❤️ !

LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@XavierM XavierM merged commit 677f662 into elastic:master Sep 17, 2019
XavierM added a commit to XavierM/kibana that referenced this pull request Sep 17, 2019
@XavierM
[SIEM] Do not update state component when they did unmount (elastic#4…
802f327 
…5847)

* do not update state component when they did unmount

* Adds signals to the fetches

* review I
@XavierM XavierM mentioned this pull request Sep 17, 2019
Merged
XavierM added a commit to XavierM/kibana that referenced this pull request Sep 17, 2019
@XavierM
[SIEM] Do not update state component when they did unmount (elastic#4…
5130221 
…5847)

* do not update state component when they did unmount

* Adds signals to the fetches

* review I
@XavierM XavierM mentioned this pull request Sep 17, 2019
Merged
XavierM added a commit that referenced this pull request Sep 17, 2019
@XavierM
[SIEM] Do not update state component when they did unmount (#45847) (#…
457dd1e 
…45860)

* do not update state component when they did unmount

* Adds signals to the fetches

* review I
XavierM added a commit that referenced this pull request Sep 17, 2019
@XavierM
[SIEM] Do not update state component when they did unmount (#45847) (#…
af150a1 
…45861)

* do not update state component when they did unmount

* Adds signals to the fetches

* review I
rylnd added a commit to rylnd/kibana that referenced this pull request Sep 17, 2019
@rylnd
Merge branch 'master' into new-search-layout
f27d4c8 
* master: (33 commits)
  [easy] Exclude __examples__ from coverage (elastic#45556)
  [DOCS] Update CCR links (elastic#44012)
  Use unique junit report filenames again (elastic#45897)
  [ftr/savedObjects] add simple saved object api client to ftr s… (elastic#45856)
  New visualization editor Lens (elastic#36437)
  Sort using unix timestamp value (elastic#43162)
  [APM] Use POST instead of implicit GET (elastic#45903)
  [Canvas] Converting workpad header components to typescript and adding i18n (elastic#45274)
  skip flaky test (elastic#45884)
  set IS_PIPELINE_JOB in intake jobs (elastic#45850)
  [Uptime] Fix/issue 48 integration popup closes after refresh (elastic#45759)
  [Logs UI] Support zoom by brushing in the log rate chart (elastic#45879)
  [DOCS] Changes name to host (elastic#45798)
  [ML] Add population job wizard test (elastic#45765)
  [skip-ci][Maps][File upload] Geojson indexing and styling docs (elastic#41394)
  remove setTimeoue for state change (elastic#45853)
  [Graph] Restructure folders and add readme (elastic#45782)
  [ML] Enhance job id error message (elastic#45349)
  [SIEM] Do not update state component when they did unmount (elastic#45847)
  [i18n] sync from 7.4 latest translations (elastic#45823)
  ...
@XavierM XavierM deleted the siem-fix-state-update branch June 4, 2020 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spong spong spong left review comments

@FrankHassanabad FrankHassanabad FrankHassanabad approved these changes

Assignees

@XavierM XavierM

Labels
release_note:skip Skip the PR/issue when compiling release notes Team:SIEM v7.4.0 v7.5.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@XavierM @elasticmachine @FrankHassanabad @spong