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

Allow resolving promise with stream #90

Open
analog-nico opened this issue Feb 5, 2016 · 22 comments
Open

Allow resolving promise with stream #90

analog-nico opened this issue Feb 5, 2016 · 22 comments

Comments

@analog-nico
Copy link
Member

Request-Promise is not designed to be used with streams. However, Request-Promise could resolve the promise not with the response but with the stream object. Then the user could do some piping further down the promise chain.

See request/request#1990 which gave me the idea.

At community: +1s?

@analog-nico
Copy link
Member Author

This wouldn't be possible currently in conjunction with the simple option because Request-Promise would need to register .on('response', ... ) to get and check the statusCode. But then all .pipe(...) calls need to follow synchronously - which is not the case due to the async hop of the promise. (See the linked issue.)

@analog-nico
Copy link
Member Author

It turns out to be possible after all by pausing the stream during the async step:

new Promise(function(resolve, reject) {
    request.get('http://affenbits.com/')
        .on('response', function(response) {
            response.pause();
            resolve(response);  
        });
}).then(function(response) {
    response.pipe(process.stdout);
});

@simov
Copy link
Member

simov commented Feb 17, 2016

That's good, you can probably use additional pause option to enable that behavior? Also not sure if .on vs .one would make any difference in this case.

@analog-nico
Copy link
Member Author

A pause option would be neat. Although the solution above works well without the option, in the future the option could give request the power to pause the stream "at the right time". Who knows, maybe some cases come up where the response event it not "the right time".

@sabrehagen
Copy link

+1

@forivall
Copy link

wip / sample: https://gist.github.com/forivall/6b3f3718b5684e9ea8daa5a4dcd855a6

I'll turn that into a pull request if nobody else is working on this.

@analog-nico
Copy link
Member Author

Hi @forivall , a PR would be really awesome!

You'll need to add the changes to request-promise-core. This core lib is used by request-promise, request-promise-native, and request-promise-any. But anyhow, the change won't require specific code for any one of those libs...

@sabrehagen
Copy link

@forivall Thank you for your work. Eagerly awaiting your pull request!

@booooh
Copy link

booooh commented Sep 20, 2016

+1

@TinOo512
Copy link

+1

@jcgillespie
Copy link

+1

1 similar comment
@danielweck
Copy link

+1

@muliyul
Copy link

muliyul commented May 27, 2018

What's the status of this issue?

@tenoriojuann
Copy link

+1

2 similar comments
@iwko
Copy link

iwko commented Sep 25, 2018

+1

@dan-h-ch
Copy link

+1

@marcoschicote
Copy link

+1

@mildsunrise
Copy link

I've made a PR for this feature at request/promise-core#16!
To try it yourself:

npm install request-promise@"botgram/request-promise#feature-pause"

@mildsunrise
Copy link

Hmm, could someone review the PR? It'd be great to have this feature

@schw3i
Copy link

schw3i commented Dec 10, 2019

+1

@ps2goat
Copy link

ps2goat commented Mar 3, 2021

This seems like it will become one of those viral tweets, soon. +1

@gabe0x02
Copy link

gabe0x02 commented Mar 8, 2021

@ps2goat request module (and i assume this module) has been deprecated no more new features will be added. request/request#3142

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

No branches or pull requests