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

Unable to pipe from superagent to superagent #1267

Closed
jakeweary opened this issue Aug 27, 2017 · 10 comments
Closed

Unable to pipe from superagent to superagent #1267

jakeweary opened this issue Aug 27, 2017 · 10 comments

Comments

@jakeweary
Copy link

jakeweary commented Aug 27, 2017

It seems there's no way to pipe an image directly from one host to another using superagent only.
I tried to pass a stream to .attach() method and... turns out it works with everything but superagent itself.

Here is an example:

const superagent = require('superagent');
const request = require('request');
const https = require('https');

const upload = 'https://httpbin.org/post';
const image = 'https://httpbin.org/image';

(async () => {

    // works (but ugly and would be even more ugly with http/https logic)
    await superagent
        .post(upload)
        .attach('file', await new Promise(resolve => https.get(image, resolve)));

    // works (but requires additional dependency)
    await superagent
        .post(upload)
        .attach('file', request.get(image));

    // gives an error
    await superagent
        .post(upload)
        .attach('file', superagent.get(image));

})().catch(console.error);

Output:

me@pc:~/js$ node bug.js
TypeError: source.pause is not a function
    at Function.DelayedStream.create (/home/me/js/node_modules/delayed-stream/lib/delayed_stream.js:35:12)
    at FormData.CombinedStream.append (/home/me/js/node_modules/combined-stream/lib/combined_stream.js:43:37)
    at FormData.append (/home/me/js/node_modules/form-data/lib/form_data.js:68:3)
    at Request.attach (/home/me/js/node_modules/superagent/lib/node/index.js:204:25)
    at __dirname (/home/me/js/bug.js:23:10)
    at <anonymous>
    at process._tickCallback (internal/process/next_tick.js:188:7)
@gautamsamal
Copy link

@thepheer Facing the same issue with the latest version 6.1.0. Did you find any workaround with this particular package or any updates?

@jakeweary
Copy link
Author

@gautamsamal Oh, well, it's been about 4 years, haha. No idea what I did back then, probably just switched to another http client. Seems like this issue is not unique to superagent by the way, I had similar troubles with node-fetch and after some research I found this: form-data/form-data#382, form-data module is broken and seems to be abandoned, and most of nodejs http clients have it as a dependency.

@gautamsamal
Copy link

@thepheer Thanks for the additional information. Indeed the PR has been in the open state for almost a year, rather makes sense to switch the library.

@niftylettuce
Copy link
Collaborator

I'm now a maintainer of form-data and I merged that PR. form-data@4.0.0 now has this https://github.com/form-data/form-data/releases/tag/v4.0.0.

@niftylettuce
Copy link
Collaborator

niftylettuce commented Feb 15, 2021

Can someone fix the broken tests? I upgraded form-data here but tests failing for now.

@niftylettuce niftylettuce reopened this Feb 15, 2021
@niftylettuce
Copy link
Collaborator

 4 failing

 1) zlib should protect from zip bombs:
    Error: Timeout of 5000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
     at listOnTimeout (internal/timers.js:554:17)
     at processTimers (internal/timers.js:497:7)

 2) Multipart #attach(name, path) when a file does not exist should report ECONNREFUSED via the callback:

     Uncaught AssertionError: expected 'ENOENT' to be 'ECONNREFUSED'
     + expected - actual

     -ENOENT
     +ECONNREFUSED

     at Assertion.fail (node_modules/should/cjs/should.js:275:17)
     at Assertion.value (node_modules/should/cjs/should.js:356:19)
     at test/node/multipart.js:113:29
     at Request.callback (lib/node/index.js:905:3)
     at FormData.<anonymous> (lib/node/index.js:301:13)
     at FormData.CombinedStream._emitError (node_modules/form-data/node_modules/combined-stream/lib/combined_stream.js:207:8)
     at DelayedStream.<anonymous> (node_modules/form-data/node_modules/combined-stream/lib/combined_stream.js:133:10)
     at DelayedStream.<anonymous> (node_modules/form-data/node_modules/delayed-stream/lib/delayed_stream.js:69:15)
     at Array.forEach (<anonymous>)
     at DelayedStream.release (node_modules/form-data/node_modules/delayed-stream/lib/delayed_stream.js:68:24)
     at DelayedStream.resume (node_modules/form-data/node_modules/delayed-stream/lib/delayed_stream.js:55:10)
     at DelayedStream.pipe (node_modules/form-data/node_modules/delayed-stream/lib/delayed_stream.js:76:8)
     at FormData.CombinedStream._pipeNext (node_modules/form-data/node_modules/combined-stream/lib/combined_stream.js:121:12)
     at FormData.CombinedStream._realGetNext (node_modules/form-data/node_modules/combined-stream/lib/combined_stream.js:99:10)
     at FormData.CombinedStream._getNext (node_modules/form-data/node_modules/combined-stream/lib/combined_stream.js:82:12)
     at FormData.CombinedStream.resume (node_modules/form-data/node_modules/combined-stream/lib/combined_stream.js:154:10)
     at FormData.CombinedStream.pipe (node_modules/form-data/node_modules/combined-stream/lib/combined_stream.js:66:8)
     at lib/node/index.js:1237:16
     at node_modules/form-data/lib/form_data.js:408:7
     at node_modules/asynckit/parallel.js:27:9
     at node_modules/asynckit/lib/iterate.js:46:5
     at Immediate.nextTick_callback (node_modules/asynckit/lib/async.js:30:9)
     at processImmediate (internal/timers.js:461:21)

 3) req.parse(fn) should not emit error on aborted chunked json:
    Error: Timeout of 5000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
     at listOnTimeout (internal/timers.js:554:17)
     at processTimers (internal/timers.js:497:7)

 4) response should act as a readable stream:
    Error: Timeout of 5000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
     at listOnTimeout (internal/timers.js:554:17)
     at processTimers (internal/timers.js:497:7)

@niftylettuce
Copy link
Collaborator

Can anyone fix these broken tests?

@niftylettuce
Copy link
Collaborator

Tests seem to be broken right now, possible due to #1651 or 3c9c0f7 (related to this issue).

If someone can fix and submit a PR, then I can publish a new release of this package to npm.

You can reproduce by cloning this repo, running yarn, and then yarn test or just make test works.

@G-Rath
Copy link

G-Rath commented Dec 23, 2021

@niftylettuce I've attempted to look into this but frankly the codebase is too old-school for me to make much progress (I've not done a lot of work with the pre-class days).

I only looked into the response should act as a readable stream test, but it seemed to be failing in most commits - the only time it was consistently passing for me was the original commit that added it.

@niftylettuce
Copy link
Collaborator

Fixed in superagent v7.0.0.

See https://github.com/visionmedia/superagent/releases/tag/v7.0.0 for more insight.

We've also published this deprecation notice:

Please upgrade to v7.0.0+ of superagent.  We have fixed numerous issues with streams, form-data, attach(), filesystem errors not bubbling up (ENOENT on attach()), and all tests are now passing.  See the releases tab for more information at <https://github.com/visionmedia/superagent/releases>. Thanks to @shadowgate15 (aka Super Agent Taylor), @spence-s, and @niftylettuce.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants