http4s/rho

Getting 405 Method Not Allowed from combined routes

Opened this issue · 3 comments

When I combine these routes and hit the second route, it returns 405 Method Not Allowed. I don't know if this is intended or a bug but I would prefer to have it aligned with http4s library.

    val routes1 = new RhoRoutes[IO] {
      GET / "path" |>> { () => Ok("good") }
    }.toRoutes()

    val routes2 = new RhoRoutes[IO] {
      POST / "path" |>> { () => Ok("good") }
    }.toRoutes()

    val combinedRoutes = routes1 <+> routes2

    val req = Request[IO](Method.POST, Uri(path = "/path"))
    val response = combinedRoutes.orNotFound.run(req).unsafeRunSync()
    response.status shouldBe Status.Ok   // fails because it returns MethodNotFound instead

This is a bit of a tricky one. We have 2 choice based on how the frameworks work.

  1. Let routes1 return None if the route matches but the method isn't found.
    • Pros:
      • The above example works and gives a 200 OK
      • This is the same as Http4s DSL
    • Cons: If there really are no other methods for that route, it would give a 404 Not Found instead of 405 Method Not Allowed
  2. Let routes return a Some(MethodNotAllowed) if the route matches but the method isn't found
    • Pros:
      • You can actually get a 405 Method Not Allowed
      • Follows the web standards I should think...
    • Cons:
      • The above isn't clear to users of row why routes2 is ignored.
      • It's different than Http4s DSL

After some thought, I'm inclined to leave the functionally as is. Rho was not designed for you to convert 2 sets of RhoRoutes to Http4s routes and then compose them using Http4s DSL <+> you really should compose all RhoRoutes together FIRST, and then convert them to Http4s.

Unless you have some edge case where you intend to create separate swagger documents within a single application, there shouldn't be any issues here.

I'd be curious if @rossabaker has any thoughts on this?

I've hit this same issue.
The scenario is similar to the above, except that the POST request requires auth. e.g.

object Auth extends org.http4s.rho.AuthedContext[IO, AuthInfo]

val routes1 = new RhoRoutes[IO] {
  GET / "path" |>> { () => Ok("good") }
}

val routes2 = new RhoRoutes[IO] {
  POST / "path" >>> Auth.auth() |>> { (_: AuthInfo) => Ok("good") }
}

val combinedRoutes = routes1.toRoutes() <+> routes2.toRoutes()

In order to auth, routes2 needs to be wrapped in authMiddleware. However, if I compose the RhoRoutes first, then routes1 will also be wrapped in middleware and then return a 401 when no header is supplied.

Whilst it's probably not technically correct, any thoughts on including a param to enable copying the http4s behaviour if desired?