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
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • 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

First I want to say, thanks for engaging on this, I think that our small group in the web-server-frameworks team is not enough input to get the best possible future solution. We are completely at the beginning of this, and mostly have a small group of active participants, so if you want to come to our meetings and help it would be super appreciated!

Secondly, I think it would be really cool if we could all come to the table with wild ideas in brainstorming (especially cool if everyone did this sort of gist as an example) so that we would have lots of examples to talk through. I will bring up the idea in an issue to help kickstart that convo.

Now that I am rested and have some time to really look at this, I figured I should share my thoughts:

  1. The event emitter to async iterable is awesome, but also for real server applications I worry folks will accidentally add await and killing it. This is not to say that for examples it is not really nice and simple, but just thought I would bring it up.
  2. Tell me more about the .handle call. Would this be optional, in that if you chose not to call it the base server would return some default response? What you make it sound like to me is .handle is only for http2/3, in which case seeing handleHTTP2 or something like that might have made more sense to me.
  3. How would you write application logic which should run on all protocol versions if the http2/3 stuff was in the above callback?
  4. One thing I hope we can add is static file serving as a first class feature, so fs.createReadStream really shouldn't be a thing IMO for this use case. Have you read the source of the send module? There are soooooo many concerns when serving static files and none of them are easily solved in an application context with an fs stream.
  5. I do like .body, I worried that it would conflict too much with what folks think of today with req.body which is the parsed json or text or whatever body-parser put there for them. Also, I think that since req is a stream, the verb .consume is commonly used for "give me all of the stream" in existing packages. Also, to play my own devils advocate, I think that the similarity might actually be good, just say "it used to be the object, now call the function to return the object" is pretty easy to explain to a beginner.
  6. Returning a response, going to just write a paragraph:

I really like the end user experience of returning a response. I think there are a ton of ways to go about this, and a bunch of prior art examples. I have even worked on a way so that the Express router could be used in this way before. Cases I think we would want to consider:

  • Return a promise. Async support should be first class (so any of the other below ideas should also support being the resolved value of a promise)
  • Return a plain object. There are two approaches I see to this:
    • Return a descriptor plain object which the underlying layer converts
    • Return the json body as an object and have the underlying layer convert it
  • Return an instance of a Response class. This is your above example, descriptive and clear, sometimes verbose
  • Return a string. Send the stream as response body
  • Return a stream. Send the stream as response body

So, all that laid out, it is my current opinion that this belongs in the frameworks which get built on top of the next generation of http in core. I think that this area is strongly tied to the user experience which "end users" need, but I think that for complexity, performance, and stability reasons, baking this into the core api or even the higher level http api might be early.

If you have not yet seen the technical plan issue, it lays out three layers and two are "in core". My thinking was to start with prototyping the "high level user api" and then move up to the "framework api".

PS. this was too long for me to properly proof read with the time I had today, if there are poorly worded or confusing ideas let me know and I will follow up again.

@bengl
Copy link
Author

bengl commented Sep 4, 2020

@wesleytodd

  1. The event emitter to async iterable is awesome, but also for real server applications I worry folks will accidentally add await and killing it. This is not to say that for examples it is not really nice and simple, but just thought I would bring it up.

An EE is totally fine here. That would at least prevent people from hurting themselves at least as much as the current API.

  1. Tell me more about the .handle call. Would this be optional, in that if you chose not to call it the base server would return some default response? What you make it sound like to me is .handle is only for http2/3, in which case seeing handleHTTP2 or something like that might have made more sense to me.

It was my goofy 1am way of avoiding an async IIFE. Stringly speaking, I'm not sure it's necessary at all. The only differentiator at a high level here is whether serverPush does something or not, so I think that whole bit can probably just be the inside of the loop, with an async IIFE if there's anything async happening.

  1. How would you write application logic which should run on all protocol versions if the http2/3 stuff was in the above callback?

I think you're basing this on the handle() method, but regardless, I'd expect something in the userland routing logic to handle the routes otherwise handled by serverPush. The reality is they're different ways of handling things, because without server push you have a request object, so I think they need to be handled separately. I could be convinced otherwise though. As you'll see in my answer to the next question.

  1. One thing I hope we can add is static file serving as a first class feature, so fs.createReadStream really shouldn't be a thing IMO for this use case. Have you read the source of the send module? There are soooooo many concerns when serving static files and none of them are easily solved in an application context with an fs stream.

Yeah I wasn't concentrating on this at all. I just added the stream for completion's sake. This actually seems like a good time to define some staticAsset-ish method that does a server push when it can (I'm hand-waving a lot of logic here) and otherwise does normal HTTP response (for HTTP1, etc.).

  1. I do like .body, I worried that it would conflict too much with what folks think of today with req.body which is the parsed json or text or whatever body-parser put there for them. Also, I think that since req is a stream, the verb .consume is commonly used for "give me all of the stream" in existing packages. Also, to play my own devils advocate, I think that the similarity might actually be good, just say "it used to be the object, now call the function to return the object" is pretty easy to explain to a beginner.

I like the idea of using whatever method is added to ReadableStream, which is where this should live, really.

  1. Returning a response, going to just write a paragraph:

Yep, yep, yeppity yep. Everything you mentioned in your bullet points is stuff supported by Osgood (modulo definitions of streams and Response), so yeah agree on all counts.

So, all that laid out, it is my current opinion that this belongs in the frameworks which get built on top of the next generation of http in core. I think that this area is strongly tied to the user experience which "end users" need, but I think that for complexity, performance, and stability reasons, baking this into the core api or even the higher level http api might be early.

Right now in core we can't return a Response object. We can't even really create them without doing some internal wizardry. If there was an avenue for this, it would enable the rest of what I proposed here, or what you proposed in the gist, to be implemented in userland. Sure we can fake it, but it would be nice not to.

When I get a chance, I'll have a look at that issue.

@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