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

Web UI event listeners disappear after page update when live polling is enabled #5184

Closed
gillisd opened this issue Feb 5, 2022 · 6 comments · Fixed by #5230
Closed

Web UI event listeners disappear after page update when live polling is enabled #5184

gillisd opened this issue Feb 5, 2022 · 6 comments · Fixed by #5230

Comments

@gillisd
Copy link

gillisd commented Feb 5, 2022

Ruby version: 2.7.5
Rails version: 5.2.5
Sidekiq / Pro / Enterprise version(s): 6.3.1 / 5.2.0

Problem

When live poll is enabled in the web UI with an interval of n seconds, the following occur after n seconds have passed:

  1. Confirmation to delete a queue is non-existent
  2. Select all functionality is broken
  3. Search functionality in retry queue is broken

Unfortunately, due to (1), a large job queue was deleted after someone accidentally clicked the delete button next to the queue. The confirmation message never appeared, as the web UI was in "live poll" mode, and the polling interval had passed.

I don't really care about (2) or (3), as they work fine when not in "live poll" mode, but I think (1) is a pretty serious issue, especially because the delete and pause buttons are so close together.

Cause

When live polling is enabled, the following callback is triggered:

function livePollCallback() {
  clearTimeout(livePollTimer);

  fetch(window.location.href).then(resp => resp.text()).then(replacePage).finally(scheduleLivePoll)
}

replacePage is called, causing all event listeners created in ready(() => { to be wiped out, and I don't see them being reinitialized.


P.S. thank you for making this software, it has enabled a very small team able to process billions of jobs each year!

@mperham
Copy link
Collaborator

mperham commented Feb 5, 2022 via email

@gillisd
Copy link
Author

gillisd commented Feb 5, 2022

I'll try to come up with something this weekend.

@mperham
Copy link
Collaborator

mperham commented Mar 3, 2022

Ping?

@gillisd
Copy link
Author

gillisd commented Mar 4, 2022

Apologies for not circling back here - if I don't have a PR up by next week, someone else is welcome to take over or you can close it for now.

@excid3
Copy link
Contributor

excid3 commented Mar 4, 2022

I think replacePage should call the ready callback so it can setup the listeners on the new DOM elements.

Alternatively, using Stimulus would handle all this seamlessly, but it'd be a bigger change than is probably necessary.

I'll give it a try and see if that does the trick.

excid3 added a commit to excid3/sidekiq that referenced this issue Mar 4, 2022
mperham pushed a commit that referenced this issue Mar 4, 2022
* Add event listeners after reload. Fixes #5184

* Remove redundant fuzzy times update
@gillisd
Copy link
Author

gillisd commented Mar 4, 2022

Thanks and nice work @excid3!

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 a pull request may close this issue.

3 participants