Being able to choose the error type in tower-test

[walfie]

Currently in tower-test, the Mock service's error type is fixed to Box<dyn Error + Send + Sync>.

I guess it's possible to work around it at the moment by creating a wrapper Service that does boxing/downcasting of errors, but are you open to the idea of having the error type be configurable? (it could even be a type param that defaults to Box<dyn Error + Send + Sync> if unspecified)

[LucioFranco]

I think we'd be open to it, Im not quite sure yet what the implications of it are. What is your use case where downcasting doesn't work?

[walfie]

I had a case which was something like:

struct Worker<S> {
    service: S
}

impl<S> Worker<S>
where
    S: Service<Request, Response = Response>,
    S::Error: std::error::Error + Send + Sync + 'static,
{
    async fn do_some_work(&mut self) -> anyhow::Result<String> {
        // ...poll_ready on inner service

        let result = self.service
            .call(make_request())
            .await
            .with_context(|| "Oh no")?; // from `anyhow::Context`, wraps an `Error`

        // do something with result

        Ok(...)
    }
}

and wanted to test the logic inside do_some_work by constructing a Worker with a Mock service and mocking some requests/responses.

However, since Mock has an error type fixed to Box<dyn std::error::Error + Send + Sync>, which actually doesn't implement std::error::Error (due to some limitations), I couldn't use it as S in this case.

But now that I think about it, I could just have a fixed error type in my test and wrap the Mock with a Service that does the downcasting like I mentioned, so maybe this isn't a great example.

I'm mostly thinking that in application-level tests, it would be convenient to have some way to just use a Mock in places like this, without having to write a layer on top of it (or maybe that downcasting layer itself is a helper that could be added on top of the existing Mock). The current setup wouldn't work for error types that aren't std::error::Error or Send or Sync, although I'm not sure how common that is.

[LucioFranco]

You should be able to change S to be S::Error: Into<Box<dyn std::error::Error + ..>>

[walfie]

I think that would solve the use of ?, although things like the with_context call (or anything else that requires that the type be a std::error::Error) wouldn't work.

Although looking at the code, I realized the use of Box currently allows the mock to yield a Closed error, which wouldn't be possible if the error was fixed to a specific type (without some redesigns). It seems like supporting this feature might require more changes than I originally thought.

[LucioFranco]

Yeah, that makes sense, we probably want to revisit tower-test at some point soon.

[conblem]

For anyone stumbling up on this issue, just searching for a way to get something which implements std::error::Error out of the Mock Service i did the following:

let (service, mut handle) = mock::spawn();
let mut service = service.map_result(|res| res.map_err(Arc::<dyn Error + Send + Sync>::from));

Since rust 1.52.0 Arc<T: std::error::Error> implements std::error::Error: https://github.com/rust-lang/rust/pull/80553. Additionally now the Error also implements Clone if that is needed, which was the case for me. With this solution there is no reason to implement a wrapper type for Box<dyn Error + Send + Sync>.

Cheers

[conblem]

Just found the map_err method on the ServiceExt, using that we can write the same like this:

let (service, mut handle) = mock::spawn();
let mut service = service.map_err(Arc::<dyn Error + Send + Sync>::from);

Pretty sweet

[slinkydeveloper]

Stumbled on this, It would be nice for Error to be a generic. I would also be fine with panicking in case of Closed error.

[LucioFranco]

Yeah, feel free to open a PR, not sure how big the refactor is gonna be tho.