servant-client: Support adding trailing slashes to API requests

[iand675]

I'm attempting to use servant-client to communicate with an API that expects all routes to end with a trailing slash. On GET requests, they return a redirect status code, so the client ends up at the proper API endpoint eventually. However, on POST/PUT/DELETE, they just return Method Not Allowed. It would be great if there was a way to configure servant to output optionally output trailing slashes.

A more general idea might be to introduce a function to ClientM that modifies each request prior to sending or something along those lines.

[jkarni]

Would managerModifyRequest from http-client suffice?

[declension]

Hmm, interesting - my problem (whilst using servant-quickcheck at least) seems to be the exact opposite...

[bumbleblym]

URLs with and without a trailing slash are not the same; shouldn't servant be able to support that?

[seanparsons]

@jkarni Modifying the path didn't seem to work for me, I have it logging out the request and I can see the trailing slash there. But it triggers a second request on the endpoint I'm testing against, which oddly adds another trailing slash, and then it's downhill from there.

[mikatammi]

I'm encountering this same issue. The API expects to have a trailing slash, and forwarding causes two requests to be generated, which is a problem because we would like to minimize the daily request count to the endpoint.

I'm wondering is this issue still open

[cdimitroulas]

Would managerModifyRequest from http-client suffice?

~~Is this a potential workaround for now? Does someone have an example code snippet of how this could work?~~

Edit: I got it to work with this code for creating the http client manager:

-- Ensure that we don't add multiple trailing slashes sinc managerModifyRequest can run multiple times
addTrailingSlash :: BS.ByteString -> BS.ByteString
addTrailingSlash s = case C.last s of
                       '/' -> s
                       _   -> C.snoc s '/'

mkManager :: IO Manager
mkManager = newManager tlsManagerSettings {managerModifyRequest = addSlash}
  where
    addSlash :: Request -> IO Request
    addSlash req = return $  req { path = addTrailingSlash (path req) }

[stevladimir]

The easiest way is to use makeClientRequest. managerModifyRequest should also work, but the former seems to be more "high-level". In particular, it allows to easily add trailing slash only to particular endpoints or even call points.

  -- I did not check if this compiles, but I think idea is clear
  let env = mkClientEnv manager baseUrl
       addTrailingSlash e@ClientEnv{makeClientRequest = defReq} =
         e{mkClientRequest = \url -> (\req{path} -> req{path = path <> "/") . defReq url}
  local addTrailingSlash $ runClientM someEndpoint env

The problem with this approach is that API definition goes out of sync with actual behavior. But conceptually, adding trailing slash is just adding an empty path fragment, so an alternative is

data Unit = Unit
instance ToHttpApiData Unit where
  toUrlPiece Unit = ""

type Endpoint = ... :> Capture "trailing" Unit :> Post ...

-- The downside is that one has to pass an extra Unit argument to client functions.
-- One can use Text instead of Unit and "" as an extra argument. 

To get rid of extra argument one can define new combinator instead of Capture "trailing" Unit

type Endpoint = ... :> TrailingSlash :> Post ...

data TrailingSlash

instance (HasLink sub) => HasLink (TrailingSlash :> sub) where
  type MkLink (TrailingSlash :> sub) a = MkLink sub a
  toLink toA _ l = toLink toA (trailingSlash @sub) l ""

instance (HasClient m sub) => HasClient m (TrailingSlash :> sub) where
  type Client m (TrailingSlash :> sub) = Client m sub
  clientWithRoute pm _ req = clientWithRoute pm (trailingSlash @sub) req ""
  hoistClientMonad pm _ = hoistClientMonad pm (Proxy @sub)

-- Just a shortcut
trailingSlash :: Proxy (Capture "trailing_slash" Text :> sub)
trailingSlash = Proxy

[cdimitroulas]

Thanks for those additional options. It's a bit frustrating that this extra code is needed just to support this use-case, though I don't have the knowledge or Haskell skills to dig into this library to see if it can be supported directly in the library.

In my case, I'm interacting with a public third-party API which has all the URLs ending with a slash, meaning that if I don't write this extra code to handle it then it doesn't work (I get 302 redirects otherwise)

[stevladimir]

In my case, I'm interacting with a public third-party API which has all the URLs ending with a slash, meaning that if I don't write this extra code to handle it then it doesn't work (I get 302 redirects otherwise)

Am I right that you are writing Servant definitions on yourself?

makeClientRequest or managerModifyRequest seems to be a simplest way (unless you care about API description correctness, e.g. you wanna have working Swagger UI or generate docs). If all endpoints have trailing slash you can slightly re-write snippet above let env = addTrailingSlash $ mkClientEnv manager baseUrl and just pass env around.

It's a bit frustrating that this extra code is needed just to support this use-case, though I don't have the knowledge or Haskell skills to dig into this library to see if it can be supported directly in the library.

IMO

The problem is a somewhat poor design of API you are using. The thing is that /foo/ and /foo are NOT the same thing. AFAIK URL path design is rooting from UNIX file path, so /foo/ was intended to be sort of a directory listing available resources and /foo an individual resource. And on a server side it would be served as simple file server. So when one opened /foo/ one would seen list of "files" available. Thus, using POST /foo/ for a single resource is not quite how it was designed to be. Though this would probably make sense for things like POST /user/ for creating a new user and PUT /user/ for updating all users, but not for a GET /user/:uid/. But that's all is more a matter of aesthetics. What is more important is that when you call /foo/ you are calling ["foo", ""] not ["foo"], and in spirit of Servant to reflect this in API type. I see no technical limitations to add TrailingSlash combinator to Servant (but I'm not a maintainer or contributor!). There are some concerns: e.g., whether to allow TrailingSlash :> Capture "foo" :> ..., but that's a technical detail. The question is whether it is a popular enough use case to be worth to bloat Servant. It could be a separate package, but overhead costs... One will need to maintain 3 packages (api, client, server) for such a minor "leftpad" thing. Maybe a section in docs could be a good compromise.

[cdimitroulas]

Appreciate your thoughts. I agree it's an odd choice to have the trailing slashes in the API design but that's something outside of my control - though as you say it's related to directories in which case a GET /my-things/ URL could make semantic sense.

This case feels a bit too specific to be added as a unique combinator in Servant, so perhaps a section in the docs/wiki would work best.

[mikatammi]

To get rid of extra argument one can define new combinator instead of Capture "trailing" Unit

type Endpoint = ... :> TrailingSlash :> Post ...

data TrailingSlash

instance (HasLink sub) => HasLink (TrailingSlash :> sub) where
  type MkLink (TrailingSlash :> sub) a = MkLink sub a
  toLink toA _ l = toLink toA (trailingSlash @sub) l ""

instance (HasClient m sub) => HasClient m (TrailingSlash :> sub) where
  type Client m (TrailingSlash :> sub) = Client m sub
  clientWithRoute pm _ req = clientWithRoute pm (trailingSlash @sub) req ""
  hoistClientMonad pm _ = hoistClientMonad pm (Proxy @sub)

-- Just a shortcut
trailingSlash :: Proxy (Capture "trailing_slash" Text :> sub)
trailingSlash = Proxy

This is nice. I added the TrailingSlash to my API, and now my mock server implementation which uses the same API-type doesn't compile because of the missing HasServer instance. What kind of implementation is required on the server side?

[stevladimir]

This is nice. I added the TrailingSlash to my API, and now my mock server implementation which uses the same API-type doesn't compile because of the missing HasServer instance. What kind of implementation is required on the server side?

Here it is a self-contained example (minus LANGUAGE extensions as I have bunch of them enabled in cabal)

module TrailingSlash where

import Data.Proxy
import Servant.API
import Servant.Client
import Servant.Server
import Servant.Server.Internal.ErrorFormatter


data EmptyPiece = EmptyPiece

instance ToHttpApiData EmptyPiece where
  toUrlPiece EmptyPiece = ""

instance FromHttpApiData EmptyPiece where
  parseUrlPiece = \case
    "" -> Right EmptyPiece
    _ -> Left "Expected empty path piece"

data TrailingSlash

instance (HasLink sub) => HasLink (TrailingSlash :> sub) where
  type MkLink (TrailingSlash :> sub) a = MkLink sub a
  toLink toA _ l = toLink toA (trailingSlash @sub) l EmptyPiece

instance (HasClient m sub) => HasClient m (TrailingSlash :> sub) where
  type Client m (TrailingSlash :> sub) = Client m sub
  clientWithRoute pm _ req = clientWithRoute pm (trailingSlash @sub) req EmptyPiece
  hoistClientMonad pm _ = hoistClientMonad pm (Proxy @sub)

instance
  ( HasServer sub ctx
  , HasContextEntry (MkContextWithErrorFormatter ctx) ErrorFormatters ) =>
  HasServer (TrailingSlash :> sub) ctx where

  type ServerT (TrailingSlash :> sub) m = ServerT sub m

  route _ ctx d = route (trailingSlash @sub) ctx $ const <$> d

  hoistServerWithContext _ = hoistServerWithContext (Proxy @sub)

-- Just a shortcut
trailingSlash :: Proxy (Capture "trailing_slash" EmptyPiece :> sub)
trailingSlash = Proxy

[mikatammi]

data EmptyPiece = EmptyPiece

instance ToHttpApiData EmptyPiece where toUrlPiece EmptyPiece = ""

instance FromHttpApiData EmptyPiece where parseUrlPiece = \case "" -> Right EmptyPiece _ -> Left "Expected empty path piece"

instance ( HasServer sub ctx , HasContextEntry (MkContextWithErrorFormatter ctx) ErrorFormatters ) => HasServer (TrailingSlash :> sub) ctx where

type ServerT (TrailingSlash :> sub) m = ServerT sub m

route _ ctx d = route (trailingSlash @sub) ctx $ const <$> d

hoistServerWithContext _ = hoistServerWithContext (Proxy @sub)

This example of the server end needs a little tweaking, it seems now that: /endpoint// returns the desired result /endpoint/something/ returns the "Expected empty path piece" error /endpoint/ returns error 404

[mikatammi]

I guess the reason why the above example works on the client side, but not on the server side, is this: https://github.com/haskell-servant/servant/blob/2323906080e50fc2774cd1b43bc59548a90152ed/servant-client-core/src/Servant/Client/Core/BaseUrl.hs#L135 parseBaseUrl function used in the client strips the second trailng slash away

[stevladimir]

First, I should say that I did not test the snippet above, so some flaws are possible.

I don't think parseBaseUrl is used for routing. IIUC it is used for getting hostname and path prefix, but not for parsing path piece.

Could you provide an example of your API? The behavior you've described looks weird. Also would be nice to tcpick/wireshark/... request to see what is actually sent by client

[mikatammi]

I don't think parseBaseUrl is used for routing. IIUC it is used for getting hostname and path prefix, but not for parsing path piece.

You're right, I even went to the lengths to test this by removing the removeTrailingSlash-part and building the project against locally modified version, and it didn't affect anything in this case.

Could you provide an example of your API? The behavior you've described looks weird.

-- Trailing slash only needed for endpoint1 type API = "endpoint1" :> TrailingSlash :> Header "api-key" APIKey :> Get '[JSON] EndPoint1Data :<|> "endpoint2" :> Header "api-key" APIKey :> Capture "id" Text :> Get '[JSON] EndPoint2Data

Also would be nice to tcpick/wireshark/... request to see what is actually sent by client

% nc -l 8081 GET /endpoint1/? HTTP/1.1 Host: localhost:8081 Accept-Encoding: gzip Accept: application/json;charset=utf-8,application/json

I'm using the version from Stackage LTS 19.25. However I saw similar behaviour when building against the servant's latest git master version

[mikatammi]

It is probably worth noting that servant-server seems to treat URI's with and without the trailing slash as the same thing. If we could define the HasServer instance in a way that it is a no-op, that would probably solve the issue