Add body as a property to response? #28

Closed
aseemk opened this Issue May 5, 2011 · 6 comments

Comments

Projects
None yet
4 participants

aseemk commented May 5, 2011

Hey there,

How do you feel about adding body as a property to response? That would simplify callbacks to be just function (error, response).

I've become a big fan of Streamline.js, which lets you write async code as if it was sync (really awesome <3). It makes the (admittedly not robust) assumption that async callbacks are of the form (error, result), which lets you do stuff like:

try {
    var result = someAsyncTask(foo, bar, _);    // the _ is the callback placeholder
    // do something with result
} catch (error) {
    // do something with error
}

Which would otherwise be:

someAsyncTask(foo, bar, function (error, result) {
    if (error) {
        // do something with error
    } else {
        // do something with result
    }
});

The reason that's awesome is because this totally removes crazy indentation levels and makes code generally so much easier to follow and reason about (especially when it comes to error handling).

But of course, since JS as a language doesn't have a notion of multiple return values, any callback arguments beyond the first two are lost by Streamline. Hence my request. =)

In parallel, I'm working with the Streamline author to see if we can add a feature that automatically detects and "packs" multiple return values into an array:

var results = someAsyncTask(foo, bar, _);
// if someAsyncTask calls the callback w/ multiple values:
var a = results[0], b = results[1], c = results[3];

So the pattern for using request would look like:

var result = request.get(opts, _);
var response = result[0];
var body = result[1];

Which is reasonable. But I figure I'd ask -- would you be open to adding body as a property onto the response object to streamline this? ;)

Thanks in advance!

gasi commented May 5, 2011

+1

I think this concept is great. I was speaking about this in #node yesterday. I'm going to create a basic object framework for Requests and Response á la HTTP::Message in the Perl community, and fork Request to use that. Ultimately, that's what needs to happen. The format for HTTP Header/Request/Response/Message must be standardized in Node, at a high level.

Owner

mikeal commented May 6, 2011

in the cases that we do buffer the response body i don't have any problem adding it to the body, it's already a property of the request object. we'll still pass it in order to be reverse compatible.

I think passing back a initiating request object on response would be kosher too, especially in the cases of a redirect where the response comes from a different Location... that information is lost as is, as the process happens internally.

Owner

mikeal commented May 24, 2011

what do you mean by "initiating"? that gets blown away during each redirect, it would always be the current http.request object.

all of this information is a property of the Request object as is, so adding more properties to other objects appears unlikely to leak. keep in mind that response.body is only there if you don't use streaming.

i'll take a patch on this at any time, or i'll add it when i get a minute.

aseemk commented May 25, 2011

Awesome, thanks @mikeal. I haven't had time to add this myself, but I'm in no rush, so I appreciate your willingness to do this yourself. I eagerly await the results!

mikeal closed this in b9aff1f Jul 21, 2011

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