Option wsEngine or EIO_WS_ENGINE env var

[kapouer]

This gives ability to dynamically choose ws implementation, either programmatically or by environment variable, the reason being the availability of the uWebSocket alternative - which will eventually pass the test suite.
That alternative cannot be a definitive replacement for ws module because it is an addon,
and users need to be able to choose between both solutions.

[ghost]

Okay then, it seems these options are Server options and that would of course explain it. Then I just wonder about eio_ws_module -> shouldn't that one be in caps? Other environment variables in the sources are in caps.

[kapouer]

Are they ? Damn

[ghost]

It seems they are only Server options. They have different options for Socket which would be the client if I understand things.

[ghost]

But you should still keep the name of EIO_SERVER_WS_MODULE since that one is global to the process. But wsModule can stay.

[ghost]

This project seems dead, they aren't accepting any PRs for months.

[kapouer]

Not dead. Crystalized :)

[ghost]

Well it's dead to me if they are just going to ignore PR's and not respond to any questions. They can call themselves "fastest" (note: superlative) just like everyone is doing these days even though this is very far from the truth. I don't care anymore.

[rauchg]

@alexhultman @kapouer I'm open to also adding a specific test that involves using uws as a dep, so that we know this option works for a realistic scenario.

[kapouer]

Test is coming

[kapouer]

@rauchg i went a little further than a specific test. Also i removed the ./node_modules/.bin/ part in npm script, since npm already looks there.

[ghost]

Isn't this the same problem again? The option itself can be 'wsEngine' but the (global to the process) should probably be named something implying it's only a server option, right? EIO_SERVER_WS_ENGINE but option wsEngine?

[kapouer]

For the record (i replied somewhere else already):
EIO_WS_ENGINE for engine.io
EIO_CLIENT_WS_ENGINE for engine.io-client

[ghost]

If you expect the test to succeed we need to depend on uws so that it downloads and installs it.

[kapouer]

Amen to that, i just committed in wrong order.

[ghost]

Is it possible to run the entire test two times, one with ws and one with uws?

[kapouer]

that's precisely what 53b6ae5 does

[kapouer]

or to be more precise, there's one test for the wsEngine option alone, and all tests are run twice.

[kapouer]

Provided you run them using npm test

[kapouer]

As an engine.io user, (used to the xxx, xxx-client convention) i'd expect env vars to be
EIO_WS_ENGINE
EIO_CLIENT_WS_ENGINE

[kapouer]

Let me fix order of commits...

[ghost]

I will need to push a new version to pass the last 3 tests, will do shortly.

[ghost]

@kapouer Should I link statically this release or should we fix the travis g++? You should be able to modify the .travis.yml yourself?

[kapouer]

I can add that modification to the PR, indeed

[ghost]

Okay I published v0.3.4 that passes every test. I did link statically in this build still.

[kapouer]

I don't think static linking was needed ? The problem here is more about travis config - i'm pretty sure the config i added in the .travis.yml file has not been taken into account (at least not the addons part).
Now we just need @rauchg to review and accept the pull request.

[ghost]

It seems you are not downloading the latest version? It's failing where it shouldn't fail if you have the latest ver.

[ghost]

This needs to be at least ^0.3.4

[pesho]

Moreover, it should specify an exact version. Not a version range.

[ghost]

I guess fix version is okay but in that case you will get many PRs from me, wanting to up the version when I fix things.

[kapouer]

Well, since it's in the devDependencies, and all tests pass, you won't have to PR that much. Only when some other update really breaks a test.

[ghost]

You need 4.4.3 or 4.4.4 Node.js - it was fixed just recently.

[kapouer]

I'd rather put just version 4 or 5 and rely on https://docs.travis-ci.com/user/languages/javascript-with-nodejs#Provided-Node.js-Versions

[kapouer]

And put a warning when testing ssl using uws and a version < 4.4.3 (is this the required minimum for ssl to work ?)

[kapouer]

Apparently other disabled tests are not writing a warning, so i just disabled when node < 4.4.3 and module is uws and test is using https.

[ghost]

@rauchg I think we are pretty happy with this. Sorry for the extra checks but those are needed because of bugs in Node.js prior to 4.4.3 and we cannot (yet) specify Node.js 4.4.3 in Travis. Thoughts?

[ghost]

@rauchg @kapouer Thanks for the collaboration :)

[palavrov]

@rauchg, @kapouer Guys, this PR breaks the ability to pack socket.io / engine.io with browserify because it is not able to detect the module name anymore. I.e. every time require with variable breaks it. I'm going to fix it in #397 to avoid creating a new PR.

[lakamsani]

Hi guys, what is the status on this and is there a user level doc? I have an app in production that is working well from a functional standpoint with socket.io(1.7.3) , socket.io-redis (4.0.0), ioredis(3.0.0-0) and node.js 7.7.1. I 'd love to get better performance form uws but need to make sure my app doesn't break and can't afford to rip out socket.io at this point. I have the same (unanswered) question as 'sgrytoyr' on this thread https://news.ycombinator.com/item?id=12905939 @alexhultman related to sending message received on server A to socket on server B . If I just set the env EIO_WS_ENGINE=uws will it work? I don't see uws when I do a find under my app's node_modules. Is there a specific socket.io node.js server version where it has the uws dependency? I 'd happily be a beta test user for this if it helps.

[ghost]

Engine.io 2.x has uws by default, you could look that up yourself. Nobody can know if it will work or not, you have to test. JavaScript technically has support for enforced encapsulation but no library is really making use of it, instead most libraries "mark variables as private" by putting an underscore infront but since this is not enforced you still can write code that depends on this "private" variable and thus breaks when you swap to uws as it has far different internals. If people would just learn to properly encapsulate their code it would be a different story.

[lakamsani]

OK. I messaged @goldfire on his tweet today about successful move of his app to uws (https://twitter.com/GoldFireStudios/status/846340097180098561). He said he did a custom implementation of messaging between servers but suggested one option could be: https://github.com/mmalecki/primus-redis-rooms. Also socket.io 1.7.3 uses engine.io 1.8.3. What version of socket.io will use engine.io 2.x?