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

"Deno.serve()" - Better UX for 103 Early Hints on HTTP server #15827

Open
Tracked by #17146
ayame113 opened this issue Sep 9, 2022 · 2 comments
Open
Tracked by #17146

"Deno.serve()" - Better UX for 103 Early Hints on HTTP server #15827

ayame113 opened this issue Sep 9, 2022 · 2 comments
Labels
ext/http related to ext/http suggestion suggestions for new features (yet to be agreed)

Comments

@ayame113
Copy link
Contributor

ayame113 commented Sep 9, 2022

Edit: In practice Chrome only supports 103 Early Hints over HTTP/2, so I realized there is no point in sending Early Hints with the current flash server. I would appreciate it if you could introduce this in the future when HTTP/2 can be used with Deno.serve().

https://chromium.googlesource.com/chromium/src/+/master/docs/early-hints.md#what_s-not-supported


Flash server can return 103 Early Hints response. It's great, but I wish there was a better UX.

// the way i currently do
import { writeAll } from "https://deno.land/[email protected]/streams/mod.ts";

const encoder = new TextEncoder();
const ok = (body: string) => {
  const { byteLength } = encoder.encode(body);
  const text = [
    "HTTP/1.1 103 Early Hints",
    "Link: </style.css>; rel=preload",
    "Link: </script.js>; rel=preload",
    "",
    "",
    "HTTP/1.1 200 OK",
    "Content-Type: text/plain",
    `Content-Length: ${byteLength}`,
    "",
    body,
    "",
  ].map((str) => `${str}\r\n`).join("");
  return encoder.encode(text);
};

const handler = (async (req: Request) => {
  const [conn, _firstPacket] = Deno.upgradeHttpRaw(req);

  const res = ok("hello");
  await writeAll(conn, res);

  conn.close();
}) as unknown as Deno.ServeHandler;

Deno.serve(handler);

I think it would be great if we could write something like the following instead of the less versatile style above. (And it should also improve performance.)

Deno.serve((req, ctx) => {
  ctx.respondInformationalResponse(
    new Response(null, {
      status: 103,
      headers: {...},
    }),
  );
  return new Response("hello");
});

For your reference:

@littledivy littledivy added suggestion suggestions for new features (yet to be agreed) flash labels Sep 9, 2022
@ayame113 ayame113 changed the title Better UX for 103 Early Hints on flash server Better UX for 103 Early Hints on HTTP server Sep 18, 2022
@bartlomieju
Copy link
Member

@ayame113 using cs.github.com I noticed you are the only user of Deno.upgradeHttpRaw. This API is gonna get removed in the next minor release of Deno (though it will start throwing and error in the next patch release). Deno.serve() got rewritten to use Deno.serveHttp() in 2846bbe and I believe this feature would currently not be supported. I'm happy to explore this and hope @mmastrac could chime in

@bartlomieju bartlomieju removed the flash label Apr 3, 2023
@bartlomieju bartlomieju changed the title Better UX for 103 Early Hints on HTTP server "Deno.serve()" - Better UX for 103 Early Hints on HTTP server Apr 3, 2023
@ayame113
Copy link
Contributor Author

ayame113 commented Apr 4, 2023

using cs.github.com I noticed you are the only user of Deno.upgradeHttpRaw.

Thank you for letting me know about that!! As I wrote in the comment above, I found that 103 Early Hints cannot be sent with the combination of Deno.upgradeHttpRaw and HTTP/1.1. So I'm not using this anymore so removing this is fine for me.

@mmastrac mmastrac added the ext/http related to ext/http label Nov 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ext/http related to ext/http suggestion suggestions for new features (yet to be agreed)
Projects
None yet
Development

No branches or pull requests

4 participants