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

fix(sync): reuse open connection #6054

Merged
merged 1 commit into from
Jul 18, 2024

Conversation

max-nextcloud
Copy link
Collaborator

📝 Summary

Do not attempt to create a new connection
if there already is one and it is not closed.

If no messages are received for 30 seconds
yjs will open a new websocket.

Since we do not close the connection anymore from the websocket polyfill we also do not need to open it.

If the network connection has gone down
creating a new connection will fail anyway.

Once it comes back we will know if the session is still valid. Then we can either continue using it or reconnect.

This is part of #6050.

🖼️ Screenshots

🏚️ Before 🏡 After
Bildschirmfoto vom 2024-07-16 11-40-43 No error message anymore

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • No tests. We'd have to wait > 30sec. for yjs to consider the websocket closed and open it again.
  • No documentation needed.

@max-nextcloud
Copy link
Collaborator Author

Chances are this will actually fix #6050 as the newly created sessions come with polling backends and the old polling backends keep running in parallel. So once the current session is closed the other polling backends will continue running and hand the session list to the sync service which still has the closed session and then fails to find it in the session list. So the currentSession ends up getting cleared.

Do not attempt to create a new connection
if there already is one and it is not closed.

If no messages are received for 30 seconds
yjs will open a new websocket.

Since we do not close the connection anymore from the websocket polyfill
we also do not need to open it.

If the network connection has gone down
creating a new connection will fail anyway.

Once it comes back we will know if the session is still valid.
Then we can either continue using it or reconnect.

This is part of #6050.

Signed-off-by: Max <max@nextcloud.com>
@max-nextcloud max-nextcloud force-pushed the fix/6050-do-not-reopen-connection branch from 64c1268 to 67cb647 Compare July 18, 2024 11:46
@juliushaertl juliushaertl added bug Something isn't working 4. to release labels Jul 18, 2024
@juliushaertl juliushaertl added this to the Nextcloud 30 milestone Jul 18, 2024
@max-nextcloud
Copy link
Collaborator Author

max-nextcloud commented Jul 18, 2024

fixed a related failing cypress test and now the REUSE compliance test is failing due to some css files. Not sure what to make of that.

update: related to a pr that commits the css since yesterday. Julius is looking into it.

@max-nextcloud
Copy link
Collaborator Author

/backport to stable29

@max-nextcloud
Copy link
Collaborator Author

/backport to stable28

@max-nextcloud
Copy link
Collaborator Author

cypress failures and compliance check are unrelated.

@max-nextcloud max-nextcloud merged commit 143f743 into main Jul 18, 2024
57 of 61 checks passed
@max-nextcloud max-nextcloud deleted the fix/6050-do-not-reopen-connection branch July 18, 2024 13:47
Copy link

backportbot bot commented Jul 18, 2024

The backport to stable29 failed. Please do this backport manually.

# Switch to the target branch and update it
git checkout stable29
git pull origin stable29

# Create the new backport branch
git checkout -b backport/6054/stable29

# Cherry pick the change from the commit sha1 of the change against the default branch
# This might cause conflicts, resolve them
git cherry-pick 67cb6478

# Push the cherry pick commit to the remote repository and open a pull request
git push origin backport/6054/stable29

Error: Failed to push branch backport/6054/stable29: fatal: could not read Username for 'https://github.com': No such device or address


Learn more about backports at https://docs.nextcloud.com/server/stable/go.php?to=developer-backports.

@juliushaertl
Copy link
Member

/backport to stable29

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release bug Something isn't working
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants