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.59k stars 1.33k forks source link

Using superagent as a writable stream #1250

Open jwgoh opened 7 years ago

jwgoh commented 7 years ago

Hi there,

I am trying to upload some files to OpenStack ObjectStorage, creating a read stream from a file and piping into request.

var request = require('superagent');

var req = request.post(base);
var stream = fs.createReadStream('/path/to/file');

req.on('response', function(res) {
  console.log('Success');
});

stream.pipe(req);

The above works for text files, but for images nothing happens and the request times out.

Update: I added listeners for the 'error' event on both readable and writable streams but there is nothing

Using .attach() "works" but the uploaded file now has all the multipart form metadata i.e. the image is "corrupted".

Versions

Node v4.8.3 Superagent 3.5.2

Any ideas? I feel like I am missing something obvious here, thanks in advance 😄

kornelski commented 7 years ago

We don't support piping to the request. You have to use .attach()

jwgoh commented 7 years ago

@pornel Are you say piping is an "unsupported" feature of superagent? Yes, I don't see it in the main documentation but there are test case & test documentation for it here. Edit: It is in the main documentation.

Using .attach() is not ideal because of the multipart form metadata that will be included as mentioned above, since I am not uploading to a web server. Is there a way around this?

kornelski commented 7 years ago

Oh, sorry. I wasn't aware of that :D If there's a test for it then I guess it should work, but obviously I'm not any better informed about what's going on than you.

.attach() is indeed for multipart. If you just want raw bytes sent, you can use .send() with a Buffer.

jwgoh commented 7 years ago

If you just want raw bytes sent, you can use .send() with a Buffer.

Yeap this would work.

The weird thing is that the piping works for node 0.12.7, superagent 1.2.0, but NOT for node 4.8.3, superagent 3.5.2 when the file is anything other than simple text or html files.

// app.js
'use strict';

var request = require('superagent');

var req = request
  .post('https://requestb.in/example')
  .on('response', function(res) {
    console.log(res.status);
    console.log(res.text);
  })
  .on('error', function(err) {
    console.log(err);
  });

process.stdin.pipe(req);
$ node app.js < file.txt
$ node app.js < example.png # Fails for node `4.8.3`, superagent `3.5.2`

Something must have changed in Node's Stream interface. Let me know if you can/can't reproduce the issue above.

jmarca commented 7 years ago

Perhaps my issue is related.

I'm trying to stream an image into CouchDB. It works fine with Request, but fails with superagent.

I wrote a test and put it into node/pipe.js as follows:

  it('should act as a writable stream for images', function(done){
    var req = request.post(base);
    var stream = fs.createReadStream('test/node/fixtures/plot.png');
      stream.on('data',(chunk)=>{
          console.log(`Received ${chunk.length} bytes of data.`);
      })
      req.type('png');
      req.accept('png');
      req.on('response', function(res){
          console.log('got response');
          done();
    });

    stream.pipe(req);
  })

It hangs after reading just two chunks. Same behavior on my app (reads two chunks and hangs)

james@emma superagent[bug/stream_image]$ ./node_modules/.bin/mocha  test/node/pipe.js  --timeout 5000

  request pipe
    1) should act as a writable stream
Received 65536 bytes of data.
Received 65536 bytes of data.
    2) should act as a writable stream for images
    3) should act as a readable stream

node version 7.10.0

attached is the plot file I'm using plot

jmarca commented 7 years ago

my fork with the test case details: https://github.com/jmarca/superagent/tree/bug/stream_image

bra1n commented 5 years ago

Is this still an issue?

jmarca commented 5 years ago

I’ll fire it up and see. Is been quite a while!

yocontra commented 3 years ago

Yep this is still an issue ^

FWIW I have a workaround that seems to be working as expected:

new Promise((resolve, reject) => {
    const upload = request.post(url)
    const req = upload.request()
    pipeline(stream, req, (err) => {
      if (err) {
        upload.abort()
        return reject(err)
      }

      upload.end((err, res) => {
        if (err) return reject(err)
        return resolve(res.body)
      })
    })
  })