Mikeal's request - how to pipe only on success
Martin Cooper
request.get(options).pipe(dest);
The problem is that, if the 'get' comes back with, say, a 404, and the
origin server includes an HTML page for the 404 error, that page is
"helpfully" piped to 'dest' as if it were the content I was trying to
retrieve. That's not what I want. (In fact, I need to create my own
JSON error response.)
I've been unable to find any way of getting around this. I tried
providing an onResponse function and only piping from there, like
this:
options.onResponse = function (error, response) {
// ...
if (response.statusCode >= 200 && response.statusCode < 300) {
// ...
req.pipe(dest);
}
// ...
};
req = request.get(options);
but then 'request' complains "You cannot pipe after the response event".
How can I pipe back the response only when the request was successful,
and create my own response content on error? Is there a way to do this
and retain the magic of 'request'?
--
Martin Cooper
Isaac Schlueter
acceptable response status code values.
> --
> Job Board: http://jobs.nodejs.org/
> Posting guidelines: https://github.com/joyent/node/wiki/Mailing-List-Posting-Guidelines
> You received this message because you are subscribed to the Google
> Groups "nodejs" group.
> To post to this group, send email to nod...@googlegroups.com
> To unsubscribe from this group, send email to
> nodejs+un...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/nodejs?hl=en?hl=en
>
Mikeal Rogers
It does occur to me that the restriction you're hitting here is really in the wrong place, there is no reason, technically, that you can't pipe() after the response event so long as no data has been emitted so what I should really do is move the restriction to after a data event has been emitted so you can use onResponse like you're attempting to.
-Mikeal
Martin Cooper
> I don't like adding more declarative features like list of acceptable status codes.
>
> It does occur to me that the restriction you're hitting here is really in the wrong place, there is no reason, technically, that you can't pipe() after the response event so long as no data has been emitted so what I should really do is move the restriction to after a data event has been emitted so you can use onResponse like you're attempting to.
>
That would be nice. Certainly if I comment out that particular check /
throw, I get the behaviour I'm looking for. At that point, it works
like a charm.
--
Martin Cooper
Mikeal Rogers
Martin Cooper
> I can't just comment it out, there is a small refactor that would have to happen to make it work, but it's doable once I get some time to spend on it.
Oh, sure, I realise that. I was just confirming that removing the
check from that particular spot resolves the issue for my use case.
Thanks!
--
Martin Cooper
Mikeal Rogers
Dominic Tarr
req.end()
Martin Cooper
> you could do this easily with core http, it's really not that different.
What's different is that I would lose the magic of 'request' - things
like automatic redirect handling, proxy handling, header transfer,
multipart, to name only a few. The 'request' library wraps all that up
very nicely into an easy to use package.
> var req = http.request(opts)
> req.end()
> req.on('response', function (res) {
> //check status & start pipe
> })
> now an onResponse: property is really ugly, it's not idiomatic node js.
> if request was to support this, that is it should do it by remitting
> 'response' like in core.
The onResponse property is part of the 'request' API today.
--
Martin Cooper
Mikeal Rogers
you could do this easily with core http, it's really not that different.var req = http.request(opts)
req.end()req.on('response', function (res) {//check status & start pipe})now an onResponse: property is really ugly, it's not idiomatic node js.
Paul
point. Having the ability to conditionally pipe in onResponse would
help me greatly in my node-azure API project (which has already
benefited greatly from 'request').
Thanks for bringing it up!
- Paul
On Oct 22, 12:38 pm, Mikeal Rogers <mikeal.rog...@gmail.com> wrote:
> On Oct 22, 2011, at October 22, 20111:10 AM, Dominic Tarr wrote:
>
> > you could do this easily with core http, it's really not that different.
>
> > var req = http.request(opts)
> > req.end()
> > req.on('response', function (res) {
> > //check status & start pipe
> > })
>
> > now an onResponse: property is really ugly, it's not idiomatic node js.
>
> yeah, it's a somewhat legacy API in request from the more declarative days before I had all the pipeing.
>
> the thing is, if you pipe() request to another stream before the "response" event, onResponse isn't necessary as the default callback is fired at that time because there won't be any buffering of the response body.
>
> the issue here is, it hasn't been piped yet, it won't be piped until the response event.
>
> but, I could easily emit "response" on the request object to make it like core, I might already do it ( i can't remember offhand ).
>
>
>
>
>
>
>
>
>
> > if request was to support this, that is it should do it by remitting 'response' like in core.
>
> > request (opts).on('response', function (res) {
> > //check status & start pipe
> > })
>
> > It's not just a check for a state, it actually throws that because it knows that no data will get sent because the code that sets up sending the data already fired, hence the refactor.
>
> > On Oct 21, 2011, at October 21, 20113:53 PM, Martin Cooper wrote:
>
> > >> I can't just comment it out, there is a small refactor that would have to happen to make it work, but it's doable once I get some time to spend on it.
>
> > > Oh, sure, I realise that. I was just confirming that removing the
> > > check from that particular spot resolves the issue for my use case.
> > > Thanks!
>
> > > --
> > > Martin Cooper
>
> > >> On Oct 21, 2011, at October 21, 20112:55 PM, Martin Cooper wrote:
>
> > >>>> I don't like adding more declarative features like list of acceptable status codes.
>
> > >>>> It does occur to me that the restriction you're hitting here is really in the wrong place, there is no reason, technically, that you can't pipe() after the response event so long as no data has been emitted so what I should really do is move the restriction to after a data event has been emitted so you can use onResponse like you're attempting to.
>
> > >>> That would be nice. Certainly if I comment out that particular check /
> > >>> throw, I get the behaviour I'm looking for. At that point, it works
> > >>> like a charm.
>
> > >>> --
> > >>> Martin Cooper
>
> > >>>> -Mikeal
>
> > >>>> On Oct 21, 2011, at October 21, 20112:08 PM, Isaac Schlueter wrote:
>
> > >>>>> It seems like it'd be a good idea to have an option to specify
> > >>>>> acceptable response status code values.
>
Mikeal Rogers
https://github.com/mikeal/request/commit/248e9d65e73ac868948a82d07feaf33387723a1d
Please try this out, i have a minimal test but I want some more feedback before I push it in a release.
I also added a response event so you can do.
request.post(url).on('response', function () {
})
Should make Dominic happy :)
-Mikeal
Dominic Tarr
Martin Cooper
> Fixed in master.
>
> https://github.com/mikeal/request/commit/248e9d65e73ac868948a82d07feaf33387723a1d
>
> Please try this out, i have a minimal test but I want some more feedback before I push it in a release.
Works like a charm. Thank you!
--
Martin Cooper