Support backtraces for diesel errors

[hasezoey]

Currently diesel Errors does not have a backtrace attached, this issue is proposing to add (optional) backtrace support, for now via a manual .backtrace() function and in the future via std::error::Error::provide.

from my quick glance at the diesel error code, this would require some refactoring to have all errors either made via a function (like new) to add a backtrace at construction, or manually create a backtrace when constructing a function like:

struct TopError {
  source: diesel::Error,
  #[cfg(feature = "backtrace")]
  backtrace: std::backtrace::Backtrace,
}

impl TopError {
  pub fn new(source: diesel::Error) -> Self {
    Self {
      source,
      #[cfg(feature = "backtrace")]
      backtrace: std::backtrace::Backtrace::capture(),
    }
  }

  pub fn backtrace(&self) -> &std::backtrace::Backtrace {
    &self.backtrace
  }
}

// implement all From<> variants that diesel::Error also implements
impl<T> From<T> for TopError
where
  T: Into<diesel::Error>,
{
  fn from(value: T) -> Self {
    return Self::new(value.into());
  }
}

...and the same again for all separate errors diesel has

why?
because this would make tracking down issues easier, for example tracking down why a specific chrono error happens (because chrono errors are not great) or for diesel Err(diesel::Error) reports

[weiznich]

Doing such drastical changes to the error type is not possible without another major diesel version bump. We do not plan to issue another major diesel version for the next year or two.

That written: I think we wan work on improving individual error messages if you provide examples of such "bad" messages. I also generally feel that this is the better solution here as such a backtrace wouldn't be that helpful for users anyway.

[hasezoey]

That written: I think we wan work on improving individual error messages if you provide examples of such "bad" messages. I also generally feel that this is the better solution here as such a backtrace wouldn't be that helpful for users anyway.

for me the error was something along the lines of

let res = /* ... */.first::<Self>();
println!("res {:#?}", res);
println!("pretty {}", res.unwrap_err());

res Err(
    DeserializationError(
        ParseError(
            TooLong,
        ),
    ),
)
pretty trailing input

which is kinda cryptic and not specifying that chrono / column type is the problem, a backtrace would have lead me directly to

diesel/diesel/src/sqlite/types/date_and_time/chrono.rs

Line 17 in 13c2374

.map_err(Into::into)

, from which i would know that NaiveDate would be the problem and then i could think that parse_from_str is the problem and search from there; turned out that i needed to replace chrono::NaiveDate with chrono::NaiveDateTime (the former was generated by print-schema from sql definition created_at DATE NOT NULL DEFAULT now())

possible fixes to this (without backtrace) would be:

• chrono adds some better errors with messages of which type is the problem and what failed
• diesel adds error information like which column and "into" type failed

[weiznich]

As written above: I'm open to accept a PR that improves this specific error (and similar ones as well). What wouldn't be possible is including something like column names into the error message as that information is just not there at the point the error message gets generated.

turned out that i needed to replace chrono::NaiveDate with chrono::NaiveDateTime (the former was generated by print-schema from sql definition created_at DATE NOT NULL DEFAULT now())

print-schema does not generate any of these types as it does not generate the rust side types at all.

[hasezoey]

print-schema does not generate any of these types as it does not generate the rust side types at all.

diesel/diesel_cli/src/infer_schema_internals/sqlite.rs

Lines 260 to 263 in 13c2374

	} else if type_name == "datetime" || type_name == "timestamp" {
	String::from("Timestamp")
	} else if type_name == "date" {
	String::from("Date")

diesel/diesel/src/sql_types/mod.rs

Lines 270 to 288 in 13c2374

	/// The date SQL type.
	///
	/// ### [`ToSql`](crate::serialize::ToSql) impls
	///
	/// - [`chrono::NaiveDate`][NaiveDate] with `feature = "chrono"`
	/// - [`time::Date`][Date] with `feature = "time"`
	///
	/// ### [`FromSql`](crate::deserialize::FromSql) impls
	///
	/// - [`chrono::NaiveDate`][NaiveDate] with `feature = "chrono"`
	/// - [`time::Date`][Date] with `feature = "time"`
	///
	/// [NaiveDate]: https://docs.rs/chrono/*/chrono/naive/struct.NaiveDate.html
	/// [Date]: https://docs.rs/time/0.3.9/time/struct.Date.html
	#[derive(Debug, Clone, Copy, Default, QueryId, SqlType)]
	#[diesel(postgres_type(oid = 1082, array_oid = 1182))]
	#[diesel(sqlite_type(name = "Text"))]
	#[diesel(mysql_type(name = "Date"))]
	pub struct Date;

yes it does not generate chrono::NaiveDate directly, but was still using the "wrong" type (i tried to make a PR, but ended up not doing it because of the date and datetime difference and that sqlite enforces neither, but all sqlite time functions expect a ISO time (like now() generates))

and having column -> Date in the diesel schema and trying to use chrono::NaiveDateTime and derive(AsChangeset) does not work and removing that derive, there is a error along the lines of the trait load_dsl::private::CompatibleType<TableStruct, Sqlite> is not satisfied when the schema uses column -> Date and the TableStruct having pub column: chrono::NaiveDateTime (changing to schema column -> Timestamp works)

[weiznich]

Just to be clear: It's an explicit decision to map date to Date and Datetime to Timestamp, otherwise that wouldn't be spelled out that explicitly there. As you already wrote: Sqlite does not have distinct types for neither so this is just diesel guessing these types there. And yes it's also expected that you cannot map a Date column to a NativeDateTime as that column as no time component, while the rust side type has that component. (At that level it does not matter whether that time component would be returned by the now() function or not). These compiler errors are fully expected as you have a mismatch between your schema and your rust type there. They are an essential part of what diesel tries to provide: A compile time checked mapping between your schema and your rust types.