Skip to content

Commit 637f198

Browse files
committed
error handling: improve visual presentation of error messages
- Updated error messages to be more user-friendly and visually appealing in the browser. - Introduced a new method for retrieving static files from the cache. - Improved error reporting for invalid UTF-8 encoding in SQL files.
1 parent bb722d9 commit 637f198

File tree

8 files changed

+106
-42
lines changed

8 files changed

+106
-42
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
from users;
2222
```
2323
- When a sql file is saved with the wrong character encoding (not UTF8), SQLPage now displays a helpful error messages that points to exactly where in the file the problem is.
24+
- More visual error messages: errors that occured before (such as file access issues) used to generate plain text messages that looked scary to non-technical users. All errors are now displayed nicely in the browser.
2425

2526
## v0.36.1
2627
- Fix regression introduced in v0.36.0: PostgreSQL money values showed as 0.0

sqlpage/templates/error.handlebars

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,11 @@
1313
<div class="text-muted">
1414
<p>We are sorry, but an error occurred while generating this page. You should contact the site's
1515
administrator.</p>
16+
{{~#if description~}}
1617
<pre class="mt-2"><code class="sqlpage-error-description">{{description}}</code></pre>
18+
{{~/if~}}
1719
{{#if backtrace}}
18-
<details class="mt-2">
20+
<details class="mt-2" open>
1921
<summary>Backtrace</summary>
2022
{{~#each backtrace~}}
2123
<pre class="fs-6 mt-1 p-1 my-1"><code>{{this}}</code></pre>

src/file_cache.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,13 @@ impl<T: AsyncFromStrWithState> FileCache<T> {
108108
self.get_with_privilege(app_state, path, true).await
109109
}
110110

111+
pub fn get_static(&self, path: &Path) -> anyhow::Result<Arc<T>> {
112+
self.static_files
113+
.get(path)
114+
.map(|cached| Arc::clone(&cached.content))
115+
.ok_or_else(|| anyhow::anyhow!("File {} not found in static files", path.display()))
116+
}
117+
111118
/// Gets a file from the cache, or loads it from the file system if it's not there
112119
/// The privileged parameter is used to determine whether the access should be denied
113120
/// if the file is in the sqlpage/ config directory

src/filesystem.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,9 @@ impl FileSystem {
7878

7979
let display_path = path.display();
8080
anyhow::format_err!(
81-
"SQLPage expects all sql files to be encoded in UTF-8. \
82-
The file \"{display_path}\" contains the following invalid UTF-8 byte sequence around line {line_num} character {bad_char_idx}: \"{bad_seq}\". \
81+
"SQLPage expects all sql files to be encoded in UTF-8. \n\
82+
In \"{display_path}\", around line {line_num} character {bad_char_idx}, the following invalid UTF-8 byte sequence was found: \n\
83+
\"{bad_seq}\". \n\
8384
Please convert the file to UTF-8.",
8485
)
8586
})

src/render.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ impl HeaderContext {
131131
let data = json!({
132132
"component": "error",
133133
"description": err.to_string(),
134-
"backtrace": get_backtrace(&err),
134+
"backtrace": get_backtrace_as_strings(&err),
135135
});
136136
self.start_body(data).await
137137
}
@@ -391,16 +391,6 @@ async fn verify_password_async(
391391
.await?
392392
}
393393

394-
fn get_backtrace(error: &anyhow::Error) -> Vec<String> {
395-
let mut backtrace = vec![];
396-
let mut source = error.source();
397-
while let Some(s) = source {
398-
backtrace.push(format!("{s}"));
399-
source = s.source();
400-
}
401-
backtrace
402-
}
403-
404394
fn get_object_str<'a>(json: &'a JsonValue, key: &str) -> Option<&'a str> {
405395
json.as_object()
406396
.and_then(|obj| obj.get(key))
@@ -781,7 +771,7 @@ impl<W: std::io::Write> HtmlRenderContext<W> {
781771
json!({
782772
"query_number": self.current_statement,
783773
"description": error.to_string(),
784-
"backtrace": get_backtrace(error),
774+
"backtrace": get_backtrace_as_strings(error),
785775
"note": "You can hide error messages like this one from your users by setting the 'environment' configuration option to 'production'."
786776
})
787777
};
@@ -895,6 +885,16 @@ impl<W: std::io::Write> HtmlRenderContext<W> {
895885
}
896886
}
897887

888+
pub(super) fn get_backtrace_as_strings(error: &anyhow::Error) -> Vec<String> {
889+
let mut backtrace = vec![];
890+
let mut source = error.source();
891+
while let Some(s) = source {
892+
backtrace.push(format!("{s}"));
893+
source = s.source();
894+
}
895+
backtrace
896+
}
897+
898898
struct HandlebarWriterOutput<W: std::io::Write>(W);
899899

900900
impl<W: std::io::Write> handlebars::Output for HandlebarWriterOutput<W> {

src/templates.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,24 +110,33 @@ impl AllTemplates {
110110
Ok(())
111111
}
112112

113+
fn template_path(name: &str) -> PathBuf {
114+
let mut path: PathBuf =
115+
PathBuf::with_capacity(TEMPLATES_DIR.len() + 1 + name.len() + ".handlebars".len());
116+
path.push(TEMPLATES_DIR);
117+
path.push(name);
118+
path.set_extension("handlebars");
119+
path
120+
}
121+
113122
pub async fn get_template(
114123
&self,
115124
app_state: &AppState,
116125
name: &str,
117126
) -> anyhow::Result<Arc<SplitTemplate>> {
118127
use anyhow::Context;
119-
let mut path: PathBuf =
120-
PathBuf::with_capacity(TEMPLATES_DIR.len() + 1 + name.len() + ".handlebars".len());
121-
path.push(TEMPLATES_DIR);
122-
path.push(name);
123-
path.set_extension("handlebars");
128+
let path = Self::template_path(name);
124129
self.split_templates
125130
.get(app_state, &path)
126131
.await
127132
.with_context(|| format!("Unable to get the component '{name}'"))
128133
}
129-
}
130134

135+
pub fn get_static_template(&self, name: &str) -> anyhow::Result<Arc<SplitTemplate>> {
136+
let path = Self::template_path(name);
137+
self.split_templates.get_static(&path)
138+
}
139+
}
131140
#[test]
132141
fn test_split_template() {
133142
let template = Template::compile(

src/webserver/error.rs

Lines changed: 64 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,49 +2,93 @@
22
33
use std::path::PathBuf;
44

5+
use crate::render::get_backtrace_as_strings;
56
use crate::webserver::ErrorWithStatus;
67
use crate::AppState;
78
use actix_web::error::UrlencodedError;
89
use actix_web::http::{header, StatusCode};
910
use actix_web::{HttpRequest, HttpResponse};
1011
use actix_web::{HttpResponseBuilder, ResponseError};
12+
use handlebars::{Renderable, StringOutput};
13+
use serde_json::json;
14+
15+
fn error_to_html_string(app_state: &AppState, err: &anyhow::Error) -> anyhow::Result<String> {
16+
let mut out = StringOutput::new();
17+
let shell_template = app_state.all_templates.get_static_template("shell")?;
18+
let error_template = app_state.all_templates.get_static_template("error")?;
19+
let registry = &app_state.all_templates.handlebars;
20+
let shell_ctx = handlebars::Context::null();
21+
let data = if app_state.config.environment.is_prod() {
22+
json!(null)
23+
} else {
24+
json!({
25+
"description": err.to_string(),
26+
"backtrace": get_backtrace_as_strings(err),
27+
"note": "You can hide error messages like this one from your users by setting the 'environment' configuration option to 'production'.",
28+
})
29+
};
30+
let err_ctx = handlebars::Context::wraps(data)?;
31+
let rc = &mut handlebars::RenderContext::new(None);
32+
33+
// Open the shell component
34+
shell_template
35+
.before_list
36+
.render(registry, &shell_ctx, rc, &mut out)?;
37+
38+
// Open the error component
39+
error_template
40+
.before_list
41+
.render(registry, &err_ctx, rc, &mut out)?;
42+
// Close the error component
43+
error_template
44+
.after_list
45+
.render(registry, &err_ctx, rc, &mut out)?;
46+
47+
// Close the shell component
48+
shell_template
49+
.after_list
50+
.render(registry, &shell_ctx, rc, &mut out)?;
51+
52+
Ok(out.into_string()?)
53+
}
1154

1255
fn anyhow_err_to_actix_resp(e: &anyhow::Error, state: &AppState) -> HttpResponse {
1356
let mut resp = HttpResponseBuilder::new(StatusCode::INTERNAL_SERVER_ERROR);
14-
let mut body = "Sorry, but we were not able to process your request.\n\n".to_owned();
15-
let env = state.config.environment;
16-
if env.is_prod() {
17-
body.push_str("Contact the administrator for more information. A detailed error message has been logged.");
18-
log::error!("{e:#}");
19-
} else {
20-
use std::fmt::Write;
21-
write!(
22-
body,
23-
"Below are detailed debugging information which may contain sensitive data. \n\
24-
Set environment to \"production\" in the configuration file to hide this information. \n\n\
25-
{e:?}"
26-
)
27-
.unwrap();
28-
}
2957
resp.insert_header((
3058
header::CONTENT_TYPE,
3159
header::HeaderValue::from_static("text/plain; charset=utf-8"),
3260
));
3361

3462
if let Some(status_err @ &ErrorWithStatus { .. }) = e.downcast_ref() {
35-
status_err
36-
.error_response()
37-
.set_body(actix_web::body::BoxBody::new(body))
63+
status_err.error_response()
3864
} else if let Some(sqlx::Error::PoolTimedOut) = e.downcast_ref() {
3965
use rand::Rng;
4066
resp.status(StatusCode::TOO_MANY_REQUESTS)
4167
.insert_header((
4268
header::RETRY_AFTER,
4369
header::HeaderValue::from(rand::rng().random_range(1..=15)),
4470
))
45-
.body("The database is currently too busy to handle your request. Please try again later.\n\n".to_owned() + &body)
71+
.body("The database is currently too busy to handle your request. Please try again later.".to_owned())
4672
} else {
47-
resp.body(body)
73+
match error_to_html_string(state, e) {
74+
Ok(body) => {
75+
resp.insert_header((
76+
header::CONTENT_TYPE,
77+
header::HeaderValue::from_static("text/html; charset=utf-8"),
78+
));
79+
resp.body(body)
80+
}
81+
Err(second_err) => {
82+
log::error!("Unable to render error: {e:#}");
83+
resp.body(format!(
84+
"A second error occurred while rendering the error page: \n\n\
85+
Initial error: \n\
86+
{e:#}\n\n\
87+
Second error: \n\
88+
{second_err:#}"
89+
))
90+
}
91+
}
4892
}
4993
}
5094

src/webserver/http.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ async fn process_sql_request(
275275
.sql_file_cache
276276
.get_with_privilege(app_state, &sql_path, false)
277277
.await
278-
.with_context(|| format!("Unable to get SQL file \"{}\"", sql_path.display()))
278+
.with_context(|| format!("Unable to read SQL file \"{}\"", sql_path.display()))
279279
.map_err(|e| anyhow_err_to_actix(e, app_state))?;
280280
render_sql(req, sql_file).await
281281
}

0 commit comments

Comments
 (0)