Explore optional transform hook

[michaelnisi]

RFC

Large bodies, XML feeds for example, must be processed as streams. Here , I’m exploring a way to optionally allow hooking a transform stream into the string client. I chose Transform to give users control over accumulation. I offer this simple approach for discussion, don’t merge. 🧐

[misterdjules]

@michaelnisi I'd like to understand your use case(s) better: why not using the HttpClient class and building a pipeline of streams from the response stream instead of using the StringClient class?

[michaelnisi]

@misterdjules Extending HttpClient would be an option, in fact while doing just that, adding StreamClient, I realized that the difference is minimal. So, to leverage the hardened and well-used StringClient, I suggest this optional transform stream hook to give users control over buffering.

My use case is nothing special. Receive large amounts of data and efficiently stream it to a database without saturating memory, XML to JSON to LevelDB. I have a custom HTTP client that does that, but I would like to participate in the robustness of restify and remove the HTTP portion of my code.

[misterdjules]

@michaelnisi By using the HttpClient class, I didn't mean extending it but rather creating HttpClient instances and piping their response stream to any other stream(s) or streams pipeline. Something along those lines:

var httpClient = clients.createHttpClient({
    url: 'http://localhost/'
});

httpClient.get('/', function (getErr, req) {
    req.on('result', function (resultErr, res) {
        // Use res.pipe or any other mechanism to build a pipeline that performs
        // the appropriate operations on the response.
    });
});

My understanding is that the StringClient and JsonClient classes are meant to be used for very specific use cases that are sufficiently common to justify exposing them as built-in client types. Any other custom use case seems to be addressed by using the HttpClient class and using the request and response streams it exposes.

[michaelnisi]

You are probably right. Of course, I see the elegance of keeping StringClient pure. Having to repeat some of its code in the custom pipeline may be an OK price for that. 😏

[misterdjules]

@michaelnisi Looking again at this, I agree it's unfortunate that consumers of Restify clients would need to rewrite most of the logic in StringClient/JsonClient when writing functional equivalents that stream data instead of buffering it.

I'm not sure passing an optional transform stream to StringClient would be the approach I would recommend though, but didn't have time to come up with a rationale for why that is or with an alternative that I prefer.

I'll keep thinking a bit more about this and come back with other proposals for us to consider if that works for you. In the meantime let's keep this discussion going and feel free to add comments or questions.

Thanks again for starting the discussion 👍

[michaelnisi]

I know. My idea is kind of tempting because it’s minimal—low hanging fruit. 🐍🍎

[misterdjules]

@michaelnisi Actually, I don't know when I'll be able to get back to exploring design alternatives, but somehow I feel like we need to explore them and present why we want to go with the one you proposed (or any other one).

Do you have time to write something up about various ways to design this feature with the pros and cons of each of them, and why you'd advocate for going with the one you suggested in this PR?

Some of the alternatives I can think of (list is not exhaustive, and this is just me thinking out loud so not all suggestions necessarily make sense):

• Making StringClient's methods return a readable stream.
• Making StringClient's methods pass a readable stream in lieu of the data argument passed to its callbacks.
• Adding a "StreamingStringClient" class that inherits from stream.Readable and that reuses most of the logic of StringClient.

What do you think?

[michaelnisi]

@misterdjules Great, let’s keep this ball rolling. I’ll analyse the options, probably leading to a minimally-invasive proposal, which is kind of my stand at the moment.