cryostatio / cryostat-web

Web front-end for Cryostat: Secure JDK Flight Recorder management for containerized JVMs
https://cryostat.io/
Other
10 stars 20 forks source link

fix(Targets): Target list fails to populate if WebSocket connection is not established #1276

Closed aali309 closed 1 month ago

aali309 commented 1 month ago

Welcome to Cryostat! 👋

Before contributing, make sure you have:

To recreate commits with GPG signature git fetch upstream && git rebase --force --gpg-sign upstream/main


Fixes: #1272

Description of the change:

see https://github.com/cryostatio/cryostat-web/issues/1272

Motivation for the change:

https://github.com/cryostatio/cryostat-web/issues/1272#issue-2338382998

How to manually test:

  1. Run CRYOSTAT_IMAGE=quay.io... sh smoketest.sh...
  2. ...
andrewazores commented 1 month ago

I think the fix for this problem should include changing this behaviour:

https://github.com/cryostatio/cryostat-web/blob/1074c00536d4623085082681975acebb4874114b/src/app/Shared/Services/Targets.service.tsx#L38

ie there are places where getSessionState() is called to prevent things from updating until the websocket is connected. The line linked above prevents the target list from loading until there is a websocket connection. Instead of changing the session state calculation, which I think already accurately reflects the actual websocket connection state, I think it might be better to search for places that call getSessionState() and see if it's really necessary to call it and check for a session.

For example here as well:

https://github.com/cryostatio/cryostat-web/blob/1074c00536d4623085082681975acebb4874114b/src/app/Shared/Services/Api.service.tsx#L85