jupyterhub / configurable-http-proxy

node-http-proxy plus a REST API
BSD 3-Clause "New" or "Revised" License
242 stars 130 forks source link

Update to ws >= 8 #385

Closed guimard closed 2 years ago

guimard commented 2 years ago

Hi,

for the record, here is a fix to update to ws >= 8 (should probably be improved):

--- a/lib/testutil.js
+++ b/lib/testutil.js
@@ -3,7 +3,7 @@
 var http = require("http");
 var URL = require("url");
 var extend = require("util")._extend;
-var WebSocketServer = require("ws").Server;
+var WebSocketServer = require("ws").WebSocketServer;
 var querystring = require("querystring");

 var configproxy = require("./configproxy");
--- a/test/proxy_spec.js
+++ b/test/proxy_spec.js
@@ -62,13 +62,16 @@
     var nmsgs = 0;
     ws.on("message", function (msg) {
       if (nmsgs === 0) {
-        expect(msg).toEqual("connected");
+        expect(msg.toString()).toEqual("connected");
       } else {
         msg = JSON.parse(msg);
         expect(msg).toEqual(
           jasmine.objectContaining({
             path: "/",
-            message: "hi",
+            message: {
+              type: 'Buffer',
+              data: [ 104, 105 ]
+            }
           })
         );
         // check last_activity was updated

Cheers, Yadd

consideRatio commented 2 years ago

@guimard thank you this is great! Thank you!

consideRatio commented 2 years ago

From https://github.com/jupyterhub/configurable-http-proxy/pull/361 this seems already resolved, but I wonder if maybe it should be a WebSocketServer instead of Server as in your diff suggestion.

Closing this issue, but following up the comment above in #403.