CrowdHailer / raxx

Interface for HTTP webservers, frameworks and clients
https://hexdocs.pm/raxx
Apache License 2.0
402 stars 29 forks source link

Feature Proposal: Raxx.Context #156

Closed nietaki closed 5 years ago

nietaki commented 5 years ago

The current interface for Raxx.Server and Raxx.Middleware works pretty well, but still has some limitations.

Currently the information that the Middleware/Server modules work off of is the module's state (which by definition is not shared between modules) and the Request/Response head, data chunk and tail data, which have a mostly static structure and are transient - those get handed off through the stack once and are gone while the request is still being handled.

That being said, there are some things that don't really fit within this model. More specifically, I think anything that depends on passing arbitrary information between the http server and the Raxx.Stack or the individual stack elements for the duration of the whole request doesn't have a good place.

This is a proposal to introduce Raxx.Context - common data to be shared/passed between the HTTP(1|S|2) server and all Raxx.Stack elements.

Example Use-Cases

There's probably more than the ones I'm listing and it would be good to come up with a design that will accomodate all these and most of the related ones that we can't think of right now.

Standard server information

There's a couple of things that the HTTP server knows, but the Raxx handlers don't. That includes:

Functionality added by Middlewares

Middlewares are already useful, but their utility is limited to bypassing the server and modifying the request/response parts. What can't be done cleanly is using the middlewares to do some work and pass some information down the stack. This could be:

Parameter binding

This could be included in the previous point, but I think it's interesting and relevant enough to be separated.

The current router implementations just direct the request to a relevant controller, but don't do what many other routers in the wild do, which is binding named parameters from the path (and maybe other places) into some special params collection.

It's not practical to do it right now, as there's no good place for this information in the Request object.

Another note might be that when the controller is handling the data/tail and the Request is no longer around, it still might need some route params. Without parameter binding, those would need to go into the controller state.

The above is true for other use cases too - without some shared data throughout the request, many things need to go into controller/middleware state.

Protocol upgrade stuff (?)

This idea isn't really fleshed out. I'm curious if the context would be helpful with things like enabling a Raxx app serve websocket requests for example.

Implementation considerations

Introducing Raxx.Context doesn't necessarily require us to change the Raxx.Server and the Raxx.Middleware interfaces - we could keep the context in the Process Dictionary and provide functions to retrieve/save/modify it. This would make it backwards compatible.

I'm not sure it's the right way to go though. I think the context should be something explicit and part of an easily testable interface. Having it in the process dictionary would make it much less practical to pattern match on it in function heads, which would be especially relevant for the parameter binding use-case.

Also, all Raxx interface was nice and pure so far, I'm not sure if there's good enough reason to do it right now. I can also imagine situations where some of the code needing to interact with the context isn't in the same process and there would still be a need to pass it explicitly.

I'm not a 100% certain though, I could be persuaded otherwise.

Next steps

Agree if it's something we want, what we want it to achieve and how we want the context to be structured, and what API we want for it, both for accessors/mutators and where it meets the Raxx.Server and Raxx.Middleware

The context structure is an interesting topic in my opinion. The context could in theory be one big assigns-style map with some conventions, or some struct with more structure, but still a lot of flexibility. I'd be curious to see what the best way to cut it is.

As always, I'm very interested in hearing your thoughts.

CrowdHailer commented 5 years ago

I'm of the opinion that it is something we definitely want to do. Having middleware without this just makes middleware unviable. The reason being that something like an authentication middleware definitely requires being able to pass information to the controller.

That said I think in most applications one or two standard middleware will be all that's required. i.e. I don't think the average project should be defining many/any middleware.

The protocol upgrade stuff is interesting but I don't have anything useful to add. I'm not even sure it's worth giving much thought until someone tries implementing websockets or similar (if similar even exists, Is there anything like websockets but not websockets?)

My current goal is to go through the list of middleware that it would be nice to have (it exists on the raxx_kit proposals board) use the Process dictionary as need be and see what falls out.

The way I see it there are two questions

I am leaning to process dictionary because if a middleware doesn't need to do anything with the context. For example imagine the following stack

Raxx.Session
MyNewMiddleware
Raxx.Flash

The flash middleware will need a few things from the session middleware, and the session middleware will need to know what was added/deleted by the flash middleware in the response. Using the process dictionary allows MyNewMiddleware to be oblivious to the fact a context even exists

nietaki commented 5 years ago

I thought about it some more and I am starting to believe putting it in the process dictionary is the way to go. Adding it to the behaviours would complicate them a fair bit, without much benefit in most situations.

You're right, probably not much point trying to fit in protocol upgrade at this point.

The implementation should be simple enough, the bulk of the task is going to be providing a good API for it. I think it makes sense for me to build up a PR with some working implementation and we can discuss the interface based on it.

tsloughter commented 5 years ago

The problem with the process dictionary I've hit with context is propagating to another process when needed.

For now I've simply gone with providing both options, process dictionary context or variable context. If you use the pdict context and want to propagate you must get the current context into a variable and pass it as part of a message.

Personally in the case of middlewares like this I'd go with a variable. We've discussed doing this in Elli and it is the way I do it in grpcbox.