Add StackError and intergrate into the code

[YaroslavLitvinov]

Related to issue #1040 that currently contains a basic description, which will be extended using the principles introduced in this PR.

Improve error handling: Phase 1

(Formerly: WIP Streamline error handling 1st attempt)

This PR :

• Adds #[stack_trace_debug] that is extending our Snafu errors
  • error_stack_trace output_msg is returned and printed at the IntoResponse on api-ui.
• Single Result per api-ui crate
• Clean Box usage within Errors
• Updates insta snaphots
• It has many things but focus was mainly on reworking a way how we work with errors, applying coding conventions on some crates.

Draft proposal to be added to Contributing rules

• Errors defined in a crate should only be created inside that crate. Normally, errors should not be constructed outside of the crate. Generated Snafu selectors' visibility should be restricted to crate level: #[snafu(visibility(pub(crate)))];
• Define Snafu errors with display messages in dedicated errors.rs files. Avoid using inline error messages outside of those files;
• Naming convention: Use Error as the name for the main error type at the crate level;
• Design convention: Use Result<> per crate; Print error stack trace on a crate level Error;
• Derive errors from #[error_stack_trace::debug];
• If your error enum variant has a nested error, name the field either source or error;
• If the nested error cannot derive from #[error_stack_trace::debug], rename the field from source to error, and annotate it with #[snafu(source)] to support context selectors;
• Add a location field: #[snafu(implicit)] location: Location;
• Use .context(), build(), fail(), and into_error() from the generated Snafu types as the standard pattern;
• Using widely .map_err(...) for error handling is generally considered an anti-pattern;
• When a Result<T> needs to return a specific error, prefer using Snafu context selectors like .context(...), which can be chained. Avoid manually constructing Snafu errors;
• Manual error construction may be needed in edge cases, such as when dealing with boxed, non-Snafu, or external errors;
• If you create an error manually, include a comment explaining why;
• #[snafu(transparent)] doesn't have own context selector generated (No WhateverSnafu generated).
  Though transparent has Into conversion, that called implicitly on ? operation, like here: .context(...)?
• Use transparent when you want to wrap an underlying error without adding a new error message or context.

Dealing with foreign / non Snafu errors (not derived from #[error_stack_trace::debug] )

#[snafu(display("Object store error: {error}"))]
ObjectStore {
  #[snafu(source)] // <----- NOTE THIS
  error: ObjectStoreError,
  #[snafu(implicit)]
  location: Location,
},

Boxing errors

In case of clippy warning: clippy::result_large_err, it may need to Box error. Using Snafu in the proper way, in the wast majority of the cases, doesn't require creating Boxes manually as well as doing boxes conversions.

#[snafu(display("S3Tables error: {error}"))]
S3Tables {
  #[snafu(source(from(S3tablesError, Box::new)))]
  source: Box<S3tablesError>,
  #[snafu(implicit)]
  location: Location,
},

[github-actions]

✅ SLT Targeted Testing: No relevant SLT tests found for the changes in this PR. No testing required.

[github-actions]

✅ SLT Targeted Testing: No relevant SLT tests found for the changes in this PR. No testing required.

[github-actions]

✅ SLT Targeted Testing: No relevant SLT tests found for the changes in this PR. No testing required.

[rampage644]

@YaroslavLitvinov Is it ready for review or you're still working on it?

[rampage644]

Also, what is an example for For external error .context() doesn't work, so .context() replaced by .map_err() ? Just want make sure I understand.

It should be possible to keep .context() usage with the following #snafu decorator over error field:

    #[snafu(display("Object store error: {error}"))]
    ObjectStore {
        #[snafu(source)] // <----- NOTE THIS
        error: ObjectStoreError,
        #[snafu(implicit)]
        location: Location,
    },

Then it would be possible to apply .context() to ObjectStoreError in the code

[YaroslavLitvinov]

Also, what is an example for For external error .context() doesn't work, so .context() replaced by .map_err() ? Just want make sure I understand.

It should be possible to keep .context() usage with the following #snafu decorator over error field:

    #[snafu(display("Object store error: {error}"))]
    ObjectStore {
        #[snafu(source)] // <----- NOTE THIS
        error: ObjectStoreError,
        #[snafu(implicit)]
        location: Location,
    },

Then it would be possible to apply .context() to ObjectStoreError in the code

Ok, I see and surprised it is working in this way. Will update my PR.

[github-actions]

✅ SLT Targeted Testing: No relevant SLT tests found for the changes in this PR. No testing required.

[github-actions]

SQL Logic Tests Results ❌

Coverage by SLT File

• ifnull: 1/1 (100.0%)
• to_timestamp: 0/10 (0.0%)
• create-table: 23/40 (57.5%)
• system_stream_has_data: 0/2 (0.0%)
• count: 2/4 (50.0%)
• to_date: 0/9 (0.0%)
• cast: 0/5 (0.0%)
• to_char: 0/6 (0.0%)
• from: 2/2 (100.0%)
• try_to_timestamp: 0/1 (0.0%)
• update: 5/7 (71.4%)
• to_binary: 0/4 (0.0%)
• coalesce: 0/1 (0.0%)
• insert: 22/25 (88.0%)
• nullif: 1/1 (100.0%)
• st_makepoint: 0/1 (0.0%)
• select: 19/20 (95.0%)
• merge: 1/17 (5.9%)
• max: 4/4 (100.0%)
• drop-table: 4/6 (66.7%)
• to_decimal: 0/6 (0.0%)
• delete: 3/12 (25.0%)
• system_wait: 0/2 (0.0%)
• try_cast: 2/4 (50.0%)
• try_to_date: 0/1 (0.0%)
• min: 3/4 (75.0%)
• join: 3/11 (27.3%)
• to_geography: 0/2 (0.0%)
• to_boolean: 0/2 (0.0%)
• to_double: 0/2 (0.0%)
• avg: 0/4 (0.0%)
• sum: 1/5 (20.0%)
• truncate-table: 1/6 (16.7%)

Overall: 97/227 (42.7%)

[github-actions]

✅ SLT Targeted Testing: No relevant SLT tests found for the changes in this PR. No testing required.

[rampage644]

I went though this PR this time diagonally (just skimming mostly) as I dived deeper over the course of previous week.

LGTM as a start - approving the general approach and guidlelines. I prefer not to block this PR with nitpicks, but I assume there will be follow up PRs.

[github-actions]

✅ SLT Targeted Testing: No relevant SLT tests found for the changes in this PR. No testing required.

[rampage644]

reapprove