Skip to content

Instantly share code, notes, and snippets.

@bengl
Forked from wesleytodd/framework.js
Last active September 13, 2020 09:17
Show Gist options
  • Save bengl/585de1cf3fe13e94cdd8e97fe0a69e98 to your computer and use it in GitHub Desktop.
Save bengl/585de1cf3fe13e94cdd8e97fe0a69e98 to your computer and use it in GitHub Desktop.
Just noodling on the future Node.js and http
import http from 'http-next'
import fs from 'fs'
const server = http.createServer({
allowHTTP1: true,
allowHTTP2: true,
allowHTTP3: true,
key: fs.readFileSync('localhost-privkey.pem'),
cert: fs.readFileSync('localhost-cert.pem')
})
// Nixed a bunch of error handling to concentrate on the spirit of what I'm getting at here.
// That should totally be added back in. Sorry.
// Now it's just a simple echo server with a favicon since I didn't feel like writing the rest out :D
// Finally: note that I'm not arguing against any other options. Just throwing this out there.
for await (const session of server) {
;(async session => {
for await (const req of session) {
req.handle(() => {
// Two main things on this one:
// 1. All relevant stuff is in a callback which is simply never called in http1.
// 2. Returnable response objects, which I'll get to in a minute.
req.serverPush('/favicon.ico', () => {
const res = new http.ServerResponse()
fs.createReadStream('./public/favico.ico').pipe(res)
return res
}
const reqBody = await req.body()
const res = new http.ServerResponse(200)
res.write(reqBody)
return res // returning here also implicitly ends the writable stream if it has not been ended.
// Could also support returning buffers or strings
})
}
})(session)
}
// tl;dr i really like the concept of returning a response.
@wesleytodd
Copy link

The only differentiator at a high level here is whether serverPush does something or not

Makes more sense to me when you add that. I think I agree this is the end user api which makes the most sense. I added a small section to my example where I tried to illustrate the same thing but in the "framework" layer.

My thinking is, if we always hide that detail away, it might end up that users get unexpected behaviors. If we throw when they try to push and cannot, then they know that can happen and will account for it. Frameworks can and should consider making this simple for users, but also they also typically have a more robust error handling approach to help.

This actually seems like a good time to define some staticAsset-ish method

This is one of the things I think will most improve folks basic ability to use the core http apis, so I think the push parts are nice, but I think we just need a good set of built in "serve thing thing" methods (json, files, buffer) were the http stuff is handled for you like when you do it in express. This would make less code for framework authors to maintain, and also make it so folks don't have to find and pull in libraries when using the core apis for the most basic of tasks.

whatever method is added to ReadableStream

Now this is an idea I hadn't thought of, but now that you say it, having a .consume method there would be AMAZING! That is one of my most common annoying bits of boilerplate:

let d = []
s.on('data', (d) => data.push(d)).on('end', () => onConsume(Buffer.from(data)))
function onConsume (buf) {}

Would become:

onConsume(await s.consume())

Right now in core

I think right now in core you just cannot use the api's directly. And plenty of frameworks have implemented the "return a response" pattern. The goal of this is to build an api which gives number one priority to framework authors but also greatly improves the ability for direct usage for real end user use cases.

This takes me back to, I think the "return a response" pattern is best done in the framework layer. I think it is more elegant as a "sugar" while keeping the layer maintained by us "basic" in some sense. The additions I personally want to make are the ones where people think, oh that was easy, but really just added a footgun. For example fs.createReadStream('file.json').pipe(res), which just doesn't cut it as a static file server.

@bengl
Copy link
Author

bengl commented Sep 7, 2020

My thinking is, if we always hide that detail away, it might end up that users get unexpected behaviors. If we throw when they try to push and cannot, then they know that can happen and will account for it. Frameworks can and should consider making this simple for users, but also they also typically have a more robust error handling approach to help.

SGTM.

This is one of the things I think will most improve folks basic ability to use the core http apis, so I think the push parts are nice, but I think we just need a good set of built in "serve thing thing" methods (json, files, buffer) were the http stuff is handled for you like when you do it in express.

At first glance, this seems like a lofty goal, because making everyone happy here is going to be tough. Probably worth the effort though.

Now this is an idea I hadn't thought of, but now that you say it, having a .consume method there would be AMAZING!

I'm super curious as to how this would be received in core. There are probably numerous naming collisions in userland (probably also in core itself). You'd also have to support object mode, which I guess would return an array of objects? The interesting bit here, to me, is that a naive implementation would use Buffer.concat, which is kind fo slow, but by being in core, more interesting optimizations could be made.

Anyways, since streams are already async iterators, maybe this would to the trick? https://github.com/tc39/proposal-iterator-helpers

@awwright
Copy link

awwright commented Sep 13, 2020

I have a few thoughts on this I would like to present, but I would like to focus on one, the idea of returning a ServerResponse.

I have a library, http-transform, that allows you to do this:

function makeResponse(req){
	const res = new ResponsePassThrough;
	res.statusCode = 200;
	res.setHeader('Content-Type', 'text/plain')
	res.write('Line\r\n');
	// you can also `return res;` but this will return the writable interface, too.
	return res.readable;
}

From here, you can call makeResponse to get an IncomingMessage, and you can pipe it to a ServerResponse object:

function request(req, res){
	// you can also call .pipe() but for compatibility, this doesn't set the headers, it pipes only the body.
	makeResponse(req).pipeMessage(res);
}

Doing this native in Node.js would require a significant undertaking because it has an asymmetrical interface for "Readable" and "Writable" streams. (f the interfaces were symmetrical, you would make data readable by calling write()—but you call push(). Why?)

There are several problems with error propagation I've been unable to completely solve, because of bad assumptions that Node.js makes about event emitters and streams.

To properly implement this, Node.js would need a "Simplex Pair" object, where there is a single buffer that is filled from a Writable side, and drained from a Readable side. (As it stands right now, this is not easy to do, and linking a Readable to a Writable will create at minimum two places where data is buffered.)

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