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

Callback-Based Response Retrieval #23

Open
MikuAuahDark opened this issue Jul 2, 2023 · 2 comments
Open

Callback-Based Response Retrieval #23

MikuAuahDark opened this issue Jul 2, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@MikuAuahDark
Copy link
Contributor

MikuAuahDark commented Jul 2, 2023

One of the limitation in lua-https is that it's not possible for application to create download progress bars on it (if the Content-Length header is available). This is because https.request will wait until all the response data is retrieved.

My proposal is to add optional 3rd and 4th parameter callback and context. Callback is defined as these fields:

local callback = {
    -- `context` in here means user-supplied data (4th-parameter), which can be anything.
    response = function(context, status, headers)
        -- Called when HTTP status code and headers has been retrieved. Called once.
        -- `status` is HTTP status code
        -- `headers` is HTTP headers in **all lowercase** for the keys.
    end,
    body = function(context, data)
        -- Called when new HTTP body data arrives. Called multiple times as new data arrives.
        -- `data` is response data as Lua string.
        -- Return "falsy" value and additional error message (default is "aborted") to abort the request
        return true -- success
        return false, "aborted" -- failure
    end,
    complete = function(context, err)
        -- Called when data has been transferred, or an error occurs.
        -- `err` will be the error message, or `nil` if everything worked as intended.
    end,
}

Example usage:

local data = {current = 0, length = 0}

https.request("https://love2d.org", {}, {
    -- `context` in here means user-supplied data (4th-parameter), which can be anything.
    response = function(context, status, headers)
        print("HTTP status code", status)
        if headers["content-length"] then
            context.length = tonumber(headers["content-length"]) or 0
        end
    end,
    body = function(context, data)
        context.current = context.current + #data
        if context.length > 0 then
            print(string.format("Received data %d bytes (%.2f)", #data, context.current * 100 / context.length))
        else
            print(string.format("Received data %d bytes", #data))
        end
    end,
    complete = function(context, err)
        if err then
            print("Download error", err)
        else
            print("Download completed!")
        end
    end,
}, data)

As an additional feature/requirement, the callback table can be a class object (not instance) by retrieving the function list using Lua C function that invokes metamethod and the context can be the class instance itself such that when a class is declared like this:

local MyDownloader = class "MyDownloader"

function MyDownloader:response(status, headers)
end

function MyDownloader:body(data)
    return true
end

function MyDownloader:complete(err)
end

Doing something like this simply works and calls the appropriate functions.

local dlinstance = MyDownloader() -- new class instance
https.request("https://love2d.org", {}, MyDownloader, dlinstance)

The return value of https.request when using this variant would be questionable.

  • Returning the status code, body, and the headers will preserve backward compatibility, but at same time make things redundant (the library has to keep everything until the function returns instead of discarding them).
  • Returning only true or false + error message (assert-compatible style) means less redundancy, but breaks backward compatibility.

I probably can do this, but the question becomes "should this is be done?" and/or "is this valid usecase to justify the feature addition?".

@semyon422
Copy link

Is it possible to make it api-compatible with luasocket like luasec and lapis do? I think using a single API will be more convenient and clearer than creating a new one.

@MikuAuahDark
Copy link
Contributor Author

MikuAuahDark commented Jul 4, 2023

No, it's not possible to do so.

The intention of lua-https is clear: to make HTTP(S) request using OS-provided or popular library API. If lua-https is modified to have similar API as LuaSec then you're looking for TLS alone. Furthermore, if OS decided to support future HTTP version such as HTTP3 in the future, lua-https doesn't require any change to the API. This is already true for the recent WinINet backend (in Windows) and cURL backend. It will try to use HTTP2 automatically if the OS and the site supports it and fallback to HTTP 1.1 otherwise.

Meanwhile if you're using LuaSec-style API, then implementing HTTP2 is difficult, if not impossible. You most likely stuck in HTTP 1.1 protocol.

@MikuAuahDark MikuAuahDark added the enhancement New feature or request label Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants