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

Add functions for setting default custom handlers? #801

Closed
findmyway opened this issue Feb 14, 2022 · 6 comments
Closed

Add functions for setting default custom handlers? #801

findmyway opened this issue Feb 14, 2022 · 6 comments

Comments

@findmyway
Copy link

Similar to #608, is it possible to allow setting default custom handlers for both listen and serve?

@quinnj
Copy link
Member

quinnj commented May 25, 2022

Hi @findmyway, thanks for opening an issue. Can you explain a little more what your use-case is? Here's a few thoughts:

  • HTTP.listen is the lowest-level server function; it requires a "Stream" handler function and I don't really see us providing a default there; it's kind of meant as a "hey, only use this directly if you really know what you're doing" function, so I think it's fine making users provide the stream handler explicitly
  • I recently merged in an overhaul of the "handlers" interface/framework, which aims to both simplify and make things more flexible by the addition of an explicit middleware interface. We now have the concepts of 2 interfaces on the server side: Handler, which is a function of the form f(::Request) -> Response, and Middleware, which is a function of the form f(::Handler) -> Handler. A couple examples of middleware are here and here.

I think we could build up a few more of these convenient middlewares that are defined in the Handlers module. Once we have those, we could think about doing something similar to the client-side, where we have a HTTP.handlers(; kw...) like function that would take keyword args and "build up" the middleware + handlers by default, but for now, we just don't have enough to really need to stack things yet.

@findmyway
Copy link
Author

Hi @quinnj ,

I was planed to extract trace context from the headers and inject it into the current task local storage. So without a place to default custom handlers, users have to add such handler for every route explicitly. This may be error-prone.

@quinnj
Copy link
Member

quinnj commented May 25, 2022

The way I would handle this in the new framework (I know this is still just HTTP#master, but we're trying to get the new breaking release out soon), is to define a tracing middleware, something like:

function tracing_context_middleware(handler)
    function (req)
        if hasheader(req, "traceparent")
            req.context[:traceparent] = header(req, "traceparent")
        end
        if hasheader(req, "tracestate")
            req.context[:tracestate] = header(req, "tracestate")
        end
        return handler(req)
    end
end

We can then compose this with a Router and running our server by doing something like:

const router = HTTP.Router()
HTTP.register!(router, "GET", "/api/config/values")

HTTP.serve(tracing_context_middleware(router), "0.0.0.0", 8081)

So note that the HTTP.Router is a handler, in that you can call router(req) -> HTTP.Response, whereas the middleware we defined takes a handler and returns a handler (req -> resp) function.

I'm planning on overhauling all the docs soonish, so this might end up being an example of how to write a custom middleware and how it composes with handlers when setting up a server.

[EDIT]: and just to close the loop on your recent comment: note how every request will first pass through the tracing_context_middleware layer and the trace headers will be extracted and set in the (new) Request context before being passed to the HTTP.Router object to be routed to a specific route handler. Hence, instead of needing to wrap every route handler with this functionality, we've defined a middleware to run before the router.

@findmyway
Copy link
Author

Thanks for the detailed explaination!

Hence, instead of needing to wrap every route handler with this functionality, we've defined a middleware to run before the router.

I see, that's pretty concise.

@quinnj
Copy link
Member

quinnj commented May 25, 2022

Great! I'll close this issue for now, but watch for a big documentation update coming here soon to help walk through the new frameworks for HTTP.jl 1.0.

@quinnj quinnj closed this as completed May 25, 2022
@findmyway
Copy link
Author

Hi @quinnj ,

While looking into the latest code, I find the concept of MiddleWare is quite neat.

However, AFAIK, the default middleware of each Route is set to nothing. I'm wondering if it is possible to create a global middleware to be set for each Route. Just like REQUEST_LAYERS and STREAM_LAYERS. So that, third party packages can inject some default middlewares automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants