Skip to content

🐛 fix loss of logs (on project close/websocket disconnect) & add wallet permission check on wallet (on project open) 🚨 #8426

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
Merged
merged 10 commits into from
Sep 30, 2025
Merged

🐛 fix loss of logs (on project close/websocket disconnect) & add wallet permission check on wallet (on project open) 🚨 #8426

merged 10 commits into from
Sep 30, 2025

Conversation

@matusdrobuliak66
Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 commented Sep 26, 2025 •
edited
Loading

What do these changes do?

  • 🐛 When another user session, or another user, closes a project, we unsubscribe from the logs queue. As a consequence, the other active sessions stop receiving logs.
  • 🐛 When a user tries to open a project they have access to, we need to ensure the project wallet permission check is performed. Until now, only one user could open a project. Since multiple users can now open a project, the permission check must also be enforced on the backend (it currently exists only on the frontend).
  • 🚨 MD - test manually after deploy

Documentation for project logs / socket io learned on a way

image
WS rooms or Rabbit MQ Action Details
Websocket rooms open project Connect to project room
on user connect Connect to projects rooms, group rooms
close project Leave project room
on user disconnect Handled automatically by socket io library
Rabbit MQ topic queue open project Subscribe to project logs
on user connect 1. Check user project resource exists
2. Subscribe to project logs with specific project topic
close project unsubscribe from project logs (if the last connection)
on user disconnect unsubscribe from project logs (if the last connection)

Related issue/s

How to test

Dev-ops

@matusdrobuliak66 matusdrobuliak66 changed the title 🐛 fix loss of logs 🐛 fix loss of logs & add check on wallet access when opening project Sep 26, 2025
@codecov
Copy link

codecov bot commented Sep 26, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 84.58%. Comparing base (15df617) to head (41e11f9).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #8426      +/-   ##
==========================================
- Coverage   87.83%   84.58%   -3.25%     
==========================================
  Files        1964      768    -1196     
  Lines       76327    35200   -41127     
  Branches     1342      175    -1167     
==========================================
- Hits        67043    29775   -37268     
+ Misses       8880     5368    -3512     
+ Partials      404       57     -347
Flag Coverage Δ
integrationtests 64.17% <85.71%> (-0.01%) ⬇️
unittests 86.94% <96.42%> (+0.43%) ⬆️
Components Coverage Δ
pkg_aws_library ∅ <ø> (∅)
pkg_celery_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 76.75% <ø> (-8.25%) ⬇️
agent ∅ <ø> (∅)
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 78.09% <ø> (-12.82%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 81.87% <ø> (-8.56%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 87.67% <96.42%> (-0.08%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15df617...41e11f9. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mergify
Copy link
Contributor

mergify bot commented Sep 26, 2025 •
edited
Loading

🧪 CI Insights

Here's what we observed from your CI run for 41e11f9.

🟢 All jobs passed!

But CI Insights is watching 👀

@matusdrobuliak66 matusdrobuliak66 marked this pull request as ready for review September 26, 2025 09:22
@matusdrobuliak66 matusdrobuliak66 self-assigned this Sep 26, 2025
@matusdrobuliak66 matusdrobuliak66 added the a:webserver webserver's codebase. Assigning the area is particularly useful for bugs label Sep 26, 2025
@matusdrobuliak66 matusdrobuliak66 added this to the Cheops milestone Sep 26, 2025
@matusdrobuliak66 matusdrobuliak66 changed the title 🐛 fix loss of logs & add check on wallet access when opening project 🐛 fix loss of logs (on project close) & add wallet permission check on wallet (on project open) Sep 26, 2025
odeimaiz
odeimaiz approved these changes Sep 26, 2025
View reviewed changes
Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

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

thanks

pcrespov
pcrespov reviewed Sep 26, 2025
View reviewed changes
Copy link
Member

@pcrespov pcrespov left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Can you provide a test that reproduces this issue?

i.e.

🐛 When another user session, or another user, closes a project, we unsubscribe from the logs queue. As a consequence, the other active sessions stop receiving logs.
🐛 When a user tries to open a project they have access to, we need to ensure the project wallet permission check is performed. Until now, only one user could open a project. Since multiple users can now open a project, the permission check must also be enforced on the backend (it currently exists only on the frontend).

sanderegg
sanderegg reviewed Sep 26, 2025
View reviewed changes
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

Please fix the missing part with user disconnection

@matusdrobuliak66
Copy link
Collaborator Author

matusdrobuliak66 commented Sep 29, 2025

Please fix the missing part with user disconnection

@sanderegg I have updated the solution with checking the actual socket connection (please double check it when you are back)

@matusdrobuliak66 matusdrobuliak66 changed the title 🐛 fix loss of logs (on project close) & add wallet permission check on wallet (on project open) 🐛 fix loss of logs (on project close/websocket disconnect) & add wallet permission check on wallet (on project open) Sep 29, 2025
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 30, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@matusdrobuliak66 matusdrobuliak66 merged commit 25f2302 into ITISFoundation:master Sep 30, 2025
92 of 95 checks passed
@matusdrobuliak66 matusdrobuliak66 changed the title 🐛 fix loss of logs (on project close/websocket disconnect) & add wallet permission check on wallet (on project open) 🐛 fix loss of logs (on project close/websocket disconnect) & add wallet permission check on wallet (on project open) 🚨 Sep 30, 2025
This was referenced Oct 28, 2025
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pcrespov pcrespov pcrespov left review comments

@sanderegg sanderegg sanderegg left review comments

@giancarloromeo giancarloromeo giancarloromeo approved these changes

@GitHK GitHK GitHK approved these changes

@odeimaiz odeimaiz odeimaiz approved these changes

@bisgaard-itis bisgaard-itis Awaiting requested review from bisgaard-itis

Assignees

@matusdrobuliak66 matusdrobuliak66

Labels

a:webserver webserver's codebase. Assigning the area is particularly useful for bugs

Projects

None yet

Milestone

Cheops

Development

Successfully merging this pull request may close these issues.

6 participants

@matusdrobuliak66 @giancarloromeo @GitHK @pcrespov @odeimaiz @sanderegg