Unable to combine two directory servers.

[marcotoniut]

First one fails with 404 before making an attempt on the second directory server.

server :: String -> String -> Server (Raw :<|> Raw)
server x y = serveDirectoryFileServer x
          :<|> serveDirectoryFileServer y

[alpmestan]

Hmm, I think this is because Raw applications "always succeed" in a sense. Because we don't have anything to distinguish between a failure (4xx) being the expected result or a sign that we should try what's next, if this wasn't the last route among a few that we can chose from. Should we just handle 404 in a specific way to have this program succeed? cc @phadej @kosmikus @jkarni

[alpmestan]

@kosmikus and I discussed this a bit, it seems like adding a dedicated file-serving constructor to Router' would allow this example to work as intended by @marcotoniut. The argument is that given how we must treat Raw applications (we're basically handing them the request and they can respond any way they want, we don't know anything about what they do so we just forward whatever response they give us to the client), file-serving deserves a special case, because we know a lot more about file serving than we do about arbitrary wai Applications. In particular, if a serveDirectoryXXX function returns 404, it just signals that it could not find what we're asking for and that we might want to try other handlers that match, if any (in the example above, the second serveDirectory).

[fizruk]

I don't think servant should specialise for static file servers.

I think rather than upgrading Router' we can simply provide a combinator like this one:

-- | Fallback to the second application when the first one fails.
--
-- Failure is determined with HTTP Status predicate.
fallbackOn
  :: (Status -> Bool)
  -> Application
  -> Application
  -> Application
fallbackOn p app1 app2 req respond = app1 req $ \response ->
  -- check response from app1
  if p (responseStatus response)
    then app2 req respond  -- if response status satisfies predicate — fallback to app2
    else respond response  -- otherwise use response from app1

Moreover I would suggest adding something like this directly to wai.

For servant I can see a possibility for combinator extending :<|>. Perhaps something like

-- | Fallback to the second API whenever the first one responds
-- with any of the provided "failed" statuses.
--
-- Servers will fallback to another (set of) handlers.
--
-- Clients can try a set of API endpoints
-- (whenever parameters and response types match for both APIs).
--
-- Fallback mechanism can also be reflected in documentation.
data FallbackOn (statuses :: [Nat]) api1 api2

-- | Combine APIs without extra fallback mechanisms.
type (:<|>) = FallbackOn '[]

The main problem with FallbackOn is that ideally you'd want arbitrary predicate (possibly not only for response status but any Response -> Bool). Or even a validation function like Response -> [Error]. This is impossible to do at the type-level, so a term-level combinator like fallbackOn is still necessary.

[fizruk]

@marcotoniut by the way, can you give us more context for your use case?
What are your actual file servers? Do their contents overlap? Can you instead to something like this? (I'm using "images" and "videos" to demonstrate the idea since I don't know your domain)

type API
    = "static" :> "images" :> Raw
 :<|> "static" :> "videos" :> Raw

server :: Server API
server
    = serveDirectoryFileServer "/opt/static/uploads/images"
 :<|> serveDirectoryFileServer "/opt/static/generated/videos"

Here images will be served under /static/images and videos — under /static/videos.
Thus file server will be picked according to the URL prefix.

[fizruk]

@alpmestan related idea: issue a TypeError (with a separate API validating combinator) whenever we see Raw :<|> Raw (well, it's a bit more complicated, but you get the point). I think there was a related issue regarding path shadowing/overlapping in an API type, but I can't find it :(

[phadej]

If we gonna make servant-lint (which e.g. mentions about slashes in static path pieces), it could also warn about overlapping Raw endpoints. IMO they make little sense (in fact you'd like them to always be last in the "subgroup").

[alpmestan]

Hmm maybe we can get away without a constructor, but just a function, indeed @fizruk.

While we're throwing TODO items around, we should document the router's behaviour better, especially in the tutorial. =)

[marcotoniut]

@fizruk I could do with having specific domains for each of the file types, but I thought to mention this anyway. Should an Application in the chain throw a 404, wouldn't it make sense for servant to see if there is something else remaining before responding to the request? From reading other issues, I thought that was indeed the case.

[alpmestan]

Well, for this specific case of file-serving, yes it would make sense. In general, I'm not sure. It worked like this early on and was the reason for tons of bugs.