agrafix / Spock

Another Haskell web framework for rapid development
https://www.spock.li
679 stars 56 forks source link

How to return error on mismatch of route param #172

Closed cideM closed 2 years ago

cideM commented 3 years ago

This is something that many Haskell route libraries are plagued with, I think.

I modified the snippet from the hello world project, so that it requires an Int:

    get ("hello" <//> var) $ \(name :: Int) ->
      do
        (DummyAppState ref) <- getState
        visitorNumber <- liftIO $ atomicModifyIORef' ref $ \i -> (i + 1, i + 1)
        text ("Hello " <> T.pack (show name) <> ", you are visitor number " <> T.pack (show visitorNumber))

http://localhost:8080/hello/foo -> 404 http://localhost:8080/hello/2 -> 200

I think that there's no universally correct answer for what should happen in the /hello/foo case. But I'd like to have the option of returning an error message to my users. In this case it's likely that someone wants to access the hello/* route, but didn't remember the accepted parameters. I would like to return a 400 Bad Request in this case.

Right now the only way I can imagine this would work is to always use Text and then manually cast types. But then it's pretty much like WAI pattern matching in the style of ["hello", var] -> case readEither var of.

It would be nice if that behavior was at least documented :book: would you accept a PR for that?

~On a somewhat related note, most type safe route libs in Haskell seem to be similar in what they achieve. What would be really cool was if route matching would be checked for totality. In this case the compiler should then warn that I'm missing patterns for var.~

I think what would work quite well would be an exception that you can catch like so:

get ("hello" <//> var)` $ fn `catchStuff` sendA400
agrafix commented 3 years ago

Ah this is an interesting observation! I think you could probably implement a similar behavior using a custom type that you could throw on, e.g.:

data MaybeInt = MiInt Int | MiOther

instance FromHttpApiData MaybeInt of
   parseUrlPiece x =
         case parseUrlPiece x of
            Left _ -> Other
            Right i -> MiInt i

get ("hello" <//> var) $ \(name :: MaybeInt) ->
      case name of
        MiInt i -> ok i
        MiOther -> throw ...

But I'm happy to add this natively to the library too. One option is to introduce an alternative combinator to var. The other option would be to make this globally configurable. I think the alternative combinator is probably the best route?

cideM commented 3 years ago

I think that does indeed sound very nice

agrafix commented 3 years ago

👍 Cool feel free to send a PR!