theplant / containers

Better structures for building web applications
1 stars 0 forks source link

Return redirect error to redirect, and can customize handle normal error #20

Closed sunfmin closed 7 years ago

sunfmin commented 7 years ago

@JadenTeng 搞定。

bodhi commented 7 years ago

@sunfmin you want to allow specific containers to redirect the whole request? Why isn't the top-level HTTP handler taking care of this? Containers should just be for rendering data, no?

sunfmin commented 7 years ago

A lots of specific logic depends on the data that get inside from the container. If we do it in middleware, We have to get those data in the middleware from the beginning. and duplicate that logic inside container.. Or pass it in. If we passing data into containers, Then container is no difference with gorazor templates...

bodhi commented 7 years ago

Ok, but why can't an application specific page handler handle this case?

The problem I see is that there's no way to know from the signature of a container whether the container could ask to redirect instead of rendering. This means you need to wrap every handler with a redirection error handler.

It also will prevent streaming containers, you are forced to render everything and keep it as a string, before you can send it back to the browser. This isn't a major problem, but something to consider.

Also, can you explain why the middleware is needed?

sunfmin commented 7 years ago

I mean middleware to the same as Handler... We now don't have handler for each URL, they are sharing the same one now.

bodhi commented 7 years ago

Hold your horses! Why'd you merge this?

bodhi commented 7 years ago

Is the context necessary? Can you just pass an error handler in when you set up the initial page handlers?

sunfmin commented 7 years ago

We can have a conversation after aigle launch. But right now we need this for aigle to launch.

bodhi commented 7 years ago

But right now we need this for aigle to launch.

Next time please lock to your PR branch instead of master in this case.