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

Prevent stray polling interval from running and prevent duplicate event listeners #5247

Merged
merged 1 commit into from
Mar 17, 2022

Conversation

jr180180
Copy link
Contributor

Related to: #5230

Bug 1: Multiple event listeners are added to elements

When adding event listeners to elements, Javascript will generally prevent duplicating the listener if the callback function is named. Anonymous functions aren't tracked the same way. Javascript will continue to add an event listener every time the polling happens. This commit names those functions where the listeners are added to elements outside of the "page" div.

The duplicate event listeners caused some odd behavior:

  1. Clicking in the "Stop Polling" caused a single GET call to the server for each event listener on the element. This will be problematic if someone leaves the polling on for an extended period of time.
  2. It would some times cause the "Stop Polling" functionality to not stop polling.

How to duplicate:

  1. Open the page inspector from the developer tools of your browser.
  2. Click on the "event" button next to the "Live Poll" button in the inspector. You'll see that there's only one event listener.
  3. After the page does a poll, click on it again and you'll now see two event listeners with the same function.
  4. Click on the stop polling button
  5. Look at your server logs and you'll see a single request per event listener

Bug 2: Ghost interval continues polling after clicking "Stop Polling"

Every time the addListeners function was called, it would call the scheduleLivePoll() function and setup a new setTimeout. Thus, when you clicked on the "Stop Polling" button, it would continue to poll because of a stray setTimeout. The only way to stop the polling was to refresh. This commit prevents the function from running if a timeout already exists.

How to duplicate:

  1. All the UI to poll for 30 seconds
  2. Clear your server logs
  3. Click on "Stop Polling"
  4. Watch your server logs and you'll see the same polling request continue to come in every five or so seconds.

Note: Above bugs and fixes experienced and tested on Firefox 98.0.1, respectively.

…nt listeners

Related to: sidekiq#5230

### Bug sidekiq#1: Multiple event listeners are added to elements

When adding event listeners to elements, Javascript will generally prevent duplicating the listener if the callback function is named. Anonymous functions aren't tracked the same way. Javascript will continue to add an event listener every time the polling happens. This commit names those functions where the listeners are added to elements outside of the "page" `div`.

The duplicate event listeners caused some odd behavior:

1. Clicking in the "Stop Polling" caused a single GET call to the server for each event listener on the element. This will be problematic if someone leaves the polling on for an extended period of time.
2. It would some times cause the "Stop Polling" functionality to not stop polling.

#### How to duplicate:

1. Open the page inspector from the developer tools of your browser.
2. Click on the "event" button next to the "Live Poll" button in the inspector. You'll see that there's only one event listener.
3. After the page does a poll, click on it again and you'll now see two event listeners with the same function.
4. Click on the stop polling button
5. Look at your server logs and you'll see a single request per event listener

### Bug sidekiq#2: Ghost interval continues polling after clicking "Stop Polling"

Every time the `addListeners` function was called, it would call the `scheduleLivePoll()` function and setup a new `setTimeout`. Thus, when you clicked on the "Stop Polling" button, it would continue to poll because of a stray `setTimeout`. The only way to stop the polling was to refresh. This commit prevents the function from running if a timeout already exists.

#### How to duplicate:

1. All the UI to poll for 30 seconds
2. Clear your server logs
3. Click on "Stop Polling"
4. Watch your server logs and you'll see the same polling request continue to come in every five or so seconds.
@mperham
Copy link
Collaborator

mperham commented Mar 17, 2022

John, amazing PR. Well done and thank you.

@mperham mperham merged commit a8ed20a into sidekiq:main Mar 17, 2022
@jr180180 jr180180 deleted the web-ui-listener-fixes branch March 18, 2022 01:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants