Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[http.IncomingMessage] req.toWeb() #42529

Open
jimmywarting opened this issue Mar 30, 2022 · 22 comments
Open

[http.IncomingMessage] req.toWeb() #42529

jimmywarting opened this issue Mar 30, 2022 · 22 comments
Labels
feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem.

Comments

@jimmywarting
Copy link

jimmywarting commented Mar 30, 2022

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 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.formData()
  await request.json()
  await request.body.pipeTo(dest)
  
  request.headers.has('content-length')
  
  request.url // full url (populated with host from request headers and http(s) based on using http or https)
  
  request.signal // a abort signal that aborts when user cancel the request
})

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 jimmywarting added the feature request Issues that request new features to be added to Node.js. label Mar 30, 2022
@VoltrexKeyva VoltrexKeyva added the http Issues or PRs related to the http subsystem. label Mar 31, 2022
@ronag
Copy link
Member

ronag commented Apr 2, 2022

Doesn't the alternative you propose already work?

@jimmywarting
Copy link
Author

jimmywarting commented Apr 2, 2022

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 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'))
})
@github-actions
Copy link
Contributor

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 github-actions bot added the stale label Oct 18, 2022
@github-actions
Copy link
Contributor

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.

@ronag
Copy link
Member

ronag commented Jan 4, 2023

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/manual@v1.28.3/examples/http_server#using-the-stdhttp-library

@ronag ronag closed this as completed Jan 4, 2023
@ronag ronag reopened this Jan 4, 2023
@ronag
Copy link
Member

ronag commented Jan 4, 2023

I think the biggest challenge is implementing req.

@jimmywarting
Copy link
Author

jimmywarting commented Jan 4, 2023

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 })
@ronag
Copy link
Member

ronag commented Jan 4, 2023

Actually my previous example was wrong. Should be:

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

ronag commented Jan 4, 2023

@jimmywarting where does the responseWith come from?

@ronag
Copy link
Member

ronag commented Jan 4, 2023

Maybe this should be moved to wintercg?

@jimmywarting
Copy link
Author

jimmywarting commented Jan 4, 2023

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()

@ronag
Copy link
Member

ronag commented Jan 4, 2023

I like it!

@ronag
Copy link
Member

ronag commented Jan 4, 2023

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);
  })());
});
@jimmywarting
Copy link
Author

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
Copy link
Author

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/manual@v1.29.1/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) => {
@github-actions github-actions bot removed the stale label Jan 5, 2023
@tom-sherman
Copy link

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.

@merlindru
Copy link

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

@natemoo-re
Copy link

👆 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.

@daniel-nagy
Copy link

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.

Copy link
Contributor

github-actions bot commented Jul 4, 2024

There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the never-stale Mark issue so that it is never considered 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 github-actions bot added the stale label Jul 4, 2024
@mbrevda
Copy link

mbrevda commented Jul 4, 2024

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

@Rich-Harris
Copy link

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.

@github-actions github-actions bot removed the stale label Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem.
9 participants