errors: expose pub getters for type-erased errors

[muzarski]

Motivation

Sometimes, user might want to downcast Arc<dyn Error> to specific error type.

Actually, there is a use case for this in cpp-rust-driver, where we need to define custom SerializeRow implementation. It returns a custom error type. We would like to match against it during rust-to-C error conversion - downcasting is thus required.

See: scylladb/cpp-rust-driver#187

Pre-review checklist

• I have split my patch into logically separate commits.
• All commit messages clearly explain what they change and why.
• [ ] I added relevant tests for new features and bug fixes.
• All commits compile, pass static checks and pass test.
• PR description sums up the changes and reasons why they should be introduced.
• I have provided docstrings for the public items that I want to introduce.
• [ ] I have adjusted the documentation in ./docs/source/.
• [ ] I added appropriate Fixes: annotations to PR description.

[github-actions]

cargo semver-checks detected some API incompatibilities in this PR.
Checked commit: 2a23f06

See the following report for details:

cargo semver-checks output

./scripts/semver-checks.sh --baseline-rev 8a7bf3029436af2cc92ef5f0f05010010472d96d
+ cargo semver-checks -p scylla -p scylla-cql --baseline-rev 8a7bf3029436af2cc92ef5f0f05010010472d96d
     Cloning 8a7bf3029436af2cc92ef5f0f05010010472d96d
     Parsing scylla v0.14.0 (current)
      Parsed [  21.914s] (current)
     Parsing scylla v0.14.0 (baseline)
      Parsed [  20.488s] (baseline)
    Checking scylla v0.14.0 -> v0.14.0 (no change)
     Checked [   0.109s] 89 checks: 88 pass, 1 fail, 0 warn, 0 skip

--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.35.0/src/lints/inherent_method_missing.ron

Failed in:
  BrokenConnectionError::get_reason, previously in file /home/runner/work/scylla-rust-driver/scylla-rust-driver/target/semver-checks/git-8a7bf3029436af2cc92ef5f0f05010010472d96d/44d67517f20d27ebaef13d01c3c290b004ac7dd9/scylla/src/transport/errors.rs:744

     Summary semver requires new major version: 1 major and 0 minor checks failed
    Finished [  42.568s] scylla
     Parsing scylla-cql v0.3.0 (current)
      Parsed [  10.134s] (current)
     Parsing scylla-cql v0.3.0 (baseline)
      Parsed [  10.287s] (baseline)
    Checking scylla-cql v0.3.0 -> v0.3.0 (no change)
     Checked [   0.101s] 89 checks: 89 pass, 0 skip
     Summary no semver update required
    Finished [  20.578s] scylla-cql
make: *** [Makefile:61: semver-rev] Error 1

[muzarski]

v2: replaced public getters with custom downcast_ref implementations for type-erased error types.

[Lorak-mmk]

Maybe not very related: Error impls / deriving seem to be a bit inconsistent:

• SerializationError / DeserializationError has derive(Error) + impl Display
• BrokenConnectionError additionally has #[error("Connection broken, reason: {0}")] and no manual Display impl
• TypeCheckError has #[error(transparent)] and no manual Display impl

I wonder if there is a good reason for those differences.

[wprzytula]

Maybe not very related: Error impls / deriving seem to be a bit inconsistent:

* SerializationError / DeserializationError has derive(Error) + impl Display

* BrokenConnectionError additionally has `#[error("Connection broken, reason: {0}")]` and no manual Display impl

* TypeCheckError has `#[error(transparent)]` and no manual Display impl

I wonder if there is a good reason for those differences.

TypeCheckError should be consistent with DeserializationError. If it's not, let's fix this.

[Lorak-mmk]

Sorry, one more thing: TypeCheckError is the only one of those 4 types that has its inside pub(crate) instead of private. Can we change that? I doubt that with downcast_ref there is need to use this member directly by other places in the code.

[muzarski]

Sorry, one more thing: TypeCheckError is the only one of those 4 types that has its inside pub(crate) instead of private. Can we change that? I doubt that with downcast_ref there is need to use this member directly by other places in the code.

This member is actually used directly in scylla-cql/src/frame/response/result.rs (on line 1052), and it requires pub(crate)

[muzarski]

Flaky test transport::connection::tests::test_coalescing failed yet again...

[Lorak-mmk]

Flaky test transport::connection::tests::test_coalescing failed yet again...

Restarted

[wprzytula]

Flaky test transport::connection::tests::test_coalescing failed yet again...

Another time in a row.

[Lorak-mmk]

Well, flakiness is one more reason to change this test