ladjs / superagent

Ajax for Node.js and browsers (JS HTTP client). Maintained for @forwardemail, @ladjs, @spamscanner, @breejs, @cabinjs, and @lassjs.
https://ladjs.github.io/superagent/
MIT License
16.58k stars 1.33k forks source link

Uploading a large file using stream.pipe(req) does not work #1654

Open gramakri opened 2 years ago

gramakri commented 2 years ago

Uploading files larger than the read stream's highWatermark does not work.

This is tested with node 16.13.1

Sample server:

const express = require('express'),
    fs = require('fs'),
    http = require('http');

function upload(req, res) {
    const out = fs.createWriteStream('/tmp/out');
    req.pipe(out).on('finish', function () {
        console.log('upload finished');
        res.status(200).send({});
    });
}

const app = express();
app.post('/upload', upload);

http.createServer({}, app).listen(3000, function () {
    console.log('listening on port 3000');
});

You can test the server itself works using curl -F file=@largefile.txt -X POST http://localhost:3000/upload

Superagent upload code:

'use strict';

const fs = require('fs'),
    superagent = require('superagent');

const readStream = fs.createReadStream('./largefile.txt');
const request = superagent.post('http://localhost:3000/upload')
request.on('response', function (response) {
    console.log('got response', response.status);
});
readStream.pipe(request);

The code above will just "hang" if largefile.txt is more than twice the highWaterMark. For fs, the default water mark is 64KB. So, if the file is say 200KB, the upload code will hang.

gramakri commented 2 years ago

The reason the code hangs is because the 'drain' event is only emitted once at https://github.com/visionmedia/superagent/blob/048cf185d954028b1dccde0717d2488b2284c297/src/node/index.js#L799

Changing that bit of code to the below makes the upload work.

  req.on('drain', () => {
    this.emit('drain');
  });

'drain' event can be emitted multiple times (each time req.write() returns false).