Azure / azure-relay-node

☁️Node.js library for Azure Relay Hybrid Connections
https://docs.microsoft.com/en-us/azure/service-bus-relay/relay-what-is-it
MIT License
12 stars 14 forks source link

Hyco ServerResponse Stream Interface Stalling #43

Closed philmay closed 4 years ago

philmay commented 4 years ago

Actual Behavior

  1. I have implemented a file server in Javascript using the hyco-https server, and when I create a file read stream for the fetched file and pipe it to the hyco ServerResponse object, the transfer stalls before the entire file has been streamed.
  2. It appears that the first chunk is being written to the ServerResponse object via the streams write() method, but that subsequent writes do not occur. The hyco implementation of the streams write() function appears to always return false, but then never issues the 'drain' event to restart writes.
  3. As a simple test, I modified the _writeRaw() method of the hyco OutgoingMessage object to always return true rather than false (_hyco_outgoing.js, line 321), and that "fixed" the problem. This change, of course, doesn't take into account the stream's interaction with the hyco Websocket, but it seems to demonstrate that the issue I am observing is a result of streams flow control.
  4. Here's test code that behaves the same way as my fileserver, which I tested with a 100Kb jpg image (this server works fine if I substitute the standard nodejs http library for hyco-https and access the server directly):
    
    const hycoHttp = require('hyco-https');
    const http = require('http');
    const url = require('url');
    const fs = require('fs');
    const path = require('path');
    const port = 8083;

// only interested in images and raw bytes (defaults to plain text below) const mimeType = { '.jpg': 'image/jpeg', '.dat': 'application/octet-stream' };

const relayNamespace = process.env.RELAY_NAMESPACE || ''; const relayPath = process.env.RELAY_PATH || ''; const saPolicy = process.env.RELAY_SA_POLICY_NAME || ''; const saKey = process.env.RELAY_SA_POLICY_KEY || '';

const TOKEN_TIMEOUT_SEC = 3000;

const hycoPath = '/test';

const uri = hycoHttp.createRelayListenUri(relayNamespace, relayPath);

// To run without hyco-https, uncomment the following line and comment out the following nine lines // const server = http.createServer( const server = hycoHttp.createRelayedServer( { server: uri, token: () => { console.debug('token refresh'); var relayToken = hycoHttp.createRelayToken(uri, saPolicy, saKey, TOKEN_TIMEOUT_SEC); return relayToken; } }, function (req, res) { console.log('GET ' + req.url);

    var modifiedPath = req.url.split(hycoPath)[1] || req.url;
    let pathname = path.join(__dirname, modifiedPath);

    fs.exists(pathname, function (exist) {
        if (!exist) {
            res.statusCode = 404;
            res.end(pathname + ' not found!');
            return;
        }

        let readStream = fs.createReadStream(pathname);
        readStream.on('error', function(err) {
            res.statusCode = 500;
            res.end('Error fetching file: ' + pathname + ' - ' + JSON.stringify(err));
        });
        readStream.on('open', function() {
            const ext = path.parse(pathname).ext;
            res.setHeader('Content-type', mimeType[ext] || 'text/plain' );
            readStream.pipe(res);
        });
    });
}

); server.listen(parseInt(port));

console.log('Fileserver listening on port ' + port);



## Expected Behavior
1. The `ServerResponse` object should allow streaming via the nodejs streams interface.

## Versions  
- OS platform and version: MacOS 10.14.6 and Docker `node:lts`
- Node Version: 12.9.1 and 12.13 (docker)
- NPM package version or commit ID: hyco-https 1.4.1
bainian12345 commented 4 years ago

Thank you for reporting this issue with details! This should be fixed now in the newest version 1.4.3