Return 404s when resources are not found.

This is implemented by making the return values, in most cases, idiosyncratic ad-hoc types that then convert to the approprate error response. This also should make endpoints more testable, since the return values can now be inspected to check their properties without having to process or parse an HTTP response.

4 files changed, 116 insertions, 33 deletions

diff --git a/src/channel/routes.rs b/src/channel/routes.rs
index 1379153..847e0b4 100644
--- a/src/channel/routes.rs
+++ b/src/channel/routes.rs
@@ -1,10 +1,12 @@
 use axum::{
     extract::{Form, Path, State},
-    response::{IntoResponse, Redirect},
+    http::StatusCode,
+    response::{IntoResponse, Redirect, Response},
     routing::post,
     Router,
 };
 
+use super::app::EventsError;
 use crate::{
     app::App,
     clock::RequestedAt,
@@ -44,10 +46,26 @@ async fn on_send(
     State(app): State<App>,
     login: Login,
     Form(form): Form<SendRequest>,
-) -> Result<impl IntoResponse, InternalError> {
+) -> Result<impl IntoResponse, ErrorResponse> {
     app.channels()
         .send(&login, &channel, &form.message, &sent_at)
-        .await?;
+        .await
+        // Could impl `From` here, but it's more code and this is used once.
+        .map_err(ErrorResponse)?;
 
     Ok(Redirect::to(&format!("/{}", channel)))
 }
+
+struct ErrorResponse(EventsError);
+
+impl IntoResponse for ErrorResponse {
+    fn into_response(self) -> Response {
+        let Self(error) = self;
+        match error {
+            not_found @ EventsError::ChannelNotFound(_) => {
+                (StatusCode::NOT_FOUND, not_found.to_string()).into_response()
+            }
+            EventsError::DatabaseError(error) => InternalError::from(error).into_response(),
+        }
+    }
+}
diff --git a/src/events.rs b/src/events.rs
index 5d2dcf0..fd73d63 100644
--- a/src/events.rs
+++ b/src/events.rs
@@ -1,15 +1,16 @@
 use axum::{
     extract::State,
+    http::StatusCode,
     response::{
         sse::{self, Sse},
-        IntoResponse,
+        IntoResponse, Response,
     },
     routing::get,
     Router,
 };
 use axum_extra::extract::Query;
-use chrono::{format::SecondsFormat, DateTime};
-use futures::stream::{self, StreamExt as _, TryStreamExt as _};
+use chrono::{self, format::SecondsFormat, DateTime};
+use futures::stream::{self, Stream, StreamExt as _, TryStreamExt as _};
 
 use crate::{
     app::App,
@@ -34,11 +35,13 @@ async fn on_events(
     _: Login, // requires auth, but doesn't actually care who you are
     last_event_id: Option<LastEventId>,
     Query(query): Query<EventsQuery>,
-) -> Result<impl IntoResponse, InternalError> {
+) -> Result<Events<impl Stream<Item = ChannelEvent<broadcast::Message>>>, ErrorResponse> {
     let resume_at = last_event_id
         .map(|LastEventId(header)| header)
         .map(|header| DateTime::parse_from_rfc3339(&header))
-        .transpose()?
+        .transpose()
+        // impl From would take more code; this is used once.
+        .map_err(ErrorResponse::LastEventIdError)?
         .map(|ts| ts.to_utc());
 
     let streams = stream::iter(query.channels)
@@ -55,12 +58,47 @@ async fn on_events(
             }
         })
         .try_collect::<Vec<_>>()
-        .await?;
+        .await
+        // impl From would take more code; this is used once.
+        .map_err(ErrorResponse::EventsError)?;
 
-    let stream = stream::select_all(streams).map(to_sse_event);
-    let sse = Sse::new(stream).keep_alive(sse::KeepAlive::default());
+    let stream = stream::select_all(streams);
 
-    Ok(sse)
+    Ok(Events(stream))
+}
+
+struct Events<S>(S);
+
+impl<S> IntoResponse for Events<S>
+where
+    S: Stream<Item = ChannelEvent<broadcast::Message>> + Send + 'static,
+{
+    fn into_response(self) -> Response {
+        let Self(stream) = self;
+        let stream = stream.map(to_sse_event);
+        Sse::new(stream)
+            .keep_alive(sse::KeepAlive::default())
+            .into_response()
+    }
+}
+
+enum ErrorResponse {
+    EventsError(EventsError),
+    LastEventIdError(chrono::ParseError),
+}
+
+impl IntoResponse for ErrorResponse {
+    fn into_response(self) -> Response {
+        match self {
+            Self::EventsError(not_found @ EventsError::ChannelNotFound(_)) => {
+                (StatusCode::NOT_FOUND, not_found.to_string()).into_response()
+            }
+            Self::EventsError(other) => InternalError::from(other).into_response(),
+            Self::LastEventIdError(other) => {
+                (StatusCode::BAD_REQUEST, other.to_string()).into_response()
+            }
+        }
+    }
 }
 
 fn to_sse_event(event: ChannelEvent<broadcast::Message>) -> Result<sse::Event, serde_json::Error> {
diff --git a/src/index/routes.rs b/src/index/routes.rs
index 32c7f12..37f6dc9 100644
--- a/src/index/routes.rs
+++ b/src/index/routes.rs
@@ -1,13 +1,13 @@
 use axum::{
     extract::{Path, State},
     http::{header, StatusCode},
-    response::IntoResponse,
+    response::{IntoResponse, Response},
     routing::get,
     Router,
 };
 use maud::Markup;
 
-use super::templates;
+use super::{app, templates};
 use crate::{
     app::App,
     error::InternalError,
@@ -49,11 +49,31 @@ async fn channel(
     State(app): State<App>,
     _: Login,
     Path(channel): Path<channel::Id>,
-) -> Result<Markup, InternalError> {
-    let channel = app.index().channel(&channel).await?;
+) -> Result<Markup, ChannelError> {
+    let channel = app
+        .index()
+        .channel(&channel)
+        .await
+        // impl From would work here, but it'd take more code.
+        .map_err(ChannelError)?;
     Ok(templates::channel(&channel))
 }
 
+#[derive(Debug)]
+struct ChannelError(app::Error);
+
+impl IntoResponse for ChannelError {
+    fn into_response(self) -> Response {
+        let Self(error) = self;
+        match error {
+            not_found @ app::Error::ChannelNotFound(_) => {
+                (StatusCode::NOT_FOUND, not_found.to_string()).into_response()
+            }
+            app::Error::DatabaseError(error) => InternalError::from(error).into_response(),
+        }
+    }
+}
+
 pub fn router() -> Router<App> {
     Router::new()
         .route("/", get(index))
diff --git a/src/login/routes.rs b/src/login/routes.rs
index 3c58b10..1ed61ce 100644
--- a/src/login/routes.rs
+++ b/src/login/routes.rs
@@ -8,7 +8,7 @@ use axum::{
 
 use crate::{app::App, clock::RequestedAt, error::InternalError};
 
-use super::{app::LoginError, extract::IdentityToken};
+use super::{app, extract::IdentityToken};
 
 pub fn router() -> Router<App> {
     Router::new()
@@ -27,29 +27,36 @@ async fn on_login(
     RequestedAt(now): RequestedAt,
     identity: IdentityToken,
     Form(form): Form<LoginRequest>,
-) -> Result<impl IntoResponse, InternalError> {
-    match app.logins().login(&form.name, &form.password, now).await {
-        Ok(token) => {
-            let identity = identity.set(&token);
-            Ok(LoginResponse::Successful(identity))
-        }
-        Err(LoginError::Rejected) => Ok(LoginResponse::Rejected),
-        Err(other) => Err(other.into()),
-    }
+) -> Result<LoginSuccess, LoginError> {
+    let token = app
+        .logins()
+        .login(&form.name, &form.password, now)
+        .await
+        .map_err(LoginError)?;
+    let identity = identity.set(&token);
+    Ok(LoginSuccess(identity))
 }
 
-enum LoginResponse {
-    Rejected,
-    Successful(IdentityToken),
+struct LoginSuccess(IdentityToken);
+
+impl IntoResponse for LoginSuccess {
+    fn into_response(self) -> Response {
+        let Self(identity) = self;
+        (identity, Redirect::to("/")).into_response()
+    }
 }
 
-impl IntoResponse for LoginResponse {
+struct LoginError(app::LoginError);
+
+impl IntoResponse for LoginError {
     fn into_response(self) -> Response {
-        match self {
-            Self::Successful(identity) => (identity, Redirect::to("/")).into_response(),
-            Self::Rejected => {
+        let Self(error) = self;
+        match error {
+            app::LoginError::Rejected => {
                 (StatusCode::UNAUTHORIZED, "invalid name or password").into_response()
             }
+            app::LoginError::DatabaseError(error) => InternalError::from(error).into_response(),
+            app::LoginError::PasswordHashError(error) => InternalError::from(error).into_response(),
         }
     }
 }