discussion: Make std/http use web standard FetchEvent

[lucacasonato]

I know this is really late for such a change, but I think it's important to discuss and I think I can implement this in a couple of hours.

I propose to switch the server in std/http from using a proprietary ServerRequest (and ServerResponse, denoland/deno#4861) to use the web standard FetchEvent, Request and Response that is used by fetch and service workers (https://developer.mozilla.org/en-US/docs/Web/API/FetchEvent).

This would mean that Server changes from a AsyncIterableIterator<ServerRequest> to a AsyncIterableIterator<FetchEvent>. This would slightly change the semantics of using Server, but it would reduce complexity and we would provide an interface that is web compatible and familiar to many web devs.

[samuelgozi]

This is not a good idea.
In the FetchAPI a Request represents data being sent outside, and a Response represents incoming data. On the server its the opposite.

For example on Fetch you can do response.json() because the response is incoming, but in the server that is irrelevant. Practically most of the FetchAPI equivalent class's methods are not relevant on the server-side because they serve the opposite function.

I have a proposal for the ServerRequest that will make the API similar when relevant. Maybe something similar can be done with FetchEvent, but straight up using the Fetch API won't.

[lucacasonato]

@samuelgozi This is incorrect. Please read https://developer.mozilla.org/en-US/docs/Web/API/FetchEvent. There is precedent for this approach in the web platform service worker and on platforms like Cloudflare Workers.

For example on Fetch you can do response.json() because the response is incoming, but in the server that is irrelevant.

For fetch you can also do request.json() (https://developer.mozilla.org/en-US/docs/Web/API/Request) which is irrelevant for the client. So what you are saying is not correct.

[samuelgozi]

@lucacasonato I didn't disagree on the FetchEvent.
I disagreed on Request/Response. You are right there is .json() on both, which was a bad example.

But what about: Response.trailers, Response.ok,Response.url, Response.FinalUrl , Request.redirect(possible but not reliable), Request.destination.

I'm just trying to understand, but don't feel obligated to explain.
I'll look in the standard to see if there is anything about it.
I do remember that in service workers you can kind of act like a server so maybe it can work...

[nayeemrmn]

This would slightly change the semantics of using Server, but it would reduce complexity and we would provide an interface that is web compatible and familiar to many web devs.

If needed, we could (should?) use a Deno.extension symbol to hide Deno-specific stuff in it.

Example:

console.log(request); // Request { ... }
console.log(request[Deno.extension].conn); // Deno.Conn { ... }

[lucacasonato]

@samuelgozi But what about: Response.trailers, Response.ok,Response.url, Response.FinalUrl , Request.redirect(possible but not reliable), Request.destination.

The user manually creates this response using new Response() so they would be populated by the response constructor - which is already standardized.

For the request they should just equal to what service workers do (). I would have to check the spec what exactly they do with these properties (Request.destination would be "", and Request.redirect would be "manual")

@nayeemrmn: If needed, we could (should?) use a Deno.symbols.extension to hide Deno-specific stuff in it.

I agree, but the Deno.Conn would have to live on the FetchEvent rather than the request, because you respond by doing event.respondWith(response) rather than request.respond(response)

[lucacasonato]

So I just looked at this, and we can't implement it properly without moving the entire http handler into cli, because FetchEvent is part of global scope.

If we decide to switch to a rust based http server to increase perf we can use this approach for a possible Deno.listenHTTP. Until then a FetchEvent based approach is not really possible.

Something that would be possible is just not using FetchEvent, and instead defining our own version of fetch event (maybe called RequestContext), that still uses the fetch Request and Response. In that case Server would be an AsyncIterable<RequestContext>, with RequestContext having a request property and a respondWith() method.

[samuelgozi]

Something that would be possible is just not using FetchEvent, and instead defining our own version of fetch event (maybe called RequestContext), that still uses the fetch Request and Response. In that case Server would be an AsyncIterable<RequestContext>, with RequestContext having a request property and a respondWith() method.

I'm coding a server for an app, and I ended up doing something very similar to this(literally right now). Not exactly following the fetch standard, but it does make sense to warp the request and response in a class with methods like respondWith on them.

[ry]

Yes I'm in favor of this.

https://googlechrome.github.io/samples/service-worker/basic/

[lucacasonato]

So I did some more testing with this and it looks pretty doable actually. Have a POC based on the current std/http here: https://github.com/lucacasonato/deno-fetchevent. I can implement it in std without too much trouble I think.

The only somewhat annoying thing is that this requires that we add a FetchEvent in cli (that throws on construction because we can't implement it in cli). This is fine though, Chrome also sorta half crashes (or hangs) when trying to construct one in a service worker manually. There is no reason for a end user to construct a FetchEvent anyway. We would implement the actual working copy of FetchEvent in std.

We should also fix the Response constructor to match spec before this change happens. This should become really simple once toReader from denoland/deno#5789 lands.

[nayeemrmn]

Raising this here because the design of the eventual built-in HTTP server is discussed in this issue.

I think we should go with a custom RequestContext over FetchEvent after all. FetchEvent inherits from Event and has all of these useless properties tied to EventTarget::addEventListener() and friends. We'd be using an async iterator API so I don't know know if we can correctly populate those -- at best it's really strange (and unprecedented?). I also can't see anything in FetchEvent other than request and respondWith() that makes sense in a true backend.

[fusionstrings]

I am looking forward for FetchEvent implementation for compatibility with browser APIs.

[kt3k]

Closing as https://github.com/denoland/deno_std/discussions/1034 has more detailed suggestions.