Skip to content

feature/2 error when using same client id#4

Merged
svnoak merged 2 commits intosvnoak:mainfrom
Joxtacy:feature/2-error-when-using-same-client-id
Jul 7, 2025
Merged

feature/2 error when using same client id#4
svnoak merged 2 commits intosvnoak:mainfrom
Joxtacy:feature/2-error-when-using-same-client-id

Conversation

@Joxtacy
Copy link
Contributor

@Joxtacy Joxtacy commented Jul 6, 2025

  • feat(server): can't use already existing client id
  • style: run cargo fmt

Closes #2

@svnoak
Copy link
Owner

svnoak commented Jul 6, 2025

@Joxtacy I've created a separate PR (#5) and cherry picked the commit with the style formatting changes.
Please rebase this branch to have it reflect the changes in main.

@Joxtacy Joxtacy force-pushed the feature/2-error-when-using-same-client-id branch from 4ba45a7 to 716a455 Compare July 6, 2025 21:16
Copy link
Owner

@svnoak svnoak left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
I think it would be best to move the check of existing active websockets and the possible rejection for the given client_id to the ws_handler, before upgrading the connection. That way we wouldn't open and close unnecessary websocket connections for rejected client_ids.

@Joxtacy
Copy link
Contributor Author

Joxtacy commented Jul 7, 2025

Thanks for the PR! I think it would be best to move the check of existing active websockets and the possible rejection for the given client_id to the ws_handler, before upgrading the connection. That way we wouldn't open and close unnecessary websocket connections for rejected client_ids.

Yeah, good point. I'll fix that!

@svnoak svnoak merged commit 5ffc8af into svnoak:main Jul 7, 2025
2 checks passed
@Joxtacy Joxtacy deleted the feature/2-error-when-using-same-client-id branch July 7, 2025 14:45
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.

Overwriting existing client when registering same client id

2 participants