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

2.x and 1.x compatibility #3007

Closed
niallel opened this issue Jul 15, 2017 · 18 comments

Comments

Projects
None yet
6 participants
@niallel
Copy link

commented Jul 15, 2017

I know its not recommended, but it seems to work for the situation I'm in.

I'm using it with Cordova in an app, so I can't just upgrade the server as many clients will be on 1.x.
However I did a test with the client on 2.0.3 and the server on 1.7.4 and all worked.

Are there examples of what would work and what wouldn't. For example if I'm just using very basic ascii characters will it all work?

I've spent the day testing with my apps setup.
Everything has worked, 1.4.5 <--> 2.0.3 (client and server both ways).

When I read the notes for the utf encoding problem I see that it only mentions if you are using polling.
So I imagine if you are using web sockets then it works fine (seems to).
Is this correct, or am I missing something that could break it?

With the other issue #1058, it mentions if you are using the non-default namespace.
So again I imagine if you are using the default namespace it works fine (again seems to).
Is this correct as well?

Would someone be able to confirm if there are any other reasons why there would be problems.

thanks,

@IlyaDiallo

This comment has been minimized.

Copy link

commented Aug 20, 2017

+1

@jwalton

This comment has been minimized.

Copy link

commented Oct 14, 2017

Is there no way the 2.x client/server can detect they are connected to a 1.x server/client and use the old protocol? I'm in this same boat; I have thousands of clients connecting to my socket.io server, and they're all 1.x, so there's no easy way for me to upgrade to 2.x. I could perhaps spin up a second socket.io server on a different hostname and have my old clients connect to the old one and my new clients connect to my new one, but that sounds like a lot of work for something that should be handled by the library. :P

@niallel

This comment has been minimized.

Copy link
Author

commented Oct 14, 2017

It does worry me that no one cares to comment. I realise that people might think there is some sort of liability associated, but a "sounds right, but can't commit" would be good.

In my case I went ahead and there were no problems, so I would imagine the only way to find out is to test it yourself. In my case everything worked fine, but it would make me sleep better at night if someone dared to answer my original question.

@jwalton

This comment has been minimized.

Copy link

commented Oct 14, 2017

@IlyaDiallo

This comment has been minimized.

Copy link

commented Oct 14, 2017

Hi guys,
Same problem here. I've posted a question on SO, no answers (except my self-answer after testing).
I find it very strange that the socket.io devs are so careless about breaking compatibility and even documenting exactly what's broken or not, and how to cope. A lot of people are using that lib, right ?

@darrachequesne

This comment has been minimized.

Copy link
Member

commented Oct 16, 2017

@IlyaDiallo please see the release note: https://github.com/socketio/socket.io/releases/tag/2.0.0

The release note contains the following breaking changes:

As described in the pull request, strings were utf8-encoded twice when using polling transport, which caused issues with other client implementations. Basically, your application is concerned if you use strings where utf8.encode(utf8.encode(s)) != utf8.encode(s).

So, unless you only use websocket transport, or basic ASCII characters, that's an incompatible change.

Example:

let client = io();
let adminClient = io('/admin');

client.on('connect', () => console.log(this.id)); // 6em3d4TJP8Et9EMNAAAA
adminClient.on('connect', () => console.log(this.id)); // /admin#6em3d4TJP8Et9EMNAAAA
// 1.x
adminClient.on('connect', () => console.log(this.id)); // 6em3d4TJP8Et9EMNAAAA
@niallel

This comment has been minimized.

Copy link
Author

commented Oct 16, 2017

Thats great, confirms what I had tested.

@niallel niallel closed this Oct 16, 2017

@IlyaDiallo

This comment has been minimized.

Copy link

commented Oct 16, 2017

@darrachequesne I've read the release notes. They're not giving explicitly the informations that you are giving here, i.e. it only (partly) breaks polling transport, so you can mix 1.x and 2.0 versions with websocket transport (which is the protocol used in the vast majority of cases). I'd say that's important enough to be mentioned in the release notes.
For the #1058 issue, just to be sure, the question was:

So again I imagine if you are using the default namespace it works fine (again seems to).
Is this correct as well?

So that's correct, it only affects non-default namespaces ? And in that case, what are the symptoms ?

@jwalton

This comment has been minimized.

Copy link

commented Oct 16, 2017

@IlyaDiallo It sounds like the namespace is now incorporated into the client socket's id. So if you don't rely on that id, it won't affect you.

@IlyaDiallo

This comment has been minimized.

Copy link

commented Oct 16, 2017

@jwalton a you sure ? If it's only that (i.e. id is not used internally), that seems like a pretty weak incompatibility. More like "compatible with caveat" than a blunt "not compatible" statement.

@jwalton

This comment has been minimized.

@IlyaDiallo

This comment has been minimized.

Copy link

commented Oct 16, 2017

@jwalton I've seen the PR but I'm not familiar with the internals of socket.io, so I can't easily translate it to a yes/no response to the question "is the change affecting socket.io internally or is it only affecting users relying on the id being equal or not". And NB it's also related to #959 #2405 and #2509.
My point is that the exact compatibility issues could be much more easily and reliably documented by the people making the change than by others unfamiliar with the affected code.

@jwalton

This comment has been minimized.

Copy link

commented Oct 16, 2017

I too am unfamiliar with the code, so take my assertions with an appropriate grain of salt. :) But AFAIK, yes, it's just that the ID changed. The half of it with the polling transport is much more worrisome.

@IlyaDiallo

This comment has been minimized.

Copy link

commented Oct 16, 2017

The half of it with the polling transport is much more worrisome.

Depends. The default behavior of the client is to switch to websocket if available, and it should be available in nearly all cases. Maybe you could check that server side by tracking the clients that are actually using polling ?

@darrachequesne

This comment has been minimized.

Copy link
Member

commented Oct 17, 2017

@IlyaDiallo fair enough, the documentation could be improved!

So that's correct, it only affects non-default namespaces ? And in that case, what are the symptoms ?

Yes, only non-default namespaces, for which the socket id is now <namespace>#<connection id> (it was only <connection id> previously), in order to match the id on the server-side. There may be an impact if socket id are used for private messaging for example (socket.to(<other id>).emit()).

@IlyaDiallo

This comment has been minimized.

Copy link

commented Oct 17, 2017

@darrachequesne thanks for the confirmation. So that incompatibility only affects the (small ?) subset of users using non-default namespaces + using connection id in their code to send messages. If that was clarified in the release notes it would help a lot of people I think.
Same thing for the other issue (double-encoding), but it's dicier because the client behavior might vary (for instance a proxy blocking websocket protocol). Beside documenting the issue, the best would be as @jwalton suggested that the server detects 1.x client and double-encodes for them, I don't know if it's feasible...?

@azachar

This comment has been minimized.

Copy link

commented Jan 27, 2018

Hi there, is there any response to @IlyaDiallo question? Seem like very rational backward compatibility solution especially when there is no an easy way to update all clients.

LeSuisse added a commit to Enalean/tuleap that referenced this issue Aug 18, 2018

request #12149: Bump Kanban dependency to socket.io-client ^2
The realtime is expected to continue to work as before.

While being a major version upgrade to a breaking change [0][1],
not a lot of trouble is expected from this contribution.
Even if our server still uses socket.io ^1, the TestManagement plugin
is using socket.io-client ^2 since a while without particular issues.

[0] https://github.com/socketio/socket.io/releases/tag/2.0.0
[1] socketio/socket.io#3007 (comment)

Change-Id: Ic7f3fd4f9acacfc3fd5ddd48c82523f73f6495c6

muxator added a commit to ether/etherpad-lite that referenced this issue Aug 18, 2018

dependencies: update socket.io 1.7.3 -> 2.1.1
Version 2.x is not backwards compatible with 1.x.
However, according to [0], [1] and [2], it seems that the biggest concern is
when mixing different server and client versions, and this is not Etherpad's
case.

Smoke tested (successfully) on Firefox 61, Chromium 68.

npm audit before this change:
  found 12 vulnerabilities (9 low, 3 high) in 8205 scanned packages
    11 vulnerabilities require semver-major dependency updates.
    1 vulnerability requires manual review. See the full report for details.

npm audit after this change:
  found 1 low severity vulnerability in 8196 scanned packages
    1 vulnerability requires manual review. See the full report for details.

Fixes #3462

[0] https://socket.io/blog/socket-io-2-0-0/
[1] socketio/socket.io#3007 (comment)
[2] Enalean/tuleap@a0d7a79
@muayyad-alsadi

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.