feat(ext/flash): split upgradeHttp into two APIs (#15557)

This commit splits `Deno.upgradeHttp` into two different APIs, because
the same API is currently overloaded with two different functions. Flash
requests upgrade immediately, with no need to return a `Response`
object. Instead you have to manually write the response to the socket.
Hyper requests only upgrade once a `Response` object has been sent.

These two behaviours are now split into `Deno.upgradeHttp` and
`Deno.upgradeHttpRaw`. The latter is flash only. The former only
supports hyper requests at the moment, but can be updated to support
flash in the future.

Additionally this removes `void | Promise<void>` as valid return types
for the handler function. If one wants to use `Deno.upgradeHttpRaw`,
they will have to type cast the handler signature - the signature is
meant for the 99.99%, and should not be complicated for the 0.01% that
use `Deno.upgradeHttpRaw()`.

cli/bench/testdata/deno_upgrade_http.js

@@ -1,8 +1,8 @@
-const { serve, upgradeHttp } = Deno;
+const { serve, upgradeHttpRaw } = Deno;
 const u8 = Deno.core.encode("HTTP/1.1 101 Switching Protocols\r\n\r\n");
 
 async function handler(req) {
-  const [conn, _firstPacket] = upgradeHttp(req);
+  const [conn, _firstPacket] = upgradeHttpRaw(req);
   await conn.write(u8);
   await conn.close();
 }

cli/dts/lib.deno.unstable.d.ts

@@ -1344,25 +1344,51 @@ declare namespace Deno {
     options: ServeInit & (ServeOptions | ServeTlsOptions),
   ): Promise<void>;
 
-  /** **UNSTABLE**: new API, yet to be vetter.
+  /** **UNSTABLE**: new API, yet to be vetted.
    *
-   * Allows to "hijack" a connection that the request is associated with.
+   * Allows "hijacking" the connection that the request is associated with.
-   * Can be used to implement protocols that build on top of HTTP (eg.
+   * This can be used to implement protocols that build on top of HTTP (eg.
    * WebSockets).
    *
-   * The return type depends if `request` is coming from `Deno.serve()` API
-   * or `Deno.serveHttp()`; for former it returns the connection and first
-   * packet, for latter it returns a promise.
-   *
    * The returned promise returns underlying connection and first packet
    * received. The promise shouldn't be awaited before responding to the
    * `request`, otherwise event loop might deadlock.
    *
+   * ```ts
+   * function handler(req: Request): Response {
+   *   Deno.upgradeHttp(req).then(([conn, firstPacket]) => {
+   *     // ...
+   *   });
+   *   return new Response(null, { status: 101 });
+   * }
+   * ```
+   *
+   * This method can only be called on requests originating the `Deno.serveHttp`
+   * server.
+   *
    * @category HTTP Server
    */
   export function upgradeHttp(
     request: Request,
-  ): [Deno.Conn, Uint8Array] | Promise<[Deno.Conn, Uint8Array]>;
+  ): Promise<[Deno.Conn, Uint8Array]>;
+
+  /** **UNSTABLE**: new API, yet to be vetted.
+   *
+   * Allows "hijacking" the connection that the request is associated with.
+   * This can be used to implement protocols that build on top of HTTP (eg.
+   * WebSockets).
+
+   * Unlike `Deno.upgradeHttp` this function does not require that you respond
+   * to the request with a `Response` object. Instead this function returns
+   * the underlying connection and first packet received immediately, and then
+   * the caller is responsible for writing the response to the connection.
+   *
+   * This method can only be called on requests originating the `Deno.serve`
+   * server.
+   *
+   * @category HTTP Server
+   */
+  export function upgradeHttpRaw(request: Request): [Deno.Conn, Uint8Array];
 
   /** @category Sub Process */
   export interface SpawnOptions {

cli/tests/unit/flash_test.ts

@@ -1002,14 +1002,14 @@ Deno.test(
   },
 );
 
-Deno.test("upgradeHttp tcp", async () => {
+Deno.test("upgradeHttpRaw tcp", async () => {
   const promise = deferred();
   const listeningPromise = deferred();
   const promise2 = deferred();
   const ac = new AbortController();
   const signal = ac.signal;
   const handler = async (req: Request) => {
-    const [conn, _] = await Deno.upgradeHttp(req);
+    const [conn, _] = Deno.upgradeHttpRaw(req);
 
     await conn.write(
       new TextEncoder().encode("HTTP/1.1 101 Switching Protocols\r\n\r\n"),
@@ -1028,6 +1028,8 @@ Deno.test("upgradeHttp tcp", async () => {
     conn.close();
   };
   const server = Deno.serve({
+    // NOTE: `as any` is used to bypass type checking for the return value
+    // of the handler.
     handler: handler as any,
     port: 4501,
     signal,

ext/flash/01_http.js

@@ -3,7 +3,9 @@
 
 ((window) => {
   const { BlobPrototype } = window.__bootstrap.file;
-  const { fromFlashRequest, toInnerResponse } = window.__bootstrap.fetch;
+  const { TcpConn } = window.__bootstrap.net;
+  const { fromFlashRequest, toInnerResponse, _flash } =
+    window.__bootstrap.fetch;
   const core = window.Deno.core;
   const {
     ReadableStream,
@@ -18,7 +20,6 @@
     _readyState,
     _eventLoop,
     _protocol,
-    _server,
     _idleTimeoutDuration,
     _idleTimeoutTimeout,
     _serverHandleIdleTimeout,
@@ -597,7 +598,28 @@
     });
   }
 
+  function upgradeHttpRaw(req) {
+    if (!req[_flash]) {
+      throw new TypeError(
+        "Non-flash requests can not be upgraded with `upgradeHttpRaw`. Use `upgradeHttp` instead.",
+      );
+    }
+
+    // NOTE(bartlomieju):
+    // Access these fields so they are cached on `req` object, otherwise
+    // they wouldn't be available after the connection gets upgraded.
+    req.url;
+    req.method;
+    req.headers;
+
+    const { serverId, streamRid } = req[_flash];
+    const connRid = core.ops.op_flash_upgrade_http(streamRid, serverId);
+    // TODO(@littledivy): return already read first packet too.
+    return [new TcpConn(connRid), new Uint8Array()];
+  }
+
   window.__bootstrap.flash = {
     serve,
+    upgradeHttpRaw,
   };
 })(this);

ext/http/01_http.js

@@ -477,17 +477,9 @@
 
   function upgradeHttp(req) {
     if (req[_flash]) {
-      // NOTE(bartlomieju):
+      throw new TypeError(
-      // Access these fields so they are cached on `req` object, otherwise
+        "Flash requests can not be upgraded with `upgradeHttp`. Use `upgradeHttpRaw` instead.",
-      // they wouldn't be available after the connection gets upgraded.
+      );
-      req.url;
-      req.method;
-      req.headers;
-
-      const { serverId, streamRid } = req[_flash];
-      const connRid = core.ops.op_flash_upgrade_http(streamRid, serverId);
-      // TODO(@littledivy): return already read first packet too.
-      return [new TcpConn(connRid), new Uint8Array()];
     }
 
     req[_deferred] = new Deferred();

runtime/js/90_deno_ns.js

@@ -109,7 +109,6 @@
     serveHttp: __bootstrap.http.serveHttp,
     resolveDns: __bootstrap.net.resolveDns,
     upgradeWebSocket: __bootstrap.http.upgradeWebSocket,
-    upgradeHttp: __bootstrap.http.upgradeHttp,
     kill: __bootstrap.process.kill,
     addSignalListener: __bootstrap.signals.addSignalListener,
     removeSignalListener: __bootstrap.signals.removeSignalListener,
@@ -154,5 +153,7 @@
     spawn: __bootstrap.spawn.spawn,
     spawnSync: __bootstrap.spawn.spawnSync,
     serve: __bootstrap.flash.serve,
+    upgradeHttp: __bootstrap.http.upgradeHttp,
+    upgradeHttpRaw: __bootstrap.flash.upgradeHttpRaw,
   };
 })(this);