Bumped ws to v2.0.0. #106

mcollina · 2017-02-08T08:49:56Z

Fixes #105.

This is a semver-major change, as it will work only on Node v4+.

antoniobusrod · 2017-02-15T12:47:00Z

Don't you prefer to stay at next https://github.com/websockets/ws/releases/tag/1.1.2 ?

mcollina · 2017-02-15T12:52:53Z

At some point we will have to migrate to v2.

sublimator · 2017-02-20T05:31:25Z

https://github.com/websockets/ws/releases/tag/2.0.0

Dropped support for Node.js < 4.5.0.

Man, AWS lambda is stuck on 4.3.2, they need to get their act together.

simonmcl · 2017-03-14T14:17:15Z

Linked here from @mcollina

My 2 cents:

I understand the issue and the effort involved. While my understanding of this module is limited as I'm using one which depends on this, there is a very long stack trace appearing in the console coming from v8 directly, when using Node v6+, to say that the version of bufferutils being used, is using deprecated methods (bufferutils is being used by ws).

We faced a lot of errors and crashes in Node v7 when trying to use the module, and ultimately couldn't get it to work. I realise the drop in support is not ideal, but it may become necessary very soon.

mcollina · 2017-03-14T14:27:55Z

@simonmcl we are not using bufferutils anywhere in this module. websocket-stream works without problem on v7 on my machine.
What errors are you seeing on Node v7? Can you open specific issues on those, or maybe contribute some unit tests so that we can improve things?

simonmcl · 2017-03-14T14:36:16Z

@mcollina bufferutils is a dependency of ws, which is a dependency of your module. We didn't have time to debug the v7 issue on our current project, so we just stayed with v6.9.5, we will look to upgrade another time when we have the bandwidth, just guessing it might have been related due to the deprecation warnings.

But when trying to use https://www.npmjs.com/package/ibmiotf , on node 6.9.5 a large stacktrace appears warning of deprecations for bufferutil. They are using mqtt, which is using websocket-stream.

I planned to ask them to update their dependencies, but discovered that there was no version of websocket-stream which had the appropriate version of ws to have the appropriate version of bufferutils

... anyone else's head hurting or just me?

mcollina · 2017-03-14T14:41:12Z

@simonmcl bufferutil is not a dependency of ws v1: https://github.com/websockets/ws/blob/v1.x/package.json#L24.

So, something else should be going on in here. I think you should open an issue on ibmiotf, and we can work with them to solve the issue. They probably have more bandwidth than us anyway.
Feel free to tag me in that issue.

simonmcl · 2017-03-14T14:58:21Z

@mcollina ah its a devDependency, I just searched for bufferutil, missed what brace it was in. Ok cool, i'll check this issue again and follow up with them then. Thanks for the help

simonmcl · 2017-03-16T14:38:27Z

@mcollina had another few minutes to look at this. Turns out the versions being used by that IOT module, is using a version of ws where bufferutil is listed as an optional dependency. It was moved to a dev dependency later on. Running npm install --no-optional solves the issue for now.

Logged an issue with the IOT module to update their versions.

Thanks for the help debugging this!

sublimator · 2017-03-16T14:40:21Z

U using aws IoT too?

simonmcl · 2017-03-16T14:43:57Z

@sublimator no IBM's IOT platform on Bluemix

sublimator · 2017-03-16T14:51:50Z

I saw aws/azure use mqtt 1.x (though not sure about the latter)

sublimator · 2017-03-16T14:52:51Z

ws 2 does not work on aws ... they are still on node 4.3.2 ...

simonmcl · 2017-03-16T15:01:03Z

@sublimator 4.3.2? jesus thats horrific, my thoughts and prayers are with you at this difficult time lol

sublimator · 2017-03-16T15:01:52Z

Hahaha

lpinca · 2017-04-02T08:03:26Z

FWIW aws now (Mar 22nd, 2017) supports and recommends Node.js v6.10

sublimator · 2017-04-02T08:04:28Z

Yeah, aws 6.10!

mcollina · 2017-04-02T10:43:25Z

Still, a lot of people uses older version of v4. Can we get ws to work on v4.0.0? Lamda's node 4.3.2 is still there for a lot of people.

lpinca · 2017-04-02T10:49:07Z

Node.js v4.5 is only required due to the use of safe Buffer APIs only available is 4.5+ but we can use https://www.npmjs.com/package/safe-buffer in ws to support all v4 versions.

lpinca · 2017-04-03T10:20:20Z

stream.js

@@ -17,7 +17,7 @@ function WebSocketStream(target, protocols, options) {
  if (protocols && !Array.isArray(protocols) && 'object' === typeof protocols) {
    // accept the "options" Object as the 2nd argument
    options = protocols
-    protocols = null
+    protocols = []


This is no longer needed. It was a regression bug which has been fixed in ws@2.2.3, sorry about that.

lpinca · 2017-04-03T10:41:03Z

stream.js

@@ -67,6 +67,13 @@ function WebSocketStream(target, protocols, options) {
  var coerceToBuffer = options.binary || options.binary === undefined

  function socketWriteNode(chunk, enc, next) {
+    // avoid errors, this never happens unless
+    // destroy() is called
+    if (socket.readyState !== WS.OPEN) {


As I thought this has nothing to do ws@2, it is also needed for ws@1. The same issue also happens on master if you disable the permessage-deflate extension.

diff --git a/test.js b/test.js index 18caa28..a0b10af 100644 --- a/test.js +++ b/test.js @@ -99,7 +99,9 @@ test('drain', function(t) { t.plan(1) echo.start(function() { - var client = websocket(echo.url, echo.options) + var client = websocket(echo.url, { + perMessageDeflate: false + }) client.on('drain', function() { client.destroy()

In ws@2 this also happens with permessage-deflate enabled because data is not compressed unless the size is bigger than a configurable threshold (1024 bytes by default).

The problem with the "drain" test is that a single data chunk arrives on the server with many binary frames and a close frame. All frames are parsed on the same tick and when the close frame is processed, the readyState is set to CLOSING. The payloads of the processed binary frames are pushed to the proxy, but when socketWriteNode() is called the readyState is already CLOSING.

Thanks for the explanation.

mcollina · 2017-04-05T07:25:23Z

readme.md

+extension](https://tools.ietf.org/html/rfc7692) to achieve the best
+throughput.
+
+Default: `false`


@lpinca is this documented correctly?

Default value is true in ws.

lpinca · 2017-04-05T07:29:47Z

readme.md

+extension](https://tools.ietf.org/html/rfc7692) to achieve the best
+throughput.
+
+Default: `false`


Default value is true in ws.

lpinca · 2017-04-05T07:30:59Z

readme.md

+  fs.createReadStream('bigdata.json').pipe(stream)
+}
+```
+


It might makes sense to show how to disable it on the client (it's the same) as websocket-stream could be used only as a client.

mcollina · 2017-04-05T07:34:03Z

@lpinca is the perMessageDeflate option understood also in the browser, or is it node-specific?

mcollina · 2017-04-05T07:35:28Z

@lpinca forget about that question, we are not passing the options through.

lpinca · 2017-04-05T07:35:54Z

The are no options in the browser interface. It depends on the browser. It is automatically negotiated when supported (if the server supports it ofc).

Bumped ws to v2.0.0.

138605d

mcollina mentioned this pull request Feb 15, 2017

Upgrade ws to last stable version mqttjs/MQTT.js#543

Closed

mcollina mentioned this pull request Mar 14, 2017

Upgrade to ws v2.0.0+ to remove deprecation warnings #108

Closed

mcollina mentioned this pull request Mar 30, 2017

run aedes on websocket must use websocket-stream? moscajs/aedes#97

Closed

lpinca mentioned this pull request Apr 2, 2017

Add support for Node.js 4.1.0 - 4.4.7 websockets/ws#1059

Merged

lpinca reviewed Apr 3, 2017

View reviewed changes

mcollina added 2 commits April 5, 2017 09:18

Removed support for protocols regression.

d6dff7a

Readme update for ws v2.

372725d

mcollina commented Apr 5, 2017

View reviewed changes

lpinca reviewed Apr 5, 2017

View reviewed changes

Correct perMessageDeflate.

dc39271

mcollina mentioned this pull request Apr 5, 2017

Implements writev #112

Merged

mcollina merged commit dc39271 into master Apr 7, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bumped ws to v2.0.0. #106

Bumped ws to v2.0.0. #106

mcollina commented Feb 8, 2017

antoniobusrod commented Feb 15, 2017

mcollina commented Feb 15, 2017

sublimator commented Feb 20, 2017

simonmcl commented Mar 14, 2017

mcollina commented Mar 14, 2017

simonmcl commented Mar 14, 2017

mcollina commented Mar 14, 2017 •

edited

Loading

simonmcl commented Mar 14, 2017

simonmcl commented Mar 16, 2017

sublimator commented Mar 16, 2017 via email

simonmcl commented Mar 16, 2017

sublimator commented Mar 16, 2017 via email

sublimator commented Mar 16, 2017 via email •

edited by mcollina

Loading

simonmcl commented Mar 16, 2017

sublimator commented Mar 16, 2017 via email

lpinca commented Apr 2, 2017

sublimator commented Apr 2, 2017 via email

mcollina commented Apr 2, 2017

lpinca commented Apr 2, 2017 •

edited

Loading

lpinca Apr 3, 2017

lpinca Apr 3, 2017 •

edited

Loading

mcollina Apr 5, 2017

mcollina Apr 5, 2017

lpinca Apr 5, 2017

lpinca Apr 5, 2017

lpinca Apr 5, 2017

mcollina commented Apr 5, 2017

mcollina commented Apr 5, 2017

lpinca commented Apr 5, 2017

Bumped ws to v2.0.0. #106

Bumped ws to v2.0.0. #106

Conversation

mcollina commented Feb 8, 2017

antoniobusrod commented Feb 15, 2017

mcollina commented Feb 15, 2017

sublimator commented Feb 20, 2017

simonmcl commented Mar 14, 2017

mcollina commented Mar 14, 2017

simonmcl commented Mar 14, 2017

mcollina commented Mar 14, 2017 • edited Loading

simonmcl commented Mar 14, 2017

simonmcl commented Mar 16, 2017

sublimator commented Mar 16, 2017 via email

simonmcl commented Mar 16, 2017

sublimator commented Mar 16, 2017 via email

sublimator commented Mar 16, 2017 via email • edited by mcollina Loading

simonmcl commented Mar 16, 2017

sublimator commented Mar 16, 2017 via email

lpinca commented Apr 2, 2017

sublimator commented Apr 2, 2017 via email

mcollina commented Apr 2, 2017

lpinca commented Apr 2, 2017 • edited Loading

lpinca Apr 3, 2017

Choose a reason for hiding this comment

lpinca Apr 3, 2017 • edited Loading

Choose a reason for hiding this comment

mcollina Apr 5, 2017

Choose a reason for hiding this comment

mcollina Apr 5, 2017

Choose a reason for hiding this comment

lpinca Apr 5, 2017

Choose a reason for hiding this comment

lpinca Apr 5, 2017

Choose a reason for hiding this comment

lpinca Apr 5, 2017

Choose a reason for hiding this comment

mcollina commented Apr 5, 2017

mcollina commented Apr 5, 2017

lpinca commented Apr 5, 2017

mcollina commented Mar 14, 2017 •

edited

Loading

sublimator commented Mar 16, 2017 via email •

edited by mcollina

Loading

lpinca commented Apr 2, 2017 •

edited

Loading

lpinca Apr 3, 2017 •

edited

Loading