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

NotFound Handler #20

Closed
saj1th opened this issue Mar 5, 2016 · 9 comments
Closed

NotFound Handler #20

saj1th opened this issue Mar 5, 2016 · 9 comments

Comments

@saj1th
Copy link
Member

saj1th commented Mar 5, 2016

The "notFound Handler" was a neat convenience in zenazn/goji

https://github.com/goji/goji/blob/master/router_trie.go doesn't seem to have a similar concept - probably because you decided to make less assumptions in goji2

I'm wondering what would be the best way to implement a custom 404 handler with goji2

thanks for new libs

@zenazn
Copy link
Member

zenazn commented Mar 5, 2016

Since routing is now performed before the middleware stack, you can now do this yourself! Just take a peek at the matched Handler (by calling https://godoc.org/goji.io/middleware#Handler), and if it's nil, no route was matched, and a 404 should be served.

@saj1th
Copy link
Member Author

saj1th commented Mar 5, 2016

Cool. thanks!

@saj1th saj1th closed this as completed Mar 5, 2016
@alexguan
Copy link

This does not work well when you have SubMix. For example:

rootMux := goji.NewMux()
v1Mux :=goji.SubMux()

rootMux.HandleC(pat.New("/v1/*"), v1Mux)
v1Mux.HandleC(pat.Get("/foo"), FooHandler)

Then there seems no way to return a customized 404 for /v1/bar since middleware.Handler will return a non-nil handler (which is v1Mux);

Any ideas?

@saj1th
Copy link
Member Author

saj1th commented Mar 15, 2016

@alexguan

looks like the hacky way around the problem is to apply the 404 middleware to your sub muxes as well

rootMux := goji.NewMux()
v1Mux :=goji.SubMux()
rootMux.UseC(middleware.Apply404)
v1Mux.UseC(middleware.Apply404)

rootMux.HandleC(pat.New("/v1/*"), v1Mux)
v1Mux.HandleC(pat.Get("/foo"), FooHandler)

Like you mentioned, while

if h == nil {
would get a nil handler,
if h == nil {
would not get nil as a *goji.Mux would be present.

Let me know if you found a better way to tackle this?

@zenazn thoughts?

@alexguan
Copy link

@saj1th The workaround is rather ugly since you need to apply the middleware to all sub muxes. It seems the right way to do it is to enhance Mux to provide a hook so we can supply a function to override the default NotFound behavior.

@zenazn Any other suggestions?

@zenazn
Copy link
Member

zenazn commented Mar 15, 2016

You can write an abstraction to create new Mux's with a NotFound middleware attached, instead of using goji.NewMux. If you want to cast a wider net, you can also write a http.ResponseWriter implementation that catches 404 status codes and instead renders a different 404.

Trying to auto-detect SubMuxes and do something more clever sacrifices the principle of least surprise, however, since it means the two snippets below would behave differently—there is no way to determine if a given goji.Handler logically implements a SubMux:

rootMux := goji.NewMux()
v1Mux := goji.SubMux()

rootMux.HandleC(pat.New("/v1/*"), v1Mux)
v1Mux.HandleC(pat.Get("/foo"), FooHandler)
rootMux := goji.NewMux()
v1Mux := goji.SubMux()

shim := func(ctx context.Context, w http.ResponseWriter, r *http.Request) {
    v1Mux.ServeHTTPC(ctx, w, r)
}

rootMux.HandleC(pat.New("/v1/*"), goji.HandlerFunc(shim))
v1Mux.HandleC(pat.Get("/foo"), FooHandler)

@zenazn
Copy link
Member

zenazn commented Mar 15, 2016

For prior art on a http.ResponseWriter shim, I wrote a reasonable implementation as part of the previous version of Goji: https://godoc.org/github.com/zenazn/goji/web/mutil. The code is based entirely on the net/http interfaces, however, and can be reused with any mux of your choice, including this version of Goji.

@alexguan
Copy link

@zenazn I understand your concerns, however, it feels to me that such a basic need should be directly supported by the framework, instead of asking everyone to duplicate the work.

@zenazn
Copy link
Member

zenazn commented Mar 22, 2016

Even if Goji provided some kind of NotFound capability, it would have to be manually applied to every Mux, which doesn't seem like a win to me. Is there something I'm missing?

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

3 participants