breaking(postgres): add position information to errors

Breaking change: renamed `PgDatabaseError::position()` to `::position_raw()`.

Author: abonander

sqlx-postgres/src/connection/executor.rs

@@ -1,5 +1,5 @@
 use crate::describe::Describe;
-use crate::error::Error;
+use crate::error::{Error, PgResultExt};
 use crate::executor::{Execute, Executor};
 use crate::logger::QueryLogger;
 use crate::message::{
@@ -168,7 +168,9 @@ impl PgConnection {
             return Ok((*statement).clone());
         }
 
-        let statement = prepare(self, sql, parameters, metadata).await?;
+        let statement = prepare(self, sql, parameters, metadata)
+            .await
+            .pg_apply_error_pos(sql)?;
 
         if store_to_cache && self.cache_statement.is_enabled() {
             if let Some((id, _)) = self.cache_statement.insert(sql, statement.clone()) {
@@ -267,7 +269,9 @@ impl PgConnection {
 
         Ok(try_stream! {
             loop {
-                let message = self.stream.recv().await?;
+                let message = self.stream.recv()
+                    .await
+                    .pg_apply_error_pos(query)?;
 
                 match message.format {
                     MessageFormat::BindComplete

sqlx-postgres/src/connection/stream.rs

@@ -104,7 +104,7 @@ impl PgStream {
             match message.format {
                 MessageFormat::ErrorResponse => {
                     // An error returned from the database server.
-                    return Err(PgDatabaseError(message.decode()?).into());
+                    return Err(PgDatabaseError::new(message.decode()?).into());
                 }
 
                 MessageFormat::NotificationResponse => {

sqlx-postgres/src/error.rs

@@ -1,65 +1,84 @@
+use std::borrow::Cow;
 use std::error::Error as StdError;
 use std::fmt::{self, Debug, Display, Formatter};
 
 use atoi::atoi;
-use smallvec::alloc::borrow::Cow;
 
 pub(crate) use sqlx_core::error::*;
 
 use crate::message::{Notice, PgSeverity};
 
 /// An error returned from the PostgreSQL database.
-pub struct PgDatabaseError(pub(crate) Notice);
+pub struct PgDatabaseError {
+    pub(crate) notice: Notice,
+    pub(crate) error_pos: Option<ErrorPosition>,
+}
 
 // Error message fields are documented:
 // https://www.postgresql.org/docs/current/protocol-error-fields.html
 
 impl PgDatabaseError {
+    pub(crate) fn new(notice: Notice) -> Self {
+        PgDatabaseError {
+            notice,
+            error_pos: None,
+        }
+    }
+
+    pub(crate) fn apply_error_pos(&mut self, query: &str) {
+        let error_pos = self
+            .position_raw()
+            .and_then(|pos_raw| pos_raw.original())
+            .and_then(|pos| ErrorPosition::from_char_pos(query, CharBasis::One(pos)));
+
+        self.error_pos = error_pos;
+    }
+
     #[inline]
     pub fn severity(&self) -> PgSeverity {
-        self.0.severity()
+        self.notice.severity()
     }
 
     /// The [SQLSTATE](https://www.postgresql.org/docs/current/errcodes-appendix.html) code for
     /// this error.
     #[inline]
     pub fn code(&self) -> &str {
-        self.0.code()
+        self.notice.code()
     }
 
     /// The primary human-readable error message. This should be accurate but
     /// terse (typically one line).
     #[inline]
     pub fn message(&self) -> &str {
-        self.0.message()
+        self.notice.message()
     }
 
     /// An optional secondary error message carrying more detail about the problem.
     /// Might run to multiple lines.
     #[inline]
     pub fn detail(&self) -> Option<&str> {
-        self.0.get(b'D')
+        self.notice.get(b'D')
     }
 
     /// An optional suggestion what to do about the problem. This is intended to differ from
     /// `detail` in that it offers advice (potentially inappropriate) rather than hard facts.
     /// Might run to multiple lines.
     #[inline]
     pub fn hint(&self) -> Option<&str> {
-        self.0.get(b'H')
+        self.notice.get(b'H')
     }
 
     /// Indicates an error cursor position as an index into the original query string; or,
     /// a position into an internally generated query.
     #[inline]
-    pub fn position(&self) -> Option<PgErrorPosition<'_>> {
-        self.0
+    pub fn position_raw(&self) -> Option<PgErrorPosition<'_>> {
+        self.notice
             .get_raw(b'P')
             .and_then(atoi)
             .map(PgErrorPosition::Original)
             .or_else(|| {
-                let position = self.0.get_raw(b'p').and_then(atoi)?;
-                let query = self.0.get(b'q')?;
+                let position = self.notice.get_raw(b'p').and_then(atoi)?;
+                let query = self.notice.get(b'q')?;
 
                 Some(PgErrorPosition::Internal { position, query })
             })
@@ -69,50 +88,50 @@ impl PgDatabaseError {
     /// stack traceback of active procedural language functions and internally-generated queries.
     /// The trace is one entry per line, most recent first.
     pub fn r#where(&self) -> Option<&str> {
-        self.0.get(b'W')
+        self.notice.get(b'W')
     }
 
     /// If this error is with a specific database object, the
     /// name of the schema containing that object, if any.
     pub fn schema(&self) -> Option<&str> {
-        self.0.get(b's')
+        self.notice.get(b's')
     }
 
     /// If this error is with a specific table, the name of the table.
     pub fn table(&self) -> Option<&str> {
-        self.0.get(b't')
+        self.notice.get(b't')
     }
 
     /// If the error is with a specific table column, the name of the column.
     pub fn column(&self) -> Option<&str> {
-        self.0.get(b'c')
+        self.notice.get(b'c')
     }
 
     /// If the error is with a specific data type, the name of the data type.
     pub fn data_type(&self) -> Option<&str> {
-        self.0.get(b'd')
+        self.notice.get(b'd')
     }
 
     /// If the error is with a specific constraint, the name of the constraint.
     /// For this purpose, indexes are constraints, even if they weren't created
     /// with constraint syntax.
     pub fn constraint(&self) -> Option<&str> {
-        self.0.get(b'n')
+        self.notice.get(b'n')
     }
 
     /// The file name of the source-code location where this error was reported.
     pub fn file(&self) -> Option<&str> {
-        self.0.get(b'F')
+        self.notice.get(b'F')
     }
 
     /// The line number of the source-code location where this error was reported.
     pub fn line(&self) -> Option<usize> {
-        self.0.get_raw(b'L').and_then(atoi)
+        self.notice.get_raw(b'L').and_then(atoi)
     }
 
     /// The name of the source-code routine reporting this error.
     pub fn routine(&self) -> Option<&str> {
-        self.0.get(b'R')
+        self.notice.get(b'R')
     }
 }
 
@@ -135,12 +154,13 @@ pub enum PgErrorPosition<'a> {
 impl Debug for PgDatabaseError {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
         f.debug_struct("PgDatabaseError")
+            .field("position", &self.error_pos)
             .field("severity", &self.severity())
             .field("code", &self.code())
             .field("message", &self.message())
             .field("detail", &self.detail())
             .field("hint", &self.hint())
-            .field("position", &self.position())
+            .field("position_raw", &self.position())
             .field("where", &self.r#where())
             .field("schema", &self.schema())
             .field("table", &self.table())
@@ -156,7 +176,12 @@ impl Debug for PgDatabaseError {
 
 impl Display for PgDatabaseError {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        f.write_str(self.message())
+        write!(f, "(code {}", self.code())?;
+        if let Some(error_pos) = self.error_pos {
+            write!(f, ", line {}, column {}", error_pos.line, error_pos.column)?;
+        }
+
+        write!(f, ") {}", self.message())
     }
 }
 
@@ -171,6 +196,10 @@ impl DatabaseError for PgDatabaseError {
         Some(Cow::Borrowed(self.code()))
     }
 
+    fn position(&self) -> Option<ErrorPosition> {
+        self.error_pos
+    }
+
     #[doc(hidden)]
     fn as_error(&self) -> &(dyn StdError + Send + Sync + 'static) {
         self
@@ -219,6 +248,43 @@ impl DatabaseError for PgDatabaseError {
     }
 }
 
+impl PgErrorPosition<'_> {
+    fn original(&self) -> Option<usize> {
+        match *self {
+            Self::Original(original) => Some(original),
+            _ => None,
+        }
+    }
+}
+
+pub(crate) trait PgResultExt {
+    fn pg_apply_error_pos(self, query: &str) -> Self;
+}
+
+impl<T> PgResultExt for Result<T, Error> {
+    fn pg_apply_error_pos(self, query: &str) -> Self {
+        self.map_err(|e| {
+            match e {
+                Error::Database(e) => {
+                    Error::Database(
+                        // Don't panic in case this gets called in the wrong context;
+                        // it'd be a bug, for sure, but far from a fatal one.
+                        // I gave the method a distinct name to call this out if it happens.
+                        e.try_downcast::<PgDatabaseError>().map_or_else(
+                            |e| e,
+                            |mut e| {
+                                e.apply_error_pos(query);
+                                e
+                            },
+                        ),
+                    )
+                }
+                other => other,
+            }
+        })
+    }
+}
+
 /// For reference: <https://www.postgresql.org/docs/current/errcodes-appendix.html>
 pub(crate) mod error_codes {
     /// Caused when a unique or primary key is violated.

tests/postgres/postgres.rs

@@ -116,7 +116,7 @@ async fn it_can_inspect_errors() -> anyhow::Result<()> {
     assert_eq!(err.severity(), PgSeverity::Error);
     assert_eq!(err.message(), "column \"f\" does not exist");
     assert_eq!(err.code(), "42703");
-    assert_eq!(err.position(), Some(PgErrorPosition::Original(8)));
+    assert_eq!(err.position_raw(), Some(PgErrorPosition::Original(8)));
     assert_eq!(err.routine(), Some("errorMissingColumn"));
     assert_eq!(err.constraint(), None);
 
@@ -151,7 +151,7 @@ async fn it_can_inspect_constraint_errors() -> anyhow::Result<()> {
         "new row for relation \"products\" violates check constraint \"products_price_check\""
     );
     assert_eq!(err.code(), "23514");
-    assert_eq!(err.position(), None);
+    assert_eq!(err.position_raw(), None);
     assert_eq!(err.routine(), Some("ExecConstraints"));
     assert_eq!(err.constraint(), Some("products_price_check"));
 
@@ -1934,3 +1934,62 @@ async fn test_issue_3052() {
         "expected encode error, got {too_large_error:?}",
     );
 }
+
+#[sqlx::test]
+async fn test_error_includes_position() -> anyhow::Result<()> {
+    let mut conn = new::<Postgres>().await?;
+
+    let err: sqlx::Error = conn.prepare(
+        "SELECT bar.foo as foo\nFORM bar"
+    )
+        .await
+        .unwrap_err();
+
+    let sqlx::Error::Database(dbe) = err else {
+        panic!("unexpected error kind {err:?}")
+    };
+
+    let pos = dbe.position().unwrap();
+
+    assert_eq!(pos.line, 2);
+    assert_eq!(pos.column, 1);
+    assert!(dbe.to_string().contains("line 2, column 1"), "{:?}", dbe.to_string());
+
+    let err: sqlx::Error = sqlx::query(
+        "SELECT bar.foo as foo\r\nFORM bar"
+    )
+        .execute(&mut conn)
+        .await
+        .unwrap_err();
+
+    let sqlx::Error::Database(dbe) = err else {
+        panic!("unexpected error kind {err:?}")
+    };
+
+    let pos = dbe.position().unwrap();
+
+    assert_eq!(pos.line, 2);
+    assert_eq!(pos.column, 1);
+    assert!(dbe.to_string().contains("line 2, column 1"), "{:?}", dbe.to_string());
+
+
+    let err: sqlx::Error = sqlx::query(
+        "SELECT foo\r\nFROM bar\r\nINNER JOIN baz USING (foo)\r\nWHERE foo=1 ADN baz.foo = 2"
+    )
+        .execute(&mut conn)
+        .await
+        .unwrap_err();
+
+    let sqlx::Error::Database(dbe) = err else {
+        panic!("unexpected error kind {err:?}")
+    };
+
+    let pos = dbe.position().unwrap();
+
+    assert_eq!(pos.line, 4);
+    assert_eq!(pos.column, 13);
+    assert!(dbe.to_string().contains("line 4, column 13"), "{:?}", dbe.to_string());
+
+
+    Ok(())
+}