obelisk icon indicating copy to clipboard operation
obelisk copied to clipboard

handling for unknown (404 not found) routes

Open dimsmol opened this issue 3 years ago • 1 comments

Obelisk version: v0.9.1.0 I'm running the default app created with ob init and built as described in the readme.

For the most of the unknown routes (e.g. http://localhost:8000/zzz) it responds with 200 OK and empty content.

An exception to this are routes starting with paths serving static content (e.g. http://localhost:8000/static/zzz). Those respond with 404 Not found and HTML content showing text like No handler accepted "/static/zzz", however, content-type header is missing.

Both cases are kind of weird:

  • Request to non-existing route shouldn't be responded with 200 OK
  • If HTML is served back, it should have proper content-type header

But more generally:

  • There should be an easy way to specify handler(s) for "not found" routes
  • The default should be a properly served 404 Not found page, the same for all "not found" routes regardless the path prefix

dimsmol avatar Dec 23 '21 06:12 dimsmol

In my project, I've managed to add my own "not found" handler that renders a page in a normal way. I did it by adding that "not found" page on the frontend and making backend serve it for BackendRoute_Missing.

Adding it on the frontend is a bit weird but it allows testing it with ob run without making additional code changes related to that specific path. Also, it saves some code I would need for emulating non-existing frontend route to reuse existing rendering function.

Serving a "not found" page with JS for the whole app may look weird as well. Not using the app's all.js or using some other JS instead is pretty easy, this gives us the following alternatives:

  • Use all.js as usual - a bit weird but the simplest non-static choice
  • Make the "not found" page fully static (or at least not use Reflex for it) - pretty limiting
  • Compile a separate frontend app just to use it for the "not found" page - there is no easy way to do this and sharing some routes with that different app would likely be non-trivial
  • Make a lightweight chunk for "not found" page but allow to load the rest of the app on demand - probably the best solution but we don't have chunking yet (see #883)

For now I use the first option.

Implementation-wise, I've ended up with the following code:

-- | Mimics @serveGhcjsApp@ from 'Obelisk.Backend'.
serveMissing :: (MonadSnap m, HasCookies m) => m ()
serveMissing = do
  modifyResponse $ setResponseStatus 404 "Not Found"
  modifyResponse $ setContentType staticRenderContentType
  modifyResponse $ setHeader "Cache-Control" "no-store private"

  html <- renderGhcjsFrontend' $ FrontendRoute_NotFound :/ ()
  writeBS html

-- | Like @renderGhcjsFrontend@ from 'Obelisk.Backend' but pre-parametrized with the values
-- 'Obelisk.Backend.runBackendWith' usually passes to it (via the chain of function calls).
renderGhcjsFrontend' :: (MonadIO m, HasCookies m) => R FrontendRoute -> m ByteString
renderGhcjsFrontend' route = do
  -- this chunk of code is mostly copied from Obelisk.Backend.runBackendWith
  publicConfigs <- liftIO getPublicConfigs
  -- validFullEncoder is already calculated in runBackendWith but we cannot access it
  validFullEncoder <- case checkEncoder fullRouteEncoder of
    Left e -> fail $ "Invalid fullRouteEncoder: " <> unpack e
    Right validFullEncoder -> pure validFullEncoder
  let routeToUrl (k :/ v) =
        renderObeliskRoute validFullEncoder $ FullRoute_Frontend (ObeliskRoute_App k) :/ v
  let allJsUrl = renderAllJsPath validFullEncoder
  let ghcjsWidgets = ($ allJsUrl) <$> defaultGhcjsWidgets

  -- this chunk of code is mostly copied from Obelisk.Backend.renderGhcjsFrontend
  cookies <- askCookies
  renderFrontendHtml
    publicConfigs
    cookies
    routeToUrl
    route
    frontend
    (_ghcjsWidgets_preload ghcjsWidgets)
    (_ghcjsWidgets_script ghcjsWidgets)

This implementation has the following issues:

  • It's mostly a copy-paste from renderGhcjsFrontend, serveGhcjsApp, and runBackendWith. Probably not a big deal but:
    • It took some time to trace the code path between runBackendWith and renderGhcjsFrontend (it has several intermediate functions) and put these pieces together
    • It would be nice to guarantee this to stay in sync with how runBackendWith works, currently the implementations can easily diverge
  • The bottom piece of code is basically renderGhcjsFrontend, I had to copy-paste it just because it's not exported
  • checkEncoder is already called up the stack but we need to call it again since we don't have access to the result, we also re-create routeToUrl and ghcjsWidgets
    • Probably not a big deal if that stuff is not expensive

Some other caveats:

  • The page is bound to the single "not found" frontend route no matter what path is actually used - not a big deal, just a thing to keep in mind
  • While "not found" frontend route can be used as usual with ob run, the page served on any other missing path won't work as expected in that environment since it mimics runBackendWith and not ob run-specific code

Also, serving BackendRoute_Missing doesn't cover missing pages under static/ and ghcjs/ since they are served separately. As I understand, there is no way to override 404 page for these paths.

As a conclusion, it would be nice to:

  • Have more readily available building blocks to serve frontend pages from backend routes the way runBackendWith does
    • Also, support this better in ob run so we could serve those pages in ob run-compatible way
      • Maybe also find a way to not have "not found" page explicitly listed on frontend, though may not be worth the effort
    • Allow for the lightweight 404 page JS payload with help of chunking, though that's a totally separate topic, see #883
  • Allow overriding 404 page under static-serving paths as well

dimsmol avatar Dec 29 '21 08:12 dimsmol