EvandroLG / pegasus.lua

:rocket: Pegasus.lua is an http server to work with web applications written in Lua language.
http://evandrolg.github.io/pegasus.lua/
MIT License
421 stars 36 forks source link

`req:statusCode` is poorly documented and extremley difficult to use #141

Closed Frityet closed 8 months ago

Frityet commented 8 months ago

I still do not know how to properly use this function.

Here is my code


server:start(function (req, res)
    local _<close> =
    defer(function ()
        res:close()
    end)
    local path = src_dir/req:path()
    if path:type() == "directory" then path = path/"index.html.lua" end
    if not path:exists() then path = path:add_extension("html.lua") end
    if not path:exists() then path = path:remove_extension():add_extension("html") end

    if not path:exists() then
        path = build_dir/req:path()
        if not path:exists() then
            path = build_dir/"luajs"/req:path() --luajs.data doesn't have a subdir so this has to be done like this
        end
    end

    if path:exists() then
        log.info("200 OK: ", tostring(path))
        local ext = assert(path:extension(), "File has no extension")
        if ext ~= "html.lua" then
            local data, err = path:read_all()
            if data then
                res:addHeader("Content-Type", match(ext) {
                    css = "text/css",
                    js = "application/javascript",
                    wasm = "application/wasm",
                    default = "text/plain"
                })
                res:addHeader("Content-Length", #data)
                res:write(data)
                res:statusCode(200, "OK")
            else show_server_error(err, res) end

            return
        end

        res:addHeader("Content-Type", "text/html")
        local page, err = dohtml(path)
        if page then
            local s = html_document(page)
            res:addHeader("Content-Length", #s)
            res:write(s)
        else show_server_error(err, res) end
    else
        log.warning("404 Not Found: ", tostring(path))
        local doc = html_document(not_found)
        res:addHeader("Content-Type", "text/html")
        res:addHeader("Content-Length", #doc)
        res:write(doc)
        res:statusCode(404, "Not Found")
    end
end)

I currently have errors when a request runs because it "was already sent", but I can find no docs on when things are actually sent, if possible documenting this would be nice

achou11 commented 8 months ago

calling Response.statusCode() sets the status code for the response that's eventually sent. You're probably getting an error because you're calling res:write() (which sends the response headers) right before res:statusCode()

I suspect it should be written with the statusCode() call first, then the method that actually sends the response (similar to the spec)

Example:

res:statusCode(404, "Not Found")
res:write(doc)
Frityet commented 8 months ago

calling Response.statusCode() sets the status code for the response that's eventually sent. You're probably getting an error because you're calling res:write() (which sends the response headers) right before res:statusCode()

I suspect it should be written with the statusCode() call first, then the method that actually sends the response (similar to the spec)

Example:

res:statusCode(404, "Not Found")
res:write(doc)

Right, that makes sense, is reading the spec the only good way to learn about cases like this?

achou11 commented 8 months ago

Right, that makes sense, is reading the spec the only good way to learn about cases like this?

I sometimes find it to be an effective way to figure out how to use something that isn't fully documented :) I'm sure the maintainers would be open to PRs or suggestions for docs on this though!

To be honest, I've never used this library 😅 (I mostly follow it out of interest and I appreciate its simplicity/readability) - I just found your issue interesting and tractable, and was able to follow the source code to understand what the implementation does.

EvandroLG commented 8 months ago

Thanks, @achou11 for helping with this issue. Was about to jump into it, but I see you already supported @Frityet :)

I know our docs are not the best at this point, so feel free to open PRs to improve it guys

Closing the PR, but please re-open it if it's needed @Frityet

Frityet commented 8 months ago

Thank you! Alongside adding annotations with #140 I will try and add documentation as well