CrowdHailer / raxx

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

An option for middleware that fits in the existing Raxx.Server behaviour #141

Closed CrowdHailer closed 5 years ago

CrowdHailer commented 5 years ago

I'm not opposed to a middleware stack behaving like a server, see https://elixir-lang.slack.com/archives/C56H3TBH8/p1538471405000100 (edited) nietaki [10:32 AM] But what I thought the main goal of specifying middlewares in the core project was specifying an interface/approach where people can write their middlewares to a relatively simple interface and assume they're perfectly reusable between projects. The 141 PR does the exact opposite in my mind:

I didn't think anyone argued it was impossible. In my mind the challenge was to devise a flexible and standardised way of doing it that all Raxx-based projects would rely on. #141 does the opposite, it basically says: you don't need middlewares, just put servers inside your servers and do everything by hand. As a bright note, after all this shit-talking:I think we can have it both ways: a stack of middlewares being indistinguishable from a server AND support for writing and re-using those middlewares.

from slack

CrowdHailer commented 5 years ago

@nietaki So I now have answers to your comments above.

doesn't assist in writing new middlewares at all, it basically says "you can wrap a server in another server and make it act like a middleware, but you have to do it all on your own and it's not standardised in any way, so don't even think about attaching them to a router"

I understand your point here, though I would counter it as follows. What I have tried to show is that an effective middleware system can be implemented without adding many extra parts. That doesn't limit us from providing as much guidance and standard practice as we like. A page on middleware can say standard middleware always have a function called wrap that takes two arguments and wraps state in a tuple with state for the middleware in first element and rest of pipeline in the second.

adds a ton confusion with things like def handle_data(data, next), do: Raxx.Server.handle_data(next, data)

This is the point I'm most confused by. Your PR has pretty much the sameline https://github.com/CrowdHailer/raxx/pull/139/files/9cdd6ca61c8d6a8b042947725556be381d8c34d1#diff-dcbad65e70250f550c18562b533ff7bcR28 Are you saying the confusion is because it's called on Raxx.Server and calling it on Raxx.Pipeline makes it clearer what you are doing?

(minor nit) re-invents a linked list by putting tuples inside tuples

See next comments crowdhailer [8:57 AM] Does the following updates address your concerns at all? I have stayed with the idea of not introducing a new behaviour. But providing more structure/help for middleware authors

  1. Stacking is always done by the same function {HomePage, "Hello"} |> Middleware.wrap(BasicAuth, [username: "foo", password: "bar"]) it can wrap in a tuple or prepend to a list, the idea is the shape of how next in the stack is stored is not something that you need to know
  2. Provide a helper to pull out the config from state
  3. Namespace the function to call next middleware (that looks just like a server)

  {s, next} = Middleware.pop(state)
  # some middleware stuff
  Middleware.handle_head(next, request)
  # some more middleware stuff
end```

**from slack**