Export `Result` and `Error` types given by `request::*`

[jakehamtexas]

Problem

When working with the API of the Plaid client, it is difficult to use the await'd result of various requests because the Result<T, E> given be these request functions comes from the crate httpclient. Matching on the result, handling specific error cases differently, and otherwise doing anything short of just .unwrap'ing the returned value are not possible unless the consumer crate (i.e. my project, using plaid as a dependency) maintains its own dependency for httpclient. If the underlying version of this crate were to change (or worse! the http client library is entirely changed), this would create an issue with the consumer crate's dependencies that may not be obviously solvable at first glance (or at least not without digging through the Cargo.toml of the plaid crate to see what version/library needs to be updated to).

Ideal Solution

plaid exports its own Result type and Error enums, which may practically be thin wrappers around the httpclient versions. As long as the dependency on httpclient (or another crate) is not exposed in the API of the plaid crate, the problem should be mitigated.

Less Ideal, But Still Workable Solution

plaid re-exports the httpclient definitions for Result and Error, so that at least the version of httpclient that is needed is managed by the plaid crate.

[kurtbuilds]

Great point. Why do you think the re-export is a less than ideal solution?

[jakehamtexas]

Oh, hi! Thank you for such a prompt response.

The reason that re-exporting the library's dependencies is less than ideal is because it shifts the burden to maintain changes to that dependency onto the library consumer.

We can pretend we have a library installed in plaid-rs called foolib. we use the library to make HTTP calls, and we translate the errors to the consumer transparently, without wrapping them.

// foolib@1.0/lib.rs
pub enum FooError {
  Bar,
  Baz,
}

pub type Result<T, E = FooError> = std::result::Result<T, E>;

In plaid-rs, we re-export foolib's definitions for Error and Result, and a function that uses foolib, and its Result type.

// plaid-rs@1.0/lib.rs
pub use foolib:{FooError, Result};

pub fn do_foo(v: u32) -> foolib::Result<u32> {
  let res = foolib::do(v)?;
  foolib::do_it_for_real_this_time(res)
}

We can imagine an innocent consumer of the API library, for whom plaid::Result<T> is just a type alias for foolib::Result<T>.

// consumer-rs@1.0/lib.rs
fn doing_my_own_foo() -> std::result::Result<u32, String> {
  plaid::do_foo(2).map_err(|e| match e {
    plaid::FooError::Bar => "FooBar was the problem!".to_string(),
    plaid::FooError::Baz => "FooBaz was the problem!".to_string(),
  })
}

Foo has a major version bump, and changes the FooError enum.

// foolib@2.0/lib.rs
pub enum FooError {
-  Bar,
+  Bar(u32),
-    Baz,
+    Bing,
}

When we upgrade the library in plaid-rs, we increment our semver (tangentially: maybe we do it incorrectly by mistake):

// plaid-rs@1.1/lib.r
// 
// To note, to the untrained eye, updating `foolib` might appear as a
// less-than-major semver bump because nothing we wrote had to change.

// Notice that nothing had to change here, lucky us.
pub fn do_foo(v: u32) -> foolib::Result<u32> {
  let res = foolib::do(v)?;
  foolib::do_it_for_real_this_time(res)
}

Maybe we forget that the types for Result and Error that are part of our public API aren't under our control.

Finally, we can see the downstream effect for the innocent consumer:

// consumer-rs@?/lib.rs
// Calamity
fn doing_my_own_foo() -> std::result::Result<u32, String> {
  plaid::do_foo(2).map_err(|e| match e {
    plaid::FooError::Bar(_) => "FooBar was the problem!".to_string(),
    // plaid::FooError::Baz => "FooBaz was the problem!".to_string(),
    plaid::FooError:Bing => "FooBing was the problem!".to_string(),
  })
}

I can imagine that not every FooError is going to be relevant to a consumer of plaid-rs (maybe they're too low level to be usable from an indirect perspective), so they're hard to manage gracefully. Also, allowing there to be a higher maintenance burden for use of the library is bad for the developer experience, IMO.

In the case where plaid-rs exports its own Error and Result types, the burden to maintain updates to the dependency is shifted to plaid-rs.

pub enum Error {
  FooError(foolib::FooError),
  TheConsumerReallyCaresAboutThis(u32),
}

impl From<foolib::FooError> for Error {
  fn from(value: foolib::FooError) -> Self {
    match value {
      FooError::Bing => Error::Foo(value)
      FooError::Bar(a) => Error::TheConsumerReallyCaresAboutThis(a),
    }
  }
}

Now, if the FooError enum changes, we must handle that in our From. Since in plaid-rs we are direct consumers of FooError, we have more context that the consumer about what error information is useful and what isn't.

pub enum Error {
  FooError(foolib::FooError),
  TheConsumerReallyCaresAboutThis(u32),
  AlsoCaresAboutThis(String),
}

impl From<foolib::FooError> for Error {
  fn from(value: foolib::FooError) -> Self {
    match value {
      FooError::Bing | FooError::Baobinga(_) => Error::Foo(value),
      FooError::Bar(a) => Error::TheConsumerReallyCaresAboutThis(a),
      FooError::Bazinga => Error::AlsoCaresAboutThis("Wow what an error that was".to_string()),
    }
  }
}

I decided to put a PR together that does more of the latter approach, if you'd like to see what a solution like this would look like (in my opinion). https://github.com/libninjacom/plaid-rs/pull/13

[kurtbuilds]

In order to stay up to date with the spec, this repo only accepts extensions to existing structs and can't accept PRs that modify existing signatures (as they'll be overwritten on next update.)

I've updated it to use your former suggestion of exporting the error types from httpclient.