http.request api should be improved

[johnd0e]

Sinks (as well as filters) require response metadata to properly process data. There's a significant difference between receiving a 200 or 404 status code, or whether the Content-Type is application/json, text/html, or text/event-stream.

Without this metadata, I’m forced to rely on unreliable heuristics in the sink/filter code, which is far from ideal.

It's unfortunate because the response metadata is already retrieved but simply isn't accessible within the sink.

[alerque]

Contributions welcome! I don't have much development time for this right now but would be happy to step in as a maintainer role to facilitate merging PRs if somebody tackles this.

[johnd0e]

Well, at least I have an idea for a backward-compatible improvement:

When a sink is not specified, the first value returned by http.request could be more than just 1. It could be a function that retrieves the body, with the sink passed as an argument.

[bassklampfe]

I'm having similar issues. I'd prefer not to change the API that much. A simple callback between shouldredirect and shouldrecevicebody fullfills my need.

*** http-orig.lua   2024-09-15 10:18:23.000000000 +0200
--- http.lua    2024-09-15 10:22:48.242167029 +0200
***************
*** 336,347 ****
--- 336,353 ----
      -- at this point we should have a honest reply from the server
      -- we can't redirect if we already used the source, so we report the error
      if shouldredirect(nreqt, code, headers) and not nreqt.source then
          h:close()
          return tredirect(reqt, headers.location)
      end
+ 
+   -- check if headers
+   if nreqt.processheaders then
+       nreqt.processheaders(headers)
+   end
+ 
      -- here we are finally done
      if shouldreceivebody(nreqt, code) then
          h:receivebody(headers, nreqt.sink, nreqt.step)
      end
      h:close()
      return 1, code, headers, status

My code then looks like this

    :::      
    local content_length
    local function processheaders(hdr)
        content_length=assert(tonumber(hdr["content-length"]),"no content-length in header")
    end
    local request=
    {
        method = "GET",
        url = url,
        processheaders = processheaders,
        sink = sink,
    }
    :::
``

[johnd0e]

@alerque

Contributions welcome!

May I please request that my PR be reviewed?