Skip to content

Commit b3c8d69

Browse files
committed
improved error handling: stop after first error
1 parent 97562e6 commit b3c8d69

File tree

6 files changed

+64
-33
lines changed

6 files changed

+64
-33
lines changed

CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@
1515
- TreeMap charts in the chart component allow you to visualize hierarchical data structures.
1616
- Timeline charts allow you to visualize time intervals.
1717
- Fixed multiple small display issues in the chart component.
18-
- Better error handling: Stop parsing SQL files at the first syntax error, and display a single error message about the error.
19-
- The previous behavior was to try paresing a new statement after a syntax error, leading to an avalanche of irrelevant error messages after a syntax error.
18+
- Better error handling: Stop processing the SQL file after the first error is encountered.
19+
- The previous behavior was to try paresing a new statement after a syntax error, leading to a cascade of irrelevant error messages after a syntax error.
2020

2121
## 0.26.0 (2024-08-06)
2222
### Components

src/webserver/database/error_highlighting.rs

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,52 @@
11
use std::fmt::Write;
22

3-
/// Display a database error with a highlighted line and character offset.
4-
#[must_use]
5-
pub fn display_db_error(context: &str, query: &str, db_err: sqlx::error::Error) -> anyhow::Error {
6-
let mut msg = format!("{context}:\n");
7-
let offset = if let sqlx::error::Error::Database(db_err) = &db_err {
8-
db_err.offset()
9-
} else {
10-
None
11-
};
12-
if let Some(mut offset) = offset {
13-
for (line_no, line) in query.lines().enumerate() {
14-
if offset > line.len() {
15-
offset -= line.len() + 1;
16-
} else {
17-
highlight_line_offset(&mut msg, line, offset);
18-
write!(msg, "line {}, character {offset}", line_no + 1).unwrap();
19-
break;
3+
#[derive(Debug)]
4+
struct NiceDatabaseError {
5+
db_err: sqlx::error::Error,
6+
query: String,
7+
}
8+
9+
impl std::fmt::Display for NiceDatabaseError {
10+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
11+
self.db_err.fmt(f)?;
12+
if let sqlx::error::Error::Database(db_err) = &self.db_err {
13+
let Some(mut offset) = db_err.offset() else {
14+
return Ok(());
15+
};
16+
write!(f, "\n\nError in query:\n")?;
17+
for (line_no, line) in self.query.lines().enumerate() {
18+
if offset > line.len() {
19+
offset -= line.len() + 1;
20+
} else {
21+
highlight_line_offset(f, line, offset);
22+
write!(f, "line {}, character {offset}", line_no + 1)?;
23+
break;
24+
}
2025
}
26+
} else {
27+
write!(f, "{}", self.query.lines().next().unwrap_or_default())?;
2128
}
22-
} else {
23-
write!(msg, "{}", query.lines().next().unwrap_or_default()).unwrap();
29+
Ok(())
2430
}
25-
anyhow::Error::new(db_err).context(msg)
31+
}
32+
33+
impl std::error::Error for NiceDatabaseError {
34+
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
35+
self.db_err.source()
36+
}
37+
}
38+
39+
/// Display a database error with a highlighted line and character offset.
40+
#[must_use]
41+
pub fn display_db_error(query: &str, db_err: sqlx::error::Error) -> anyhow::Error {
42+
anyhow::Error::new(NiceDatabaseError {
43+
db_err,
44+
query: query.to_string(),
45+
})
2646
}
2747

2848
/// Highlight a line with a character offset.
29-
pub fn highlight_line_offset(msg: &mut String, line: &str, offset: usize) {
49+
pub fn highlight_line_offset<W: std::fmt::Write>(msg: &mut W, line: &str, offset: usize) {
3050
writeln!(msg, "{line}").unwrap();
3151
writeln!(msg, "{}⬆️", " ".repeat(offset)).unwrap();
3252
}

src/webserver/database/execute_queries.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ impl Database {
3333
.prepare_with(query, param_types)
3434
.await
3535
.map(|s| s.to_owned())
36-
.map_err(|e| display_db_error("Failed to prepare SQL statement", query, e))
36+
.map_err(|e| display_db_error(query, e))
3737
}
3838
}
3939

@@ -85,6 +85,18 @@ pub fn stream_query_results_with_conn<'a>(
8585
.map(|res| res.unwrap_or_else(DbItem::Error))
8686
}
8787

88+
pub fn stop_at_first_error(
89+
results_stream: impl Stream<Item = DbItem>,
90+
) -> impl Stream<Item = DbItem> {
91+
let mut has_error = false;
92+
results_stream.take_while(move |item| {
93+
// We stop the stream AFTER the first error, so that the error is still returned to the client, but the rest of the queries are not executed.
94+
let should_continue = !has_error;
95+
has_error |= matches!(item, DbItem::Error(_));
96+
futures_util::future::ready(should_continue)
97+
})
98+
}
99+
88100
/// Executes the sqlpage pseudo-functions contained in a static simple select
89101
async fn exec_static_simple_select(
90102
columns: &[(String, SimpleSelectValue)],
@@ -214,11 +226,7 @@ fn parse_single_sql_result(sql: &str, res: sqlx::Result<Either<AnyQueryResult, A
214226
log::debug!("Finished query with result: {:?}", res);
215227
DbItem::FinishedQuery
216228
}
217-
Err(err) => DbItem::Error(display_db_error(
218-
"Failed to execute SQL statement",
219-
sql,
220-
err,
221-
)),
229+
Err(err) => DbItem::Error(display_db_error(sql, err)),
222230
}
223231
}
224232

src/webserver/database/migrations.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,10 @@ pub async fn apply(config: &crate::app_config::AppConfig, db: &Database) -> anyh
3535
match err {
3636
MigrateError::Execute(n, source) => {
3737
let migration = migrator.iter().find(|&m| m.version == n).unwrap();
38-
display_db_error("Error in the SQL migration", &migration.sql, source).context(
39-
format!("Failed to apply migration {}", DisplayMigration(migration)),
40-
)
38+
display_db_error(&migration.sql, source).context(format!(
39+
"Failed to apply migration {}",
40+
DisplayMigration(migration)
41+
))
4142
}
4243
source => anyhow::Error::new(source),
4344
}

src/webserver/database/sql.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ fn parse_single_statement(
163163
fn syntax_error(err: ParserError, parser: &Parser, sql: &str) -> ParsedStatement {
164164
let location = parser.peek_token_no_skip().location;
165165
ParsedStatement::Error(anyhow::Error::from(err).context(format!(
166-
"The SQLPage parser couldn't understand the SQL file. Parsing failed. Please check for syntax errors:\n{}",
166+
"Parsing failed: SQLPage couldn't understand the SQL file. Please check for syntax errors:\n\n{}",
167167
quote_source_with_highlight(sql, location.line, location.column)
168168
)))
169169
}

src/webserver/http.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use crate::render::{HeaderContext, PageContext, RenderContext};
22
use crate::webserver::content_security_policy::ContentSecurityPolicy;
3+
use crate::webserver::database::execute_queries::stop_at_first_error;
34
use crate::webserver::database::{execute_queries::stream_query_results_with_conn, DbItem};
45
use crate::webserver::http_request_info::extract_request_info;
56
use crate::webserver::ErrorWithStatus;
@@ -259,6 +260,7 @@ async fn render_sql(
259260
let mut conn = None;
260261
let database_entries_stream =
261262
stream_query_results_with_conn(&sql_file, &mut req_param, &mut conn);
263+
let database_entries_stream = stop_at_first_error(database_entries_stream);
262264
let response_with_writer = build_response_header_and_stream(
263265
Arc::clone(&app_state),
264266
database_entries_stream,

0 commit comments

Comments
 (0)