nodejs / web-server-frameworks

A place for Node.js Web-Server Framework authors and users to collaborate
Other
182 stars 27 forks source link

Future of HTTP High Level API Design #60

Open wesleytodd opened 4 years ago

wesleytodd commented 4 years ago

To continue our discussion from the meeting today, we want to have an API design for the "mid-teir" which frameworks can build on top. To get started on this, we need to setup a deep dive discussion. Topics for that agenda:

  1. What is our design philosophy (small core, usable end user api, etc)?
  2. Do we prioritize compatibility with similar web standard apis?
  3. What is our balance between performance and developer expirence?

Once we have established that, I think we can start digging into more specifics like:

  1. Should we offer a cookie setter/getter api?
  2. Should we have a send like file server?
  3. Do we expose stream apis, hooks, etc?

The question I have is, do we want to use the regularly scheduled meeting for this? Thursday, August 6⋅6:00 – 7:00am PST?

wesleytodd commented 4 years ago

cc @nodejs/web-server-frameworks ☝️

delvedor commented 4 years ago

Another topic for the agenda that can easily live inside an issue is a wish list of features we would like to have based on our experience as maintainers of webframeworks. For example a nice onFinished API, simple observability, and so on.

The question I have is, do we want to use the regularly scheduled meeting for this? Thursday, August 6⋅6:00 – 7:00am PST?

The regular meeting is fine for me, with a slight preference for the 5-6pm UTC time.

wesleytodd commented 4 years ago

Hey, so last night I was just messing around reading docs and thinking and I made a gist. I posted on twitter and @bengl responded with a link to a different take.

I think we need to discuss the above issues, but one thing I really liked about the gist format is that it give concrete examples for us to discuss. Do folks like this idea and think it would be good for us all to take small ideas to share with the group? I feel like this would be a decent format for discussion and brainstorming. Anyway, feel free to post ideas you have here as I think the more ideas we have to pull from the better.

wesleytodd commented 4 years ago

PS: @ronag is there a good way to ping you (without co-opting an unrelated thread)? I just returned comments on the gist and I don't think it does notifications so I didn't @ mention. There is also the question of setting up some time with James to go over some of the lower level apis, but I was not sure how to reach out to you.

ronag commented 4 years ago

PS: @ronag is there a good way to ping you (without co-opting an unrelated thread)? I just returned comments on the gist and I don't think it does notifications so I didn't @ mention.

It does notifications 😄. You can also reach me by mail (it's on my GH profile).

wesleytodd commented 4 years ago

Not that this is by any means a valid sample of the community, but I did an informal twitter poll which has interesting results and enough answers to make it worth considering. https://twitter.com/wesleytodd/status/1303048777687855105?s=20

wesleytodd commented 4 years ago

Another gist to reference: https://gist.github.com/Rich-Harris/4c061058176bb7f914d229d5c2a5d8ce

ghermeto commented 4 years ago

Sorry, I missed that meeting. Can anyone fill me on why a middleware pattern should be part of the HTTP high-level API design and not the responsibility of the frameworks?

wesleytodd commented 4 years ago

The gist is really just me playing with what all layers might look like. I for sure do not intend to imply that a middleware pattern should land in core. Sorry if that is how the above gist's came across. The way I often work is to see if things make sense together in a wholistic way, so my framework.js file is exploring what the top level framework api would look like, and http-next would be a middle tier api. Does that help clear it up?

ghermeto commented 4 years ago

yup 🙂

ronag commented 4 years ago

@wesleytodd async (req) => { status, headers, body } is growing on me. It’s similar to undici.

wesleytodd commented 4 years ago

Big thumbs up!! Someone in the twitter thread said this, and I like it:

a server handler is a mapper. It takes an input (the request) and produces an output.

Obviously there are details we would have to work out (like what can body be, same for headers), but I think on the whole this is the cleanest api I have seen.

One of the driving reasons Express has been so popular for so long is it's design philosophy is to stay as close to the core api's as it can while still adding value. I think that the above api would enable this same design philosophy for express or its inheritor to implement the "return a response" api and still remain close to the "core" approach.

The frameworks can layer on top (while still following the spirit of the api) things like () => 404 to generate the rest of the response based on the status code. Another nice framework api for the middleware piece would be that any middleware which does not return a response like object does not send a response, instead continues the chain.

bajtos commented 4 years ago

Great discussion! (And greetings from https://loopback.io land 👋🏻 😄 )

I'd like to point few important benefits of the proposed syntax async (req) => { status, headers, body }.

When writing Express-style middleware, there are few things that are difficult to accomplish:

The proposed syntax makes these two tasks much easier to implement:

function addLogging(originalHandler) {
  return async function(req) {
    const start = Date.now();
    const result = await originalHandler(req);
    console.log('%s %sms', result.status, Date.now() - start);
    return result;
  }
}

function addCompression(originalHandler) {
  return async function(req) {
    const result = await originalHandler(req);
    // decide if we want to apply compression
    // modify result.headers and result.body as needed
    return result;
  }
}

I have a concern about the property name status though. I find it a bit ambiguous - is it referring to status code (200), status message (OK) or an object holding both ({code: 200, message: 'OK'})? Personally, I prefer to use statusCode for clarity.

ronag commented 4 years ago

How do we handle trailers + push?

mcollina commented 4 years ago

Packages like on-headers and on-finished abstract the low-level complexity away, but the middleware author must be aware of them.

They also rely on monkeypatching core, which is a significant issue for core maintainability.

ronag commented 4 years ago

@wesleytodd

This would be very elegant and probably sufficient for 90% of use cases:

middleware(async ({ 
  headers: Object|Array, 
  body: AsyncIterator,
  trailers: Promise,
  push: PushFactory
}) => ({ 
  headers: AsyncIterator|Object|Array,
  body: AsyncIterator,
  trailers: Promise
}))

headers with AsyncIterator would allow the case for informational headers.

No special classes required.

Missing the following features:

Would it be a viable option for frameworks such as express to not support some or all of the above missing features?

Trailers could be added like so (although I don't like it):

middleware(async ({ 
  headers: Object, 
  body: Readable, 
  trailers: Promise<Object> 
}) => ({ 
  headers: Object, 
  body: Readable,
  trailers: Promise<Object> 
}))

Informational headers:

middleware(({ 
  headers: Readable<Object>, 
  body: Readable<Buffer>, 
  trailers: Readable<Object> 
}) => ({ 
  headers: Readable<Object>, 
  body: Readable<Buffer>, 
  trailers: Readable<Object>
}))

Which could be unified into:

middleware(Readable<Headers|Readable|Trailers> => Readable<Headers|Readable|Trailers>)
ronag commented 4 years ago

Just thinking out loud:

middleware (async function * (src) {
  const headers = await src.next()

  yield headers

  const body = await src.next()

  yield body

  const trailers = await src.next()

  yield trailers
})
middleware (async function * ({ headers, body, trailers }) {
  for await (const h of headers) {
    yield h
  }

  yield info

  yield info

  for await (const b of body) {
    yield b
  }

  for await (const t of trailers) {
    yield t
  }
})
wesleytodd commented 4 years ago

So to expand out on the proposal above and give it some "real" (but very oversimplified) code for how you might write a minimal express like (but with response's returned) "framework" on top:

https://gist.github.com/wesleytodd/e5642c0d39fa71bdebf8ef31ddbd5e40#file-simple-framework-src-js

So the underlying protocol things would be hidden away, and for the body, we would totally avoid streams at the higher levels (unless the user decided to pass a stream of their own making as body when creating the response).

wesleytodd commented 4 years ago

Also, to address the generator function approach (which I think would make a great framework api, but not a great lower level), I think the layer we have laid out would enable building that api on top of it really simply. I don't think this decision would block any of the major frameworks from adopting it under the hood.

wesleytodd commented 7 months ago

Turns out I am not going to have time for this today, but we discussed this on the call today and decided to start moving this work into https://github.com/nodejs/http-next where we can be a bit more concrete and start working on real code examples. When we get the ideas from here moved over there we should close this issue. If someone wants to do that work, no need to wait on me, go for it!

fed135 commented 5 months ago

Potentially relevant discussion happening here

In short, should Node expose individual http versions? This goes against the initial strategy for http-next, but might limit potential uses cases like highly-secure services.