search

open-easyrtc / open-easyrtc

Open-EasyRTC - EasyRTC Free of Priologic
https://open-easyrtc-server-demo.glitch.me/
BSD 2-Clause "Simplified" License
368 stars 107 forks source link

Apparent incompatibility with newer versions of the socket.io-client library [4.1.1] #69

Open aaclayton opened 3 years ago

aaclayton commented 3 years ago

Relevant Versions

Description of Issue

open-easyrtc appears to have been written with socket.io-client version 2.4.0 in mind. The latest version of this package is 4.1.2 and it appears to no longer be compatible with the client API provided by open-easyrtc.

The easyrtc library expects to either call io.connect to establish a socket connection as part of the easyrct.connect workflow, or be provided an existing connection explicitly via easyrtc.useThisSocketConnection.

The object returned by io.connect has no json attribute - but there are 6 locations in the easyrtc.js client code which reference self.webSocket.json.emit() which procudes a TypeError as follows:

image

This error occurs regardless of whether I construct a socket connection manually or allow easyrtc to create one on my behalf using easyrtc.connect. In both cases - immediately prior to the error message self.webSocket is defined, and self.webSocket.connected === true.

Potential Solution Paths

Replacing webSocket.json.emit() with webSocket.emit() directly does not directly solve the problems I'm experiencing, as I encounter further downstream errors which I have not yet diagnosed - but it does succeed in establishing a connection with the server.

hthetiot commented 3 years ago

Thank you @aaclayton i plan to update to latest socket.io when i have time. PR are welcome.

aaclayton commented 3 years ago

Thanks @hthetiot. I looked into solving the problem myself and submitting a merge request, but the changes to the client library seemed to be more extensive than I was comfortable attempting on my own without being familiar with the code base. Hopefully it's simpler than I realized.

hthetiot commented 3 years ago

@aaclayton feel free to send a draft PR even if not finalized.

In any case i will try to address the migration to new socket.io in the coming week. This will be a breaking change if people use socket.io internally for other than easyrtc meaning it will be a major increments on version.

hthetiot commented 3 years ago

@aaclayton I did the migration of client and server to socket.io 4+ in this PR:

So fare all the samples works, if you can test and we may release in near future.

aaclayton commented 3 years ago

I will try and take a look later this week if I can, unfortunately I'm a bit overloaded with work at the moment so I hope others can also help to test in the meantime. I'll let you know once I have a chance to pull and look.

hthetiot commented 3 years ago

thx @aaclayton i tried the demos and they still works.

hthetiot commented 3 years ago

See #70 for testing socket.io 4+

hthetiot commented 3 years ago

@aaclayton can you test the PR, if all good i will release butt increment major version number so users don't get issue on upgrade.

IgorNovozhilov commented 2 years ago

@hthetiot What are the future plans for the release of this fix?

upd: In cases of my project, it works. I have installed it in my project in package.json "open-easyrtc": "github:open-easyrtc/open-easyrtc#socket.io-4"

hthetiot commented 2 years ago

@IgorNovozhilov i was waiting for feedback, i think i will increment version to 3.x to prevent breaking people that also use socket.io internally.

Will close this issue once released.

hthetiot commented 1 year ago

See PR here for testing:

Will be released under 2.1.0 i think