node icon indicating copy to clipboard operation
node copied to clipboard

[http.IncomingMessage] `req.toWeb()`

Open jimmywarting opened this issue 3 years ago • 36 comments

What is the problem this feature will solve?

Reading the body out of the http.IncomingMessage is troublesome without any 3th party package...

Request on the other hand has features such as text, blob, json, arrayBuffer, formData bytes, and body for parsing it.

What is the feature you are proposing to solve the problem?

It would be a convenient to have something like a req.toWeb() utility added onto the simple http server 🚀

something like the stream.Readable.toWeb() but for the http.IncomingRequest to web Request

import http from 'node:http'

const app = http.createServer(async (req, res) => {
  const request = req.toWeb()
  
  await request.blob()
  await request.arrayBuffer()
  await request.bytes()
  await request.formData()
  await request.json()
  await request.body.pipeTo(dest)
  
  request.headers.has('content-length') // now getting the benefit of case insensitive.
  
  // full url (populated with host from request headers and http(s) based on using http or https)
  request.url 
  
  // a abort signal that aborts when user cancel the request
  // can be useful for aborting other pending database queries
  request.signal
})

What alternatives have you considered?

currently doing some simplier variant like this one:

const app = http.createServer((r, res) => {
  const req = new Request(r.url, {
    headers: r.headers,
    method: r.method,
    body: r
  })
})

having this req.toWeb() built in would be very handy

jimmywarting avatar Mar 30 '22 15:03 jimmywarting

Doesn't the alternative you propose already work?

ronag avatar Apr 02 '22 17:04 ronag

it's too verbose and having to do it in every project.

would have been nice to have that built in doe... there is also some signaling stuff, url construction and referrer that i would have to set up also to get a complete Request object.

const app = http.createServer((r, res) => {
  const ctrl = new AbortController()
  const headers = new Headers(r.headers)
  const url = `http://${headers.get('host')}${r.url}`
  
  req.once('aborted', () => ctrl.abort())
 
  const req = new Request(url, {
    headers,
    method: r.method,
    body: r,
    signal: ctrl.signal,
    referrer: headers.get(referrer)
  })
})

i just thought this toWeb() would be better than having to add all of the other stuff Request has to make IncomingMessage be more web-like.

a alternative could be to have a getter function like:

class IncomingMessage {
  get request () {
    return this._request ??= new Request(...))
  }
}

// and then later be able to do something like:
const app = http.createServer(async (x, res) => {
  const json = await x.request.json()
})

on another note it would also be nice to have a respondWith on the IncomingMessage as well so you can use respondWith a standardlized response object and combine it with the new fetch api and have it be very similar to a service worker and be able to have some isomorphic code shared between server and the browser.

class IncomingMessage {
  get request () { ... }
  respondWith (x) { ... }
}

// and then later be able to do something like:
const app = http.createServer(async evt => {
  const json = await evt.request.json()
  evt.respondWith(fetch('https://httpbin.org/get'))
})

jimmywarting avatar Apr 02 '22 19:04 jimmywarting

There has been no activity on this feature request for 5 months and it is unlikely to be implemented. It will be closed 6 months after the last non-automated comment.

For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] avatar Oct 18 '22 01:10 github-actions[bot]

There has been no activity on this feature request and it is being closed. If you feel closing this issue is not the right thing to do, please leave a comment.

For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] avatar Nov 18 '22 01:11 github-actions[bot]

What about just introducing a new api? To avoid confusion and overlap, i.e.:

await http.serve(async req => {
  req.respondWith(fetch('https://httpbin.org/get'))
}, { port })

https://deno.land/[email protected]/examples/http_server#using-the-stdhttp-library

ronag avatar Jan 04 '23 10:01 ronag

I think the biggest challenge is implementing req.

ronag avatar Jan 04 '23 10:01 ronag

I think the biggest challenge is implementing req.

more like:

http.serve(async extendableEvent => {
  const fd = await extendableEvent.request.formData()
  extendableEvent.respondWith(
    fetch('https://httpbin.org/post', { method: 'post', body: fd })
  )
}, { port })

jimmywarting avatar Jan 04 '23 10:01 jimmywarting

Actually my previous example was wrong. Should be:

await http.serve(async req => {
  return fetch('https://httpbin.org/get')
}, { port })
``

ronag avatar Jan 04 '23 10:01 ronag

@jimmywarting where does the responseWith come from?

ronag avatar Jan 04 '23 10:01 ronag

Maybe this should be moved to wintercg?

ronag avatar Jan 04 '23 10:01 ronag

Why extendableEvent?

FetchEvent is class that extend extendableEvent

where does the responseWith come from?

think you meant respondWith see: https://developer.mozilla.org/en-US/docs/Web/API/FetchEvent/respondWith

would like to have something that is more towards something already web-spec'ed like the evt.respondWith()

jimmywarting avatar Jan 04 '23 10:01 jimmywarting

I like it!

ronag avatar Jan 04 '23 11:01 ronag

If we take the MDN example and serverify it:

const server = http.listen({ port })
server.addEventListener('fetch', (event) => {
  // Prevent the default, and handle the request ourselves.
  event.respondWith((async () => {
    // Try to get the response from a cache.
    const cachedResponse = await caches.match(event.request);
    // Return it if we found one.
    if (cachedResponse) return cachedResponse;
    // If we didn't find a match in the cache, use the network.
    return fetch(event.request);
  })());
});

ronag avatar Jan 04 '23 11:01 ronag

Like that idea even more. it would make it even easier to move things to a service worker if you want the app to work offline

jimmywarting avatar Jan 04 '23 11:01 jimmywarting

speaking of the deno server you linked to... that server is just an higher level api on top of what deno already have at it's core

so if you are creating a server without any dependencies, then it's more like a fetch event. https://deno.land/[email protected]/examples/http_server

// Start listening on port 8080 of localhost.
const server = Deno.listen({ port: 8080 });
console.log(`HTTP webserver running.  Access it at:  http://localhost:8080/`);

// Connections to the server will be yielded up as an async iterable.
for await (const conn of server) {
  // In order to not be blocking, we need to handle each connection individually
  // without awaiting the function
  serveHttp(conn);
}

async function serveHttp(conn: Deno.Conn) {
  // This "upgrades" a network connection into an HTTP connection.
  const httpConn = Deno.serveHttp(conn);
  // Each request sent over the HTTP connection will be yielded as an async
  // iterator from the HTTP connection.
  for await (const requestEvent of httpConn) {
    // The native HTTP server uses the web standard `Request` and `Response`
    // objects.
    const body = `Your user-agent is:\n\n${
      requestEvent.request.headers.get("user-agent") ?? "Unknown"
    }`;
    // The requestEvent's `.respondWith()` method is how we send the response
    // back to the client.
    requestEvent.respondWith( // <---
      new Response(body, {
        status: 200,
      }),
    );
  }
}

not entirely like web-speced api's as your suggestion

const server = http.serve(port)
server.addEventListener('fetch', (event) => {

jimmywarting avatar Jan 04 '23 11:01 jimmywarting

A more web-standards based http server API is a good idea, but I think it's a separate topic to this issue.

Converting to a web request is a feature that stands along when for example integrating old node.js code with newer fetch-based stuff. Also a new server HTTP API can be done in userland, whereas a built in method on IncomingMessage cannot.

tom-sherman avatar Jan 16 '23 13:01 tom-sherman

Just leaving this here:

https://github.com/withastro/astro/blob/2dca81bf2174cd5c27cb63cb0ae081ea2a1ac771/packages/integrations/vercel/src/serverless/request-transform.ts#L2

Astro took this from SvelteKit. Just requires you to install set-cookie-parser:

npm i set-cookie-parser && npm i -D @types/set-cookie-parser

Then you get a Web Fetch API Request using get_request.

You can also set a ServerMessage's data from a Web Fetch API Response using set_response

merlinaudio avatar Apr 29 '23 19:04 merlinaudio

👆 Chiming in from the Astro team, we have quite a bit of code that has to manage converting Node's HTTP message primitives to their standardized web equivalents. Modern frameworks have clearly embraced web standard Request/Response for server APIs, so it would be amazing to move this kind of thing from userland to Node core.

natemoo-re avatar Oct 11 '23 15:10 natemoo-re

I wanted to use Web standard Request objects with Node's http server as well. After some digging I came across this https://github.com/honojs/node-server. It's a Node adaptor for the Hono web framework. However, I'm using it standalone to just get a basic server that uses Web standard Request and Response objects.

import { serve } from "@hono/node-server";

serve({
  async fetch(request) {
    return new Response("👋");
  },
  port: 4000
});

Seems to be working well so far. Would be great to see Node provide an http server API that uses Web standards, similar to Bun.

daniel-nagy avatar Jan 05 '24 16:01 daniel-nagy

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the https://github.com/nodejs/node/labels/never-stale label or close this issue if it should be closed. If not, the issue will be automatically closed 6 months after the last non-automated comment. For more information on how the project manages feature requests, please consult the feature request management document.

github-actions[bot] avatar Jul 04 '24 01:07 github-actions[bot]

it would be great to have this nativly available in node. (How) can us simpletons help move this forward?

mbrevda avatar Jul 04 '24 06:07 mbrevda

We on the SvelteKit team would love to be able to get rid of this code, though if it came down to a choice between req.toWeb (and some corresponding approach for stuffing the resulting Response back onto res) and a modern serve API along the lines of Deno.serve or Bun.serve, the latter seems like the more future-proof choice.

Both things can be done in userland (converting Node to Web, and designing a standards-based API), but at least for frameworks, the only reason to convert is so that we can provide the standards-based API. If we could deal with Request and Response natively, we wouldn't need req and res in the first place, which surely creates some opportunities for optimisation.

Rich-Harris avatar Jul 04 '24 11:07 Rich-Harris

Chiming in.

Next.js user here.

I love building third party libraries for Next.js. EXCEPT if I need to support both API Routes from the Pages router and API routes from the App router, both which have their strengths. I would love for these libraries to simply take "req" and be done with it, but we have NextApiRequest (IncomingMessage) and NextRequest (Request).

So yes, req.toWeb() would be sick.

daveycodez avatar Dec 15 '24 04:12 daveycodez

Just leaving this here:

https://github.com/withastro/astro/blob/2dca81bf2174cd5c27cb63cb0ae081ea2a1ac771/packages/integrations/vercel/src/serverless/request-transform.ts#L2

Astro took this from SvelteKit. Just requires you to install set-cookie-parser:

npm i set-cookie-parser && npm i -D @types/set-cookie-parser

Then you get a Web Fetch API Request using get_request.

You can also set a ServerMessage's data from a Web Fetch API Response using set_response

I installed set-cookie-parser but I'm not seeing get_request anywhere

daveycodez avatar Dec 15 '24 04:12 daveycodez

I wanted to use Web standard Request objects with Node's http server as well. After some digging I came across this https://github.com/honojs/node-server. It's a Node adaptor for the Hono web framework. However, I'm using it standalone to just get a basic server that uses Web standard Request and Response objects.

import { serve } from "@hono/node-server";

serve({ async fetch(request) { return new Response("👋"); }, port: 4000 }); Seems to be working well so far. Would be great to see Node provide an http server API that uses Web standards, similar to Bun.

This was the ticket for me. Definitely agree with others that it would be great to have this built into Node like Deno.serve/Bun.serve. What would be even better is if there was a WinterCG function that could be called the same in all runtimes.

anderspitman avatar Jan 08 '25 18:01 anderspitman

I studied several implementations of this pattern and developed a standalone lib that works with Server instances from node:http and node:http2. If anyone else wants to collaborate to publish it as a standalone lib and then prepare a PR for the core, please ping me.

eugene1g avatar Jan 08 '25 18:01 eugene1g

I studied several implementations of this pattern and developed a standalone lib that works with Server instances from node:http and node:http2. If anyone else wants to collaborate to publish it as a standalone lib and then prepare a PR for the core, please ping me.

Do you have a link to the current code?

anderspitman avatar Jan 08 '25 18:01 anderspitman

Here's a great package for it I found from Micheal Jackson on the Remix team in case anyone is interested: https://github.com/mjackson/remix-the-web/tree/main/packages/node-fetch-server

rossrobino avatar Jan 08 '25 19:01 rossrobino

Here's a great package for it I found from Micheal Jackson on the Remix team in case anyone is interested: https://github.com/mjackson/remix-the-web/tree/main/packages/node-fetch-server

This looks more fit-to-purpose. Also significantly smaller and thus hopefully easier to fork if necessary:

du -h node_modules/@hono/node-server/
20K	node_modules/@hono/node-server/dist/utils/response
40K	node_modules/@hono/node-server/dist/utils
324K	node_modules/@hono/node-server/dist
340K	node_modules/@hono/node-server

du -h node_modules/@mjackson/node-fetch-server/
32K	node_modules/@mjackson/node-fetch-server/dist
52K	node_modules/@mjackson/node-fetch-server/

Thanks!

anderspitman avatar Jan 08 '25 20:01 anderspitman

I studied several implementations of this pattern and developed a standalone lib that works with Server instances from node:http and node:http2. If anyone else wants to collaborate to publish it as a standalone lib and then prepare a PR for the core, please ping me.

Do you have a link to the current code?

@anderspitman I just extracted it into a public repo - https://github.com/eugene1g/node-http-to-fetch. It's for HTTP1, and HTTP2, has tests, and benchmarks. I should shape up the repo to make this library easier to consume.

eugene1g avatar Jan 08 '25 20:01 eugene1g