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.

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

status-page: fix race condition in initialization. #94

Merged
merged 1 commit into from Jan 8, 2020

Conversation

@turboMaCk
Copy link
Member

@turboMaCk turboMaCk commented Jan 7, 2020

Due to the fact that loading state text is inserted on window onload,
this insertion can potentially happen after all other async
actions are already finished and thus replace the actual data.
This fix makes sure that requests are triggered after onload is done.

cc @grahamc

This is what happens when the async actions are resolved with timing that leads to this issue:

image

Steps to reproduce:

  1. make sure that client side cache is disabled in browser
    • common way is to set "disable caching while developer console is open" and open the console
  2. refresh page several times until you hit critical timing in which issue occures.
@turboMaCk turboMaCk force-pushed the turboMaCk:bug/status-fix-race-condition branch from 552b023 to 085287c Jan 7, 2020
@grahamc
Copy link
Member

@grahamc grahamc commented Jan 7, 2020

cc @samueldr mind taking a look-see?

@@ -1,7 +1,12 @@
window.onload = (_) => {

This comment has been minimized.

@turboMaCk

turboMaCk Jan 7, 2020
Author Member

I've removed this wildcard _ for the argument name because in JS all arguments are optional (with default to undefined) and arrity of functions is pretty much dynamic. While I personally also like this convention of denotating ignored arguments with _ the problem in JS case is that one can never trust the number of arguments so it can be missleading when someone less experienced is reading the code and making assumptions about arity based on those wildcards even though in this case the arity won't change (unless some monkeypatching gets involved).

to make matters worse there are polupar libraries like lodash and underscore that make use of _ as a namespace for functions so using _ as an argument name can cause some unfortunete hard to track shadowing of those namespaces.

Not a big deal but better safe than sorry I would say.

@turboMaCk turboMaCk force-pushed the turboMaCk:bug/status-fix-race-condition branch from 085287c to b90b5cb Jan 7, 2020
@turboMaCk
Copy link
Member Author

@turboMaCk turboMaCk commented Jan 7, 2020

indentation of init changed to 2 space to be consistent with rest of the code. This is ready for merge from my side.

Due to the fact that loading state text is inserted on window onload,
this insertion can potentially happen **after** all other async
actions are already finished and thus replace the actual data.
This fix makes sure that requests are triggered after onload is done.
@turboMaCk turboMaCk force-pushed the turboMaCk:bug/status-fix-race-condition branch from b90b5cb to 69bfb8c Jan 7, 2020
@zimbatm zimbatm merged commit ae96158 into NixOS:master Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.