uNetworking/uWebSockets.js

Consider queueing headers on `HttpResponse.setHeader` call

lukeed opened this issue · 3 comments

Hey! 👋 Long time no see, hope all is well :)

It took me a little while to track this down, but I eventually realized that the first call to res.setHeader automatically set the outgoing response status.

I was under the assumption that setHeader worked like Node, in that I could write into some kind of ~queued dictionary that only was finalized once writeHead (which doesnt exist here) or the first .write()/.end() method.

An example use case is that I have a CORS middleware that sets the baseline CORS headers on all requests. It was then up to other handler(s) to determine the body, addl headers, and if this was going to be a 2xx or 4xx+ response.

By "queueing" the headers, it would also allow other functions to append/modify existing WIP header values (eg, "Vary", which I was handling incorrectly, intentionally, initially).

// before
function cors(req: HttpRequest, res: HttpResponse): void {
  res.setHeader('Access-Control-Allow-Origin', origin);
  res.setHeader("Access-Control-Allow-Credentials", "true");
  res.setHeader("Access-Control-Expose-Headers", "Authorization");
  res.setHeader("Vary", "Origin");
}

// after (workaround), written elsewhere
function cors(req: HttpRequest, res: HttpResponse): void {
  let headers: Headers = (res.headers ||= new Headers);

  headers.set('Access-Control-Allow-Origin', origin);
  headers.set("Access-Control-Allow-Credentials", "true");
  headers.set("Access-Control-Expose-Headers", "Authorization");
  headers.append("Vary", "Origin");
}

Hey man, that's unfortunately against the design - there's only writeHeader & writeStatus and both directly write the response so if you want a layer between you can add it above. That's what all the Express-like wrappers do (not a fan)

Yeah I figured as much :) didn’t know if you had played with the idea already and measured perf loss.

I’ll close this but perhaps there should be mention of this in the docs? I can add it if you want. I didn’t see but maybe I missed

I have the same issue during building a trpc-uwebsockets thing, I think seeing this in the doc can clarify for others