From 8e26aec619be32a78ab03ecf41daf209d220b0a2 Mon Sep 17 00:00:00 2001
From: Lev Kokotov <levkk@users.noreply.github.com>
Date: Mon, 28 Oct 2024 15:45:28 -0700
Subject: [PATCH] More helpful template errors (#11)

---
 ROADMAP.md                                   |   5 +
 rwf/src/http/error.html                      |  31 +++++
 rwf/src/http/response.rs                     |  16 ++-
 rwf/src/http/server.rs                       |   9 +-
 rwf/src/view/template/error.rs               | 121 +++++++++++++++++--
 rwf/src/view/template/language/expression.rs |   2 +-
 rwf/src/view/template/lexer/token.rs         |  15 +++
 rwf/src/view/template/mod.rs                 |  33 +++--
 8 files changed, 215 insertions(+), 17 deletions(-)
 create mode 100644 rwf/src/http/error.html
diff --git a/ROADMAP.md b/ROADMAP.md
index 34e2ae09..3f2f2879 100644
--- a/ROADMAP.md
+++ b/ROADMAP.md
@@ -19,6 +19,7 @@ Rwf is brand new, but web development is ancient. Many features are missing or a
 - [ ] EventStreams
 - [ ] Integration tests
 - [ ] Support for multiple WebSocket controllers (`Comms::websocket` assumes only one)
+- [ ] Multipart forms
 
 ## Dynanic templates
 
@@ -48,6 +49,10 @@ Rwf is brand new, but web development is ancient. Many features are missing or a
 
 - [ ] Consider using granian (https://github.com/levkk/rwf/issues/4)
 
+## Migrate from Ruby
+
+- [ ] Add support for running Rake apps (e.g. Rails)
+
 ## Built-ins
 
 - [ ] Feature flags and experiments
diff --git a/rwf/src/http/error.html b/rwf/src/http/error.html
new file mode 100644
index 00000000..76ca623b
--- /dev/null
+++ b/rwf/src/http/error.html
@@ -0,0 +1,31 @@
+<!doctype html>
+<html>
+    <head>
+        <title><%= title %></title>
+        <style>
+            .container {
+                margin: 10px 25px;
+            }
+
+            .center {
+                display: flex;
+                align-items: center;
+                justify-content: center;
+            }
+
+            .error {
+                border: 1px solid red;
+                border-radius: 2px;
+                padding: 10px 30px;
+            }
+        </style>
+    </head>
+    <body>
+        <div class="container center">
+            <h3><%= title %></h3>
+        </div>
+        <div class="container center">
+            <code class="error"><pre><%= message %></pre></code>
+        </div>
+    </body>
+</html>
diff --git a/rwf/src/http/response.rs b/rwf/src/http/response.rs
index f6350901..8fd61440 100644
--- a/rwf/src/http/response.rs
+++ b/rwf/src/http/response.rs
@@ -1,14 +1,20 @@
 //! HTTP response.
 
+use once_cell::sync::Lazy;
 use serde::Serialize;
 use std::collections::HashMap;
 use std::marker::Unpin;
 use tokio::io::{AsyncWrite, AsyncWriteExt};
 
 use super::{head::Version, Body, Cookie, Cookies, Error, Headers, Request};
-use crate::view::TurboStream;
+use crate::view::{Template, TurboStream};
 use crate::{config::get_config, controller::Session};
 
+static ERROR_TEMPLATE: Lazy<Template> = Lazy::new(|| {
+    let template = include_str!("error.html");
+    Template::from_str(template).unwrap()
+});
+
 /// Response status, e.g. 404, 200, etc.
 #[derive(Debug)]
 pub enum Status {
@@ -352,6 +358,14 @@ impl Response {
             .code(500)
     }
 
+    pub fn internal_error_pretty(title: &str, message: &str) -> Self {
+        let body = ERROR_TEMPLATE
+            .render([("title", title), ("message", message)])
+            .unwrap();
+
+        Self::new().html(body).code(500)
+    }
+
     pub fn unauthorized(auth: &str) -> Self {
         Self::new()
             .html(
diff --git a/rwf/src/http/server.rs b/rwf/src/http/server.rs
index b338dd60..95a41aaa 100644
--- a/rwf/src/http/server.rs
+++ b/rwf/src/http/server.rs
@@ -119,7 +119,7 @@ impl Server {
                         let response = match handler.handle_internal(request.clone()).await {
                             Ok(response) => response,
                             Err(err) => {
-                                error!("{:?}", err);
+                                error!("{}", err);
                                 match err {
                                     ControllerError::HttpError(err) => match err.code() {
                                         400 => Response::bad_request(),
@@ -127,6 +127,13 @@ impl Server {
                                         _ => Response::internal_error(err),
                                     },
 
+                                    ControllerError::ViewError(err) => {
+                                        Response::internal_error_pretty(
+                                            "Template error",
+                                            err.to_string().as_str(),
+                                        )
+                                    }
+
                                     err => Response::internal_error(err),
                                 }
                             }
diff --git a/rwf/src/view/template/error.rs b/rwf/src/view/template/error.rs
index c7e2ba55..0f132473 100644
--- a/rwf/src/view/template/error.rs
+++ b/rwf/src/view/template/error.rs
@@ -1,7 +1,7 @@
 use super::{Token, TokenWithContext};
 use thiserror::Error;
 
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 #[derive(Error, Debug)]
 pub enum Error {
@@ -11,24 +11,131 @@ pub enum Error {
     #[error("expression syntax error")]
     ExpressionSyntax(TokenWithContext),
 
-    #[error("expected {0}, got {0}")]
+    #[error("expected token \"{0}\", but have token \"{0}\" instead")]
     WrongToken(TokenWithContext, Token),
 
-    #[error("eof: {0}")]
+    #[error("reached end of file while performing \"{0}\", did you forget a closing tag?")]
     Eof(&'static str),
 
-    #[error("undefined variable: {0}")]
+    #[error("variable \"{0}\" is not defined or in scope")]
     UndefinedVariable(String),
 
-    #[error("unknown method: {0}")]
+    #[error("method \"{0}\" is not defined")]
     UnknownMethod(String),
 
-    #[error("template does not exist: {0}")]
+    #[error("template \"{0}\" does not exist")]
     TemplateDoesNotExist(PathBuf),
 
     #[error("serialization error")]
     SerializationError,
 
-    #[error("time format error: {0}")]
+    #[error("failed to format a timtestamp correctly, error: \"{0}\"")]
     TimeFormatError(#[from] time::error::Format),
+
+    #[error("{0}")]
+    Pretty(String),
+}
+
+impl Error {
+    pub fn pretty(self, source: &str, path: Option<impl AsRef<Path> + Copy>) -> Self {
+        let token = match self {
+            Error::Syntax(ref token) => token,
+            Error::ExpressionSyntax(ref token) => token,
+            Error::WrongToken(ref token, _) => token,
+            _ => {
+                if let Some(path) = path {
+                    let prefix = "---> ";
+                    return Error::Pretty(format!(
+                        "{}{}\n\n{}{}",
+                        prefix,
+                        path.as_ref().display(),
+                        vec![' '; prefix.len()].into_iter().collect::<String>(),
+                        self.to_string()
+                    ));
+                } else {
+                    return self;
+                }
+            }
+        };
+
+        let error_msg = match self {
+            Error::Syntax(ref _token) => "syntax error".to_string(),
+            Error::ExpressionSyntax(ref _token) => "expression syntax error".to_string(),
+            Error::WrongToken(ref _token, _) => "unexpected token".to_string(),
+            _ => "".to_string(),
+        };
+
+        let context = source.lines().nth(token.line() - 1); // std::fs lines start at 0
+        let leading_spaces = if let Some(ref context) = context {
+            context.len() - context.trim().len()
+        } else {
+            0
+        };
+        let underline = vec![' '; token.column() - token.token().len() + 1 - leading_spaces]
+            .into_iter()
+            .collect::<String>()
+            + &format!("^ {}", error_msg);
+
+        let line_number = format!("{} | ", token.line());
+        let underline_offset = vec![' '; token.line().to_string().len()]
+            .into_iter()
+            .collect::<String>()
+            + " | ";
+
+        let path = if let Some(path) = path {
+            format!(
+                "---> {}:{}:{}\n\n",
+                path.as_ref().display(),
+                token.line(),
+                token.column()
+            )
+        } else {
+            "".to_string()
+        };
+
+        if let Some(context) = context {
+            Error::Pretty(format!(
+                "{}{}\n{}{}\n{}{}",
+                path,
+                underline_offset,
+                line_number,
+                context.trim(),
+                underline_offset,
+                underline
+            ))
+        } else {
+            self
+        }
+    }
+
+    pub fn pretty_from_path(self, path: impl AsRef<Path> + Copy) -> Self {
+        let src = match std::fs::read_to_string(path) {
+            Ok(src) => src,
+            Err(_) => return self,
+        };
+
+        self.pretty(&src, Some(path))
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    #[test]
+    fn test_underline() {
+        let token = TokenWithContext::new(Token::If, 1, 9);
+        let error = Error::Syntax(token);
+        let pretty = error.pretty(
+            "<% if apples %>
+    <% if oranges are blue %>
+",
+            None::<&str>,
+        );
+
+        assert_eq!(
+            pretty.to_string(),
+            "  | \n1 | <% if apples %>\n  |         ^ syntax error"
+        );
+    }
 }
diff --git a/rwf/src/view/template/language/expression.rs b/rwf/src/view/template/language/expression.rs
index e2d73b3e..a676d693 100644
--- a/rwf/src/view/template/language/expression.rs
+++ b/rwf/src/view/template/language/expression.rs
@@ -9,7 +9,7 @@ use std::iter::{Iterator, Peekable};
 
 /// An expression, like `5 == 6` or `logged_in == false`,
 /// which when evaluated produces a single value, e.g. `true`.
-#[derive(Debug, Clone, PartialEq)]
+#[derive(Debug, Clone)]
 pub enum Expression {
     // Standard `5 + 6`-style expression.
     // It's recursive, so you can have something like `(5 + 6) / (1 - 5)`.
diff --git a/rwf/src/view/template/lexer/token.rs b/rwf/src/view/template/lexer/token.rs
index 998e8257..d318ac65 100644
--- a/rwf/src/view/template/lexer/token.rs
+++ b/rwf/src/view/template/lexer/token.rs
@@ -50,3 +50,18 @@ pub enum Token {
     RoundBracketStart,
     RoundBracketEnd,
 }
+
+impl Token {
+    pub fn len(&self) -> usize {
+        match self {
+            Token::If => 2,
+            Token::Else => 4,
+            Token::End => 3,
+            Token::BlockEnd => 2,
+            Token::BlockStart => 2,
+            Token::BlockStartPrint => 3,
+            Token::BlockStartRender => 3,
+            _ => 0,
+        }
+    }
+}
diff --git a/rwf/src/view/template/mod.rs b/rwf/src/view/template/mod.rs
index 932e9223..60993a73 100644
--- a/rwf/src/view/template/mod.rs
+++ b/rwf/src/view/template/mod.rs
@@ -12,48 +12,67 @@ use crate::view::Templates;
 
 use language::Program;
 
+use std::fs::read_to_string;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 
+/// Rwf template.
+///
+/// Contains the AST for the template.
 #[allow(dead_code)]
 #[derive(Clone, Debug)]
 pub struct Template {
     program: Program,
-    path: PathBuf,
+    path: Option<PathBuf>,
 }
 
 impl Template {
+    /// Read and compile a template from disk.
     pub fn new(path: impl AsRef<Path> + std::marker::Copy) -> Result<Self, Error> {
-        let text = match std::fs::read_to_string(path) {
+        let text = match read_to_string(path) {
             Ok(text) => text,
             Err(_) => return Err(Error::TemplateDoesNotExist(path.as_ref().to_owned())),
         };
 
         Ok(Template {
             program: Program::from_str(&text)?,
-            path: path.as_ref().to_owned(),
+            path: Some(path.as_ref().to_owned()),
         })
     }
 
+    /// Read and compile a template from a string.
     pub fn from_str(template: &str) -> Result<Self, Error> {
         Ok(Template {
             program: Program::from_str(template)?,
-            path: PathBuf::from("/dev/null"),
+            path: None,
         })
     }
 
+    /// Given a context, execute the template, producing a string.
     pub fn render(&self, context: impl TryInto<Context, Error = Error>) -> Result<String, Error> {
         let context: Context = context.try_into()?;
 
-        self.program.evaluate(&context)
+        match self.program.evaluate(&context) {
+            Ok(result) => Ok(result),
+            Err(err) => {
+                if let Some(path) = &self.path {
+                    Err(err.pretty_from_path(path))
+                } else {
+                    Err(err)
+                }
+            }
+        }
     }
 
     pub fn render_default(&self) -> Result<String, Error> {
-        self.program.evaluate(&Context::default())
+        self.render(&Context::default())
     }
 
     pub fn cached(path: impl AsRef<Path> + Copy) -> Result<Arc<Self>, Error> {
-        Templates::cache().get(path)
+        match Templates::cache().get(path) {
+            Ok(template) => Ok(template),
+            Err(err) => Err(err.pretty_from_path(path)),
+        }
     }
 
     pub fn load(path: impl AsRef<Path> + Copy) -> Result<Arc<Self>, Error> {
