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

Web socket proxying does not work with Node.js v11 and above #9

Closed
TheTyrius opened this issue Jun 9, 2019 · 9 comments
Closed

Web socket proxying does not work with Node.js v11 and above #9

TheTyrius opened this issue Jun 9, 2019 · 9 comments
Labels
enhancement New feature or request question Further information is requested

Comments

@TheTyrius
Copy link

When using node version v12.4.0 I get the following error when initiating a connection:

[...]\node_modules\global-agent\dist\classes\Agent.js:32
    if (!this.isProxyConfigured()) {
              ^

TypeError: this.isProxyConfigured is not a function

Using node v10.16.0 seems to work as expected. I'm running a super minimal node script (I could share if helpful) and use global-agent via "node -r" option.

I know very little about node and this project, so please let me know if a) the newer node is supported at all b) I can supply any other information to be helpful with this issue.

Best regards,
Tyrius

@gajus
Copy link
Owner

gajus commented Jun 9, 2019

How are you using global-agent?

@gajus gajus added the question Further information is requested label Jun 9, 2019
@TheTyrius
Copy link
Author

TheTyrius commented Jun 9, 2019

I was trying to go without modifying the programm. Full command
.\node -r 'global-agent/bootstrap' main.js

I switched between node versions serveral times, and did exactly the same npm install and run above command. Older version works fine, newer gives this error.

@gajus gajus added invalid This doesn't seem right and removed question Further information is requested labels Jun 9, 2019
@gajus
Copy link
Owner

gajus commented Jun 9, 2019

$ node -v
v12.4.0
$ cat main.js
const got = require('got');

const main = async () => {
  await got('http://gajus.com');

  console.log('OK');
};

main();
$ export GLOBAL_AGENT_HTTP_PROXY=http://127.0.0.1:8020
$ node -r 'global-agent/bootstrap' main.js
OK

Cannot replicate the issue.

@gajus gajus closed this as completed Jun 9, 2019
@TheTyrius
Copy link
Author

TheTyrius commented Jun 9, 2019

I'm so sorry. For the lack of detail so far (regarding main.js).

This is the script I'm running (main.js)
#!/usr/bin/env node

/*
 * WebSocket Proxy for Fantastic Feasts.
 * Forwards WebSocket communication transparently between multiple clients and one server.
 *
 * Initial version by Flo, for Sopra 2018-19.
 */

const http = require('http');
const websocket = require('websocket');

const WsServer = websocket.server;
const WsClient = websocket.client;


// Sopra standard specifies port 4488 for servers.
// We choose a different port number to avoid collision when server and proxy are run on the same host.
const PROXY_PORT_FOR_CLIENTS = 4444;


// The Fantastic Feasts server WebSocket URL is given as first command line arg.
const serverWebSocketURL = process.argv[2];
if (!serverWebSocketURL) {
    log(`PROXY -- Error: First command line arg should be WebSocket URL of Fantastic Feasts server! Example: "ws://example.org:4488/".`)
    process.exit(1);
}

// Map client IP addresses to short names ("Client1", "Client2", ...) for readability.
let nextClientNumber = 1; // is incremented for each client
const clientAddrToNames = {}; // map from <IP:port> to name


// An HTTP server for accepting upgrade requests to establish WebSocket connections.
const httpServer = http.createServer();
httpServer.listen(PROXY_PORT_FOR_CLIENTS, () => {
    log(`PROXY -- Listening for clients on port ${PROXY_PORT_FOR_CLIENTS}. Fantastic Feasts server is: ${serverWebSocketURL}`);
});


// The actual WebSocket server for the client side of the proxy.
const wsServer = new WsServer({
    httpServer: httpServer
});

wsServer.on('request', request => {
    const connectionFromClient = request.accept();
    const clientIP = request.remoteAddress;
    const clientPort = connectionFromClient.socket.remotePort;
    const clientAddr = `IP: ${clientIP}, port: ${clientPort}`;
    clientAddrToNames[clientAddr] = 'Client' + nextClientNumber;
    ++nextClientNumber;
    log(`PROXY -- Accepted connection from ${clientAddrToNames[clientAddr]} (${clientAddr}).`);

    connectionFromClient.on('close', (reasonCode, description) => {
        log(`${clientAddrToNames[clientAddr]} -- Disconnected: Code ${reasonCode}, Description: ${description}`);
    });

    // The WebSocket client for the server side of the proxy (specific to this client).
    const wsClient = new WsClient();

    wsClient.on('connectFailed', error => {
        log(`PROXY -- Error: Could not connect to server for ${clientAddrToNames[clientAddr]}: ${error.toString()}`);
    });

    wsClient.on('connect', connectionToServer => {
        log(`PROXY -- Connected to server for ${clientAddrToNames[clientAddr]}.`);

        connectionToServer.on('error', error => {
            log(`PROXY  -- Error: Error on connection to server for ${clientAddrToNames[clientAddr]}: ${error.toString()}`);
        });

        connectionToServer.on('close', () => {
            log(`PROXY -- Connection to server closed for ${clientAddrToNames[clientAddr]}.`);
        });

        // Forward messages from server to client.
        connectionToServer.on('message', message => {
            // We expect message to be a UTF8 encoded string, containing JSON.
            if (message.type === 'utf8') {
                const messageContent = message.utf8Data;
                connectionFromClient.sendUTF(messageContent);
                log(`Server  -> ${clientAddrToNames[clientAddr]} -- Message: ${message.utf8Data}`);
            } else {
                log(`Server  -> ${clientAddrToNames[clientAddr]} -- Error: Message was binary data instead of UTF8-encoded JSON!`);
            }
        });

        // Forward messages from client to server.
        connectionFromClient.on('message', message => {
            // We expect message to be a UTF8 encoded string, containing JSON.
            if (message.type === 'utf8') {
                const messageContent = message.utf8Data;
                connectionToServer.sendUTF(messageContent);
                log(`${clientAddrToNames[clientAddr]} -> Server  -- Message: ${messageContent}`);
            } else {
                log(`${clientAddrToNames[clientAddr]} -> Server  -- Error: Message was binary data instead of UTF8-encoded JSON!`);
            }
        });
    });

    wsClient.connect(serverWebSocketURL);
});


/**
 * Logs messages to stdout, prepends each message with a timestamp.
 * @param {string} text - The log message content.
 */
function log(text) {
    let timeTag = `[${new Date().toISOString()}]`;
    console.log(`${timeTag}  ${text}`);
}
package.json
{
  "name": "ffproxy",
  "version": "0.0.1",
  "description": "WebSocket Proxy for Fantastic Feasts",
  "main": "main.js",
  "repository": {
    "type": "git",
    "url": ""
  },
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "Flo",
  "license": "UNLICENSED",
  "dependencies": {
    "websocket": "^1.0.28"
  }
}

The error appears when first connecting to the server the script creates.
I'm on Windows for this matter, and the startup script I use is

$Env:GLOBAL_AGENT_HTTP_PROXY="http://127.0.0.1:8080"
.\node -r 'global-agent/bootstrap' main.js ws://127.0.0.1:4488

I also just now reproduced on a linux system running node v11.15.0.

  1. npm install & npm install global-agent
  2. node -r 'global-agent/bootstrap' main.js ws://127.0.0.1:4488 (doesn't need to have a websocketserver actually running there)
  3. The script crashes with aforementioned error when connecting a websocket client (tested with websocat and a chrome addon that can connect to websockets)

I'm not sure if maybe the script is buggy, but the error-message made me believe it's part of global-agent :)

@gajus gajus reopened this Jun 9, 2019
@gajus gajus added question Further information is requested enhancement New feature or request and removed invalid This doesn't seem right labels Jun 9, 2019
@gajus
Copy link
Owner

gajus commented Jun 9, 2019

Do you say that this work on Node.js <v12?

I don't think the current implementation supports WebSockets.

@TheTyrius
Copy link
Author

I am trying to use global-agent to pass websocket traffic through https://github.com/nccgroup/wssip
I just switched node versions again to v10.16.0 (latest LTS) and was able to connect and send messages, which are logged in the wssip proxy that I configured using global-agent. So to me it is working as expected. The other node version (v11.15.0) that happened to be installed on my laptop is not working either.

So if you say you would expect websockets to fail that is an explanation, strange nontheless (or even stranger).

@gajus gajus changed the title Error on node v12.4.0 (working v10.16.0) Web socket proxying does not work with Node.js v12 and above Jun 21, 2019
@gajus gajus changed the title Web socket proxying does not work with Node.js v12 and above Web socket proxying does not work with Node.js v11 and above Jun 21, 2019
@gajus
Copy link
Owner

gajus commented Jun 21, 2019

Could you please create a failing test case?

With a failing test case I will be able to add the missing feature.

Web sockets is not something I use that much.

@TheTyrius
Copy link
Author

I've just been through quite a journey trying to pinpoint the issue I'm having. (TL;DR at bottom)

First I reduced above script to a most basic test-case: Creating a server and connecting to itself, sending a "test". Result: proxytest.zip

At first I was able to use this minimal script to reproduce the exact behavior I initially observed using node 10 and 12.

Fast forward about 3 hours.

I wanted to test a couple more node versions before reporting back, so I grabbed releases of node 9, 10 and 11.
9: doesn't error, but doesn't use the proxy (seeing that global agent is tested for v10 and above I didn't mind this result at all)

10: worked as expected!
11: worked as expected!

that last one surprised me, so I retested using node v12... and it worked as well.

At that stage I was very puzzled. The only difference between the two runs using v12 was that I deleted all node_modules (tested a bunch of other versions) and reinstalled everything 3h later.
I remembered that I had reproduced on my laptop. So I pull that up as I left it and rerun the script: Still error. I backup node_modules and run "npm update" -> 2 packages updated, core-js and global-agent.

Fair enough, when I started testing I was using 1.12.1, but I put ^1.12.1 into the package.json, so later I was pulling 1.12.2 (checking the commit/release log the timeline adds up!).

Using node v11.15 I retested global-agent 1.12.1 and got the error again. Switching back and forth with 1.12.2 I was able to reproduce on 11.12.1 but not on 1.12.2, so I ended my endeavors there.

TL;DR: The issue isn't present in 1.12.2 anymore, so... thanks for that :D I will keep an eye out should the error return, but I guess for now this issue is no longer needed.

Sorry that this became a bit longer, I wrote it while doing the tests and was questioning my sanity somewhere around the middle...
So I suspect because websockets work by upgrading a http connection, there is no further need to explicitly support them in global-agent, and this issue had it's cause somewhere where it's fixed by your recent additions.

I want to end by apologizing for the chaotic issue. I'm at my very first steps with a lot of the technologies involved in this current project. So thank you for your patience, and I hope you don't mind if another "noob issue" like this would be opened in the future. Have a great weekend :)

@gajus
Copy link
Owner

gajus commented Jun 22, 2019

Thank you for detail follow up.

It is probably fixed by f17386d.

I found that some packages (e.g. https://github.com/kevva/caw) sit as dependencies of other popular packages (e.g. download) and pull-in tunnel-agent behind the scenes that tries to construct a new instance of global.Agent using .constructor property. Since global-agent Agent requires custom initialization, that did not work. Now the required properties are scoped for initialization from anywhere.

@gajus gajus closed this as completed Jun 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants