Skip to content

Commit

Permalink
better error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
lovasoa committed Jan 2, 2025
1 parent 82d7186 commit e47cbc9
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 6 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ This is a bugfix release.
- adds support for MSSQL's `JSON_ARRAY` and `JSON_OBJECT` functions
- adds support for PostgreSQL's `JSON_OBJECT(key : value)` and `JSON_OBJECT(key VALUE value)` syntax
- fixes the parsing of `true` and `false` in Microsoft SQL Server (mssql): they are now correctly parsed as column names, not as boolean values, since mssql does not support boolean literals. This means you may have to replace `TRUE as some_property` with `1 as some_property` in your SQL code when working with mssql.
- When your SQL contains errors, the error message now displays the precise line(s) number(s) of your file that contain the error.

## 0.32.0 (2024-12-29)

Expand Down
35 changes: 33 additions & 2 deletions src/webserver/database/error_highlighting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,18 @@ use std::{
path::{Path, PathBuf},
};

use super::sql::{SourceSpan, StmtWithParams};

#[derive(Debug)]
struct NiceDatabaseError {
/// The source file that contains the query.
source_file: PathBuf,
/// The error that occurred.
db_err: sqlx::error::Error,
/// The query that was executed.
query: String,
/// The start location of the query in the source file, if the query was extracted from a larger file.
query_position: Option<SourceSpan>,
}

impl std::fmt::Display for NiceDatabaseError {
Expand All @@ -22,12 +29,20 @@ impl std::fmt::Display for NiceDatabaseError {
let Some(mut offset) = db_err.offset() else {
return write!(f, "{}", self.query);
};
for (line_no, line) in self.query.lines().enumerate() {
for line in self.query.lines() {
if offset > line.len() {
offset -= line.len() + 1;
} else {
highlight_line_offset(f, line, offset);
write!(f, "line {}, character {offset}", line_no + 1)?;
if let Some(query_position) = self.query_position {
let start_line = query_position.start.line;
let end_line = query_position.end.line;
if start_line == end_line {
write!(f, "{}: line {}", self.source_file.display(), start_line)?;
} else {
write!(f, "{}: lines {} to {}", self.source_file.display(), start_line, end_line)?;
}
}
break;
}
}
Expand All @@ -51,6 +66,22 @@ pub fn display_db_error(
source_file: source_file.to_path_buf(),
db_err,
query: query.to_string(),
query_position: None,
})
}

/// Display a database error with a highlighted line and character offset.
#[must_use]
pub fn display_stmt_db_error(
source_file: &Path,
stmt: &StmtWithParams,
db_err: sqlx::error::Error,
) -> anyhow::Error {
anyhow::Error::new(NiceDatabaseError {
source_file: source_file.to_path_buf(),
db_err,
query: stmt.query.to_string(),
query_position: Some(stmt.query_position),
})
}

Expand Down
9 changes: 5 additions & 4 deletions src/webserver/database/execute_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::path::Path;
use std::pin::Pin;

use super::csv_import::run_csv_import;
use super::error_highlighting::display_stmt_db_error;
use super::sql::{
DelayedFunctionCall, ParsedSqlFile, ParsedStatement, SimpleSelectValue, StmtWithParams,
};
Expand Down Expand Up @@ -62,7 +63,7 @@ pub fn stream_query_results_with_conn<'a>(
let mut stream = connection.fetch_many(query);
let mut error = None;
while let Some(elem) = stream.next().await {
let mut query_result = parse_single_sql_result(source_file, &stmt.query, elem);
let mut query_result = parse_single_sql_result(source_file, stmt, elem);
if let DbItem::Error(e) = query_result {
error = Some(e);
break;
Expand Down Expand Up @@ -196,7 +197,7 @@ async fn execute_set_variable_query<'a>(
Ok(None) => None,
Err(e) => {
try_rollback_transaction(connection).await;
let err = display_db_error(source_file, &statement.query, e);
let err = display_stmt_db_error(source_file, statement, e);
return Err(err);
}
};
Expand Down Expand Up @@ -257,7 +258,7 @@ async fn take_connection<'a, 'b>(
#[inline]
fn parse_single_sql_result(
source_file: &Path,
sql: &str,
stmt: &StmtWithParams,
res: sqlx::Result<Either<AnyQueryResult, AnyRow>>,
) -> DbItem {
match res {
Expand All @@ -272,7 +273,7 @@ fn parse_single_sql_result(
DbItem::FinishedQuery
}
Err(err) => {
let nice_err = display_db_error(source_file, sql, err);
let nice_err = display_stmt_db_error(source_file, stmt, err);
DbItem::Error(nice_err)
}
}
Expand Down
32 changes: 32 additions & 0 deletions src/webserver/database/sql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ impl AsyncFromStrWithState for ParsedSqlFile {
pub(super) struct StmtWithParams {
/// The SQL query with placeholders for parameters.
pub query: String,
/// The line and column of the first token in the query.
pub query_position: SourceSpan,
/// Parameters that should be bound to the query.
/// They can contain functions that will be called before the query is executed,
/// the result of which will be bound to the query.
Expand All @@ -82,6 +84,20 @@ pub(super) struct StmtWithParams {
pub json_columns: Vec<String>,
}

/// A location in the source code.
#[derive(Debug, PartialEq, Clone, Copy)]
pub(super) struct SourceSpan {
pub start: SourceLocation,
pub end: SourceLocation,
}

/// A location in the source code.
#[derive(Debug, PartialEq, Clone, Copy)]
pub(super) struct SourceLocation {
pub line: usize,
pub column: usize,
}

#[derive(Debug)]
pub(super) enum ParsedStatement {
StmtWithParams(StmtWithParams),
Expand Down Expand Up @@ -165,12 +181,27 @@ fn parse_single_statement(
log::debug!("Final transformed statement: {stmt}");
Some(ParsedStatement::StmtWithParams(StmtWithParams {
query,
query_position: extract_query_start(&stmt),
params,
delayed_functions,
json_columns,
}))
}

fn extract_query_start(stmt: &impl Spanned) -> SourceSpan {
let location = stmt.span();
SourceSpan {
start: SourceLocation {
line: usize::try_from(location.start.line).unwrap_or(0),
column: usize::try_from(location.start.column).unwrap_or(0),
},
end: SourceLocation {
line: usize::try_from(location.end.line).unwrap_or(0),
column: usize::try_from(location.end.column).unwrap_or(0),
},
}
}

fn syntax_error(err: ParserError, parser: &Parser, sql: &str) -> ParsedStatement {
let location = parser.peek_token_no_skip().span;
ParsedStatement::Error(anyhow::Error::from(err).context(format!(
Expand Down Expand Up @@ -426,6 +457,7 @@ fn extract_set_variable(
variable,
StmtWithParams {
query: select_stmt.to_string(),
query_position: extract_query_start(&select_stmt),
params: std::mem::take(params),
delayed_functions,
json_columns,
Expand Down

0 comments on commit e47cbc9

Please sign in to comment.