boring

boring copied to clipboard

Currently, the HandshakeError type in tokio-boring provides a std::error::Error implementation and an as_io_error method that returns an Option<&std::io::Error>, as the only ways to access the potentially underlying error. There are a couple of cases where this is somewhat insufficient:

• If a user has an owned HandshakeError, and needs an owned io::Error, they have to create a new one, using the HandshakeError's cause and message, like this:

  let err: HandshakeError<_> = // ...;
  if let Some(io_err) = err.as_io_error() {
      return Err(io::Error::new(io_err.cause(), io_err.to_string()));
  }
  // ...
  

  This seems like kind of a shame, since it allocates two additional times (for the String and then again for the io::Error itself), and deallocates the original io::Error at the end of the scope. None of this should be necessary, since the HandshakeError in this case is owned and the original io::Error could be returned.

• HandshakeError only implements fmt::Debug and fmt::Display when the wrapped I/O stream it's generic over implements Debug. This means that bounding the S type with Debug is necessary for HandshakeError to implement the Error trait. In some cases, introducing a Debug bound on a generic I/O type is kind of a heavyweight requirement. Therefore, it would be nice to have a way to extract a type that always implements Error from the HandshakeError, in cases where the returned I/O stream is just going to be discarded if the handshake failed.

This branch introduces new functions on HandshakeError:

• HandshakeError::as_ssl_error, which is analogous to HandshakeError::as_io_error but returning an Option<&boring::error::ErrorStack> instead of an I/O error.

• HandshakeError::into_io_error and HandshakeError::into_ssl_error, which consume the error and return an Option<io::Error> or an Option<ErrorStack>, respectively.

  I noticed that some of the into_$TYPE methods on errors in the boring crate return Results rather than Options so that the original error can be reused if it couldn't be converted into the requested type. However, this can't easily be done in HandshakeError's into_io_error and into_ssl_error, because these methods call MidHandshakeSslStream::into_error, and (since MidHandshakeSslStream can only be constructed by the boring crate, not in tokio-boring), we can't ever get the MidHandshakeSslStream back...so we can't return the original Self type in this case.

Hopefully this should make it much easier to convert HandshakeErrors into other error types as required in user code!

Signed-off-by: Eliza Weisman [email protected]

Do be aware though that we're considering larger changes to the error types soon: #20

Hmm, if the intent is to make larger changes in the near future, is it worth moving forwards with the changes in this branch, or should I just try to make the changes proposed in #20?

should I just try to make the changes proposed in #20?

That would be really helpful! I'd love to hear if that new design fixes your issue.

Btw #142 completely removes HandshakeError.