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

Proxy WebSocket connections #447

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

GiviMAD
Copy link
Member

@GiviMAD GiviMAD commented Feb 13, 2024

Hello, these is a proposed PR to allow proxy WebSocket connections to the OpenHAB instances, to the endpoints exposed under path /ws/.

With the PR, the code catches the upgraded response headers and setup the required listeners on the request Socket in order to transfer the incoming data through the Socket.IO connection to the destination server using the "websocket" event. Also handles the "websocket" events from the remote server and write the received data to the socket.

I have also changed the responses like res.send(200, '...'); to be res.status(200).send('...'); because I saw deprecation warnings when running the project using docker-compose.

These PR needs changes in the Cloud Connector add-on.

Signed-off-by: Miguel Álvarez <[email protected]>
@digitaldan
Copy link
Contributor

Hi @GiviMAD just wanted to let you know I plan on reviewing this soon, possible this weekend. Thanks!

@GiviMAD
Copy link
Member Author

GiviMAD commented Feb 17, 2024

Thank you @digitaldan I was to write you the other day but I saw the message a little late. Hope everything is in place.

I have added a small change in the last commit using "process.nextTick" to write the pending socket data, because I think it will behave a little better when handling multiple clients.

* Write to socket handling backpressure
*/
function writeToSocket(stream, data) {
const pending = stream[PendingWrites] = (stream[PendingWrites] || []);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, funny, i have never come across using the Symbol type before. Is the point of this to avoid creating custom named variables on the local stream object ? So to avoid using a variable name as a unique key ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It allows to create non enumerable properties on an object (meaning it will not be returned by Object.entries and other ways to list the object keys) and unlike a custom property name it can't collision with other part of the code, because event if you create other symbol with same name is not considered to be the same symbol.

I can't remember when I learnt about it but I always found it very useful.

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.

2 participants