Move from failure to thiserror (#135)

* Move from failure to thiserror

Closes #115

This is still a WIP branch, with lots of TODOs and some things about
thiserror I still can't wrap my head around. However, the heavy-lifting
is done, the failure crate has been removed from the list of
dependencies and compilation, tests, benchmarks and linting are all
green.

The two biggest things I have yet to figure out are:
1. How to deal with the errors manually defined in ser.rs and de.rs:
   they are publicly available and as soon as I touch anything I hit
   cryptic serde errors
2. How to make errors like TryFromIntError part of more abstract ones
   like ParseSchemaError, which could have a source error or just a
   String description.

* Update tests/io.rs

Co-authored-by: Joel Höner <joel@zyantific.com>

* Renaming errors + apply clippy consistently

 Rename AvroError to Error
 Removed redundant Error suffix from variants
 Introduce AvroResult shorthand alias with crate visibility
 Align clippy invocation in tests with the one in pre-commits

* Stop stressing about generic errors and add a couple more sprecific ones

* Centralize Ser and De errors into Error

The trick was implementing the ser::Error and de::Error trait for
crate::errors::Error and return Error::Ser and Error::De variants
in the implementation of the custom() method.

* SnappyCdcError as struct for consistency

* Update CHANGELOG

* Update CHANGELOG, README and add a Migration Guide page

Co-authored-by: Joel Höner <joel@zyantific.com>

.pre-commit-config.yaml

@@ -10,7 +10,7 @@
     -   id: rust-clippy
         name: Rust clippy
         description: Run cargo clippy on files included in the commit. clippy should be installed before-hand.
-        entry: cargo clippy --all-features --all --
+        entry: cargo clippy --all-targets --all-features -- -Dclippy::all
         pass_filenames: false
         types: [file, rust]
         language: system

CHANGELOG.md

@@ -5,13 +5,21 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
+### Changed
+- Introduce custom Error enum to replace all existing errors (backward-incompatible) (#135)
+- Swapped failure for thiserror (backward-incompatible) (#135)
+- Update digest crate and digest::Digest trait to 0.9 (backward-incompatible with digest::Digest 0.8) (#133)
+- Replace some manual from_str implementations with strum (#136)
+
+## Deprecated
+- Deprecate ToAvro in favor of From<T> for Value implementations (#137)
 
 ## [0.10.0] - 2020-05-31
 ### Changed
-- Writer::into_inner() now calls flush() and returns a Result
+- Writer::into_inner() now calls flush() and returns a Result (backward-incompatible)
 
 ### Added
-- Add utilited for schema compatibility check
+- Add utility for schema compatibility check
 
 ## [0.9.1] - 2020-05-02
 ### Changed
@@ -63,7 +71,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [0.6.0]- 2018-08-11
 ### Added
-- impl Send+Sync for Schema (non-backwards compatible)
+- impl Send+Sync for Schema (backwards-incompatible)
 
 ## [0.5.0] - 2018-08-06
 ### Added
@@ -76,7 +84,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [0.4.1] - 2018-06-17
 ### Changed
-- Implememented clippy suggestions
+- Implemented clippy suggestions
 
 ## [0.4.0] - 2018-06-17
 ### Changed

Cargo.toml

@@ -35,7 +35,6 @@ harness = false
 byteorder = "1.0.0"
 crc = { version = "1.3.0", optional = true }
 digest = "0.9"
-failure = "0.1.5"
 libflate = "0.1"
 num-bigint = "0.2.6"
 rand = "0.4"
@@ -44,6 +43,7 @@ serde = { version = "1.0", features = ["derive"] }
 snap = { version = "0.2.3", optional = true }
 strum = "0.18.0"
 strum_macros = "0.18.0"
+thiserror = "1.0"
 typed-builder = "0.5.1"
 uuid = { version = "0.8.1", features = ["v4"] }
 zerocopy = "0.3.0"

README.md

@@ -56,6 +56,11 @@ version = "x.y"
 features = ["snappy"]
 ```
 
+## Upgrading to a newer minor version
+
+The library is still in beta, so there might be backward-incompatible changes between minor
+versions. If you have troubles upgrading, check the [version upgrade guide](migration_guide.md).
+
 ## Defining a schema
 
 An Avro data cannot exist without an Avro schema. Schemas **must** be used while writing and
@@ -297,8 +302,7 @@ The following is an example of how to combine everything showed so far and it is
 quick reference of the library interface:
 
 ```rust
-use avro_rs::{Codec, Reader, Schema, Writer, from_value, types::Record};
-use failure::Error;
+use avro_rs::{Codec, Reader, Schema, Writer, from_value, types::Record, Error};
 use serde::{Deserialize, Serialize};
 
 #[derive(Debug, Deserialize, Serialize)]
@@ -363,10 +367,9 @@ Note that the on-disk representation is identical to the underlying primitive/co
 ```rust
 use avro_rs::{
     types::Record, types::Value, Codec, Days, Decimal, Duration, Millis, Months, Reader, Schema,
-    Writer,
+    Writer, Error,
 };
 use num_bigint::ToBigInt;
-use failure::Error;
 
 fn main() -> Result<(), Error> {
     let raw_schema = r#"
@@ -476,8 +479,7 @@ Note: Rabin fingerprinting is NOT SUPPORTED yet.
 An example of fingerprinting for the supported fingerprints:
 
 ```rust
-use avro_rs::Schema;
-use failure::Error;
+use avro_rs::{Schema, Error};
 use md5::Md5;
 use sha2::Sha256;
 
@@ -572,4 +574,5 @@ Everyone is encouraged to contribute! You can contribute by forking the GitHub r
 All contributions will be licensed under [MIT License](https://github.com/flavray/avro-rs/blob/master/LICENSE).
 
 Please consider adding documentation, tests and a line for your change under the Unreleased section in the [CHANGELOG](https://github.com/flavray/avro-rs/blob/master/CHANGELOG.md).
+If you introduce a backward-incompatible change, please consider adding instruction to migrate in the [Migration Guide](migration_guide.md)
 If you modify the crate documentation in `lib.rs`, run `make readme` to sync the README file.

README.tpl

@@ -16,4 +16,5 @@ Everyone is encouraged to contribute! You can contribute by forking the GitHub r
 All contributions will be licensed under [MIT License](https://github.com/flavray/avro-rs/blob/master/LICENSE).
 
 Please consider adding documentation, tests and a line for your change under the Unreleased section in the [CHANGELOG](https://github.com/flavray/avro-rs/blob/master/CHANGELOG.md).
+If you introduce a backward-incompatible change, please consider adding instruction to migrate in the [Migration Guide](migration_guide.md)
 If you modify the crate documentation in `lib.rs`, run `make readme` to sync the README file.

examples/to_value.rs

@@ -1,5 +1,4 @@
-use avro_rs::to_value;
-use failure::Error;
+use avro_rs::{to_value, Error};
 use serde::{Deserialize, Serialize};
 
 #[derive(Debug, Deserialize, Serialize)]

migration_guide.md

@@ -0,0 +1,79 @@
+# Migration Guide
+## Unreleased
+- A custom `Error` enum has been introduced to replace all existing errors and
+  the `failure` crate has been replaced by `thiserror`. 
+
+  This means that all public functions returning `Result<T, failure::Error>`
+  will now return `Result<T, avro::Error>` and that you can pattern match on
+  `Error` variants if you want to gather more information about the error.
+
+  For example, code that used to be like this:
+  ```rust
+  match decoded {
+      Ok(msg) => Ok(msg.to_string()),
+      Err(ref e) => match e.downcast_ref::<SchemaResolutionError>() {
+          Some(_) => Ok("default".to_string()),
+          None => Err(format!("Unexpected error: {}", e)),
+      },
+  }
+  ```
+
+  now becomes:
+  ```rust
+  match decoded {
+      Ok(msg) => Ok(msg.to_string()),
+      Err(Error::SchemaResolution(_)) => Ok("default".to_string()),
+      Err(ref e) => Err(format!("Unexpected error: {}", e)),
+  }
+  ```
+
+  Please note that all instances of:
+  - `DecodeError`
+  - `ValidationError`
+  - `DeError`
+  - `SerError`
+  - `ParseSchemaError`
+  - `SchemaResolutionError`
+
+  must be replaced by `Error`.
+
+- The `ToAvro` trait has been deprecated in favor of `From<T>` for `Value` implementations.
+
+  Code like the following:
+  ```rust
+  use crate::types::{Record, ToAvro, Value};
+
+  let expected: Value = record.avro();
+  ```
+
+  should be updated to:
+
+  ```rust
+  use crate::types::{Record, Value};
+
+  let expected: Value = record.into(); 
+  ```
+
+  Using the `ToAvro` trait will result in a deprecation warning. The trait will
+  be removed in future versions.
+
+- The `digest` crate has been updated to version `0.9`. If you were using the
+  `digest::Digest` trait from version `0.8`, you must update to the one defined
+  in `0.9`.
+
+## 0.10.0
+- `Writer::into_inner()` now calls `flush()` and returns a `Result`.
+
+  This means that code like
+  ```rust
+  writer.append_ser(test)?;
+  writer.flush()?;
+  let input = writer.into_inner();
+  ```
+
+  can be simplified into
+  ```rust
+  writer.append_ser(test)?;
+  let input = writer.into_inner()?; 
+  ```
+  There is no harm in leaving old calls to `flush()` around.

src/codec.rs

@@ -1,6 +1,6 @@
 //! Logic for all supported compression codecs in Avro.
+use crate::errors::{AvroResult, Error};
 use crate::types::Value;
-use failure::Error;
 use libflate::deflate::{Decoder, Encoder};
 use std::io::{Read, Write};
 use strum_macros::{EnumString, IntoStaticStr};
@@ -30,12 +30,13 @@ impl From<Codec> for Value {
 
 impl Codec {
     /// Compress a stream of bytes in-place.
-    pub fn compress(self, stream: &mut Vec<u8>) -> Result<(), Error> {
+    pub fn compress(self, stream: &mut Vec<u8>) -> AvroResult<()> {
         match self {
             Codec::Null => (),
             Codec::Deflate => {
                 let mut encoder = Encoder::new(Vec::new());
                 encoder.write_all(stream)?;
+                // Deflate errors seem to just be io::Error
                 *stream = encoder.finish().into_result()?;
             }
             #[cfg(feature = "snappy")]
@@ -58,7 +59,7 @@ impl Codec {
     }
 
     /// Decompress a stream of bytes in-place.
-    pub fn decompress(self, stream: &mut Vec<u8>) -> Result<(), Error> {
+    pub fn decompress(self, stream: &mut Vec<u8>) -> AvroResult<()> {
         *stream = match self {
             Codec::Null => return Ok(()),
             Codec::Deflate => {
@@ -69,7 +70,6 @@ impl Codec {
             }
             #[cfg(feature = "snappy")]
             Codec::Snappy => {
-                use crate::util::DecodeError;
                 use byteorder::ByteOrder;
 
                 let decompressed_size = snap::decompress_len(&stream[..stream.len() - 4])?;
@@ -80,11 +80,10 @@ impl Codec {
                 let actual_crc = crc::crc32::checksum_ieee(&decoded);
 
                 if expected_crc != actual_crc {
-                    return Err(DecodeError::new(format!(
-                        "bad Snappy CRC32; expected {:x} but got {:x}",
-                        expected_crc, actual_crc
-                    ))
-                    .into());
+                    return Err(Error::SnappyCrcError {
+                        expected: expected_crc,
+                        found: actual_crc,
+                    });
                 }
                 decoded
             }