joy-framework / joy

A full stack web framework written in janet
https://joy.swlkr.com
MIT License
537 stars 30 forks source link

404 routes help #60

Closed cpjolicoeur closed 4 years ago

cpjolicoeur commented 4 years ago

I'm a bit lost on how routing is supposed to work for unhandled routes. I've read through the source and I'm not sure the idiomatic or proper way to have 404 Not Found routes. Ever request for an undefined route currently results in a server error for me.

I'm not sure how this line here would ever pass through for an unfound route. When the first let statement of route (find-route routes request) returns nil, the 2nd let statement of [route-method route-uri route-fn] route will surely fail 100% of the time and throw a server error, which in turn returns a 500 Status Code to the client.

I feel like I'm missing something obvious here, but how/where/when are un-declared routes handled in Joy?

swlkr commented 4 years ago

Oh wow this just happened, sorry about that, I pushed a few changes for wildcard params and didn't have a test for 404s 🤦‍♂️

You wren't missing anything obvious, this was totally my fault. I pushed a change to revert back to using when-let on that line you mentioned and 404s should work now.

Sorry if it drove you a little mad there 😅

swlkr commented 4 years ago

Closed with 3cde681c01e30014a38fa6e68bb2e1026daee700

swlkr commented 4 years ago

I closed this, but you know what, 404s should be handled with this middleware here:

https://github.com/joy-framework/joy/blob/master/src/joy/middleware.janet#L158-L165

swlkr commented 4 years ago

So before I close this again, let me know if that works out for you now with the latest master

swlkr commented 4 years ago

Oh and here's a really basic one file example of some new joy features that I haven't really released yet:

(use joy)

(defn / [request]
  (text/plain "welcome to joy"))

(defn not-found [request]
  (text/plain "this is not the page you were looking for"))

(def app (app {:404 not-found}))

(server app 9001)

This should work with the latest master

cpjolicoeur commented 4 years ago

Thanks, I'll try latest master again. I did see the not-found middleware, and have it in my pipeline, but the request process failed, of course, before even getting that far. I'll try latest master again as you suggested and follow-up. Thanks for the quick response.

cpjolicoeur commented 4 years ago

Ok, can confirm current master handles not found routes as expected (per the docs) when using the not-found middleware now (and returns the expected "Internal Server Error" when the not-found middleware is not used.

Thanks again.