Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • 02:04
    benjumanji closed #505
  • 02:04
    benjumanji commented #505
  • Nov 29 20:05
    benjumanji opened #505
  • Nov 29 14:57
    msrd0 closed #504
  • Nov 29 14:57
    msrd0 commented #504
  • Nov 29 14:56

    msrd0 on master

    Remove DummyError from websocke… (compare)

  • Nov 29 14:52
    codecov[bot] commented #504
  • Nov 29 14:40

    msrd0 on master

    Implement Into<Response> for Te… (compare)

  • Nov 29 14:40
    msrd0 closed #503
  • Nov 29 14:32
    codecov[bot] commented #504
  • Nov 29 14:32
    FSMaxB synchronize #504
  • Nov 29 14:25
    codecov[bot] commented #503
  • Nov 29 14:20
    codecov[bot] commented #504
  • Nov 29 14:07
    FSMaxB commented #503
  • Nov 29 14:07
    codecov[bot] commented #503
  • Nov 29 14:06
    FSMaxB synchronize #503
  • Nov 29 14:03
    msrd0 labeled #503
  • Nov 29 14:03
    msrd0 labeled #503
  • Nov 29 14:03
    msrd0 milestoned #503
  • Nov 29 14:03
    msrd0 labeled #504
Isaac Whitfield
@whitfin
Yeah there’s a lot to it that’s difficult to unravel. I looked before to see how we could peel some stuff away but came up empty.
Judson Lester
@nyarly
I'm still trying to figure out why we need MiddlewareChain and Pipelines
The latter seem to do with putting things into a BorrowBag.
And there's some cleverness so that an FnOnce can be used to process multiple requests.
Judson Lester
@nyarly
I wonder if BorrowBag should be reviewed. Basically builds linked lists out of pairs. Not sure if we need to look at it in terms of perf
Judson Lester
@nyarly
As I've been reviewing how pipelines are used, I'm really not sure why they are this way, but it'd be a breaking change to completely "promote" pipelines to surround routers.
msrd0
@msrd0_gitlab
what do you mean with "surround routers"?
Judson Lester
@nyarly
There's a reasonable case that pipelines (roughly speaking) should surround all NewHandlers, including Routers (that would be a new feature), and that the pipelines provided to build_router are the "default" pipelines for their routes.
My prejudice from Rack and PEP333 is that every request would start processing by entering a pipeline. At the bottom of that pipeline is a (New)Handler, which converts the request to a response (well, a result), which flows back up the the pipeline.
A router being the most signficant NewHandler in that scenario.
msrd0
@msrd0_gitlab
makes sense but middlewares currently only run after the router and I believe that makes sense for most pipelines, so we should probably keep that the default
Judson Lester
@nyarly
Intuitively, the Middleware that make up a Pipeline should themselves be NewHandlers, so that you've got a referential transparency (kinda) on them.
I'm not sure I agree with "makes sense for most pipelines"
We had a feature request issue a while ago that I'm coming to understand better: it's impossible e.g. to handle a 404 in the router this way.
msrd0
@msrd0_gitlab
well, for my use case at least, I don't need to check session cookies for 404 responses
Judson Lester
@nyarly
Except with a whole separate "response finalizer" concept, which feels like a weird patch.
One case that I'd love to build: a server for SPAs - so most 404s should get rebuilt as a request to /
The simplest fix here, I think, is just "decorate this NewHandler with this Pipeline" - which could apply then to anything.
And the current behavior would be "decorate all the routes of this router"
msrd0
@msrd0_gitlab
I'm not saying I don't want a way to handle routing errors, just saying that middlewares, the way they are now, mostly do stuff for successful routes
Judson Lester
@nyarly
And what I'm saying is that there's a lot of very successful prior art around using them to do stuff for all requests - successful, not, whatever. The existence of the ResponseFInalizer is really weird from that perspective.
msrd0
@msrd0_gitlab
agreed, response finalizer is a weird thing that I didn't even know existed
is that basically a complicated route.on_error(StatusCode::NOT_FOUND, |state| /* build response */)?
msrd0
@msrd0_gitlab
oh, if you know that it's called finalizer there's actually an example that explains it :)
Judson Lester
@nyarly
Right, but it could be a Middleware, dig?
msrd0
@msrd0_gitlab
yeah could be, but I'm not sure if we shouldn't have a handler generate custom error pages instead of a middleware
since middleware's (at least right now and in my use case) usually modify the state before the handler or the response after the handler, but don't build responses
Judson Lester
@nyarly
Coming from the PEP333 perspective a) no reason a middleware can't modify, even wholesale substitute the response and b) no reason a middleware can't divert processing to another handler.
In Rack, the router is just a middleware - it chooses the next processor - which is often an app (handler analogue)
Denis Lisov
@tanriol
So a Middleware is just a Handler that delegates to other handler(s)?
Judson Lester
@nyarly
From the PEP333 perspective, yes. A middleware accepts a Request and returns a Response (roughly.) That can be the "bottom" application, or it can be a middleware, which is constructed to wrap another middleware.
Very flexible compositions can result.
I've gotten somewhere on understanding pipelines-as-implemented.
(btw)
A) specifically, the pipelines provided to a router are effectively the "default" pipelines for each route. They'll be wrapped around anything that uses one of the request family of route drawing methods.
This is different in behavior to being a wrapper around the whole router because there are several methods that allow you to route _without_pipelines
Judson Lester
@nyarly
That all makes some sense to me. If it were my design, I'd wrap things in pipelines explicitly, and there'd be an easier way to wrap the top level router. I'm not sure I can make a stronger argument for that than "aesthetics."
The pipeline stuff gets a little more complicated - there's these PipelineHandleChains which are BorrowBags so that we can ensure the lifetimes of the data in the pipeline definitions.
BorrowBags exist to make a collection of disparate things share a (longer) lifetime, right?
Where it gets weird is that a PLHC is a collection of Pipelines, which are themselves collections of Middleware,
(well... there's some construct going on in there, and Pilelines -> PipelineInstances and NewMiddlewareChain -> MiddlewareChain but that confuses the issue)
When a pipeline gets called during a request, we instantiate what amounts to a list of lists, and then do a depth first walk with the (state, request) pair, and then walk back with the result.
Judson Lester
@nyarly
I mean really, it's all function composition, not list walking, and the weirdest part (which I'm still trying to get my head around) is how PipelineHandleChains compose with MiddlewareChains since both convert a cadr-style list into function composition, with (state,request) running down the side.
Finally, I don't understand why it has to be, but the MiddlewareChain itself is declared unsafe
So my primary research question now is: could the PLHC refer directly to Middleware - or some new middleware-type that's available to be implemented directly, but there's an adapter for old style Middleware
Judson Lester
@nyarly
Step one being: could that produce a related composition?
Judson Lester
@nyarly
IIRC, many of these types aren't public - Middleware is, and PLHC is. But I wonder if we could simplify there and not break anything.
Judson Lester
@nyarly
Okay, for sure pipelinechains of middlewarechains are equivalent to a single MWchain composed by concatenation.
So I think it should be refactorable.
Unless anyone understands different?