Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ps/flatten error #1132

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ uniffi = "=0.28.1"
uuid = { version = ">=1.3.3, <2.0", features = ["serde", "v4"] }
validator = { version = "0.18.1", features = ["derive"] }
wasm-bindgen = { version = ">=0.2.91, <0.3", features = ["serde-serialize"] }
wasm-bindgen-futures = "0.4.41"

[workspace.lints.clippy]
unused_async = "deny"
Expand Down
1 change: 1 addition & 0 deletions crates/bitwarden-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ wasm-bindgen = { workspace = true, optional = true }
zeroize = { version = ">=1.7.0, <2.0", features = ["derive", "aarch64"] }
zxcvbn = { version = ">=3.0.1, <4.0", optional = true }
tsify-next = { workspace = true, optional = true }
bitwarden-error = { version = "1.0.0", path = "../bitwarden-error" }

[target.'cfg(not(target_arch="wasm32"))'.dependencies]
# By default, we use rustls as the TLS stack and rust-platform-verifier to support user-installed root certificates
Expand Down
3 changes: 2 additions & 1 deletion crates/bitwarden-core/src/client/encryption_settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
use bitwarden_crypto::{AsymmetricCryptoKey, CryptoError, KeyContainer, SymmetricCryptoKey};
#[cfg(feature = "internal")]
use bitwarden_crypto::{AsymmetricEncString, EncString, MasterKey};
use bitwarden_error::prelude::*;
use thiserror::Error;
use uuid::Uuid;

#[cfg(feature = "internal")]
use crate::error::Result;
use crate::VaultLocked;

#[derive(Debug, Error)]
#[derive(Debug, Error, FlatError)]

Check warning on line 14 in crates/bitwarden-core/src/client/encryption_settings.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-core/src/client/encryption_settings.rs#L14

Added line #L14 was not covered by tests
pub enum EncryptionSettingsError {
#[error("Cryptography error, {0}")]
Crypto(#[from] bitwarden_crypto::CryptoError),
Expand Down
23 changes: 23 additions & 0 deletions crates/bitwarden-error-macro/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
[package]
name = "bitwarden-error-macro"
version.workspace = true
authors.workspace = true
edition.workspace = true
rust-version.workspace = true
homepage.workspace = true
repository.workspace = true
license-file.workspace = true
keywords.workspace = true

[dependencies]
quote = "1.0.37"
syn = "2.0.79"

[dev-dependencies]
bitwarden-error = { path = "../bitwarden-error" }

[lib]
proc-macro = true

[lints]
workspace = true
51 changes: 51 additions & 0 deletions crates/bitwarden-error-macro/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
use proc_macro::TokenStream;
use quote::quote;
use syn::{parse_macro_input, Data, DeriveInput, Fields};

#[proc_macro_derive(FlatError)]
pub fn derive_flat_error(input: TokenStream) -> TokenStream {
let input = parse_macro_input!(input as DeriveInput);
let name = &input.ident;

// Generate match arms without converting variant names to strings
let variant_matches: Vec<_> = if let Data::Enum(data_enum) = &input.data {
data_enum
.variants
.iter()
.map(|variant| {
let variant_ident = &variant.ident;
let message = match &variant.fields {
Fields::Unnamed(_) | Fields::Named(_) => {
format!("Error: {}", variant_ident)
}
Fields::Unit => {
format!("{}", variant_ident)
}
};
quote! {
#name::#variant_ident { .. } => (stringify!(#variant_ident), #message),
}
})
.collect()
} else {
panic!("FlatError can only be derived for enums");

Check warning on line 31 in crates/bitwarden-error-macro/src/lib.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-error-macro/src/lib.rs#L31

Added line #L31 was not covered by tests
};

let expanded = quote! {
impl FlatError for #name {
fn get_variant(&self) -> &str {
match self {
#(#variant_matches)*
}.0
}

fn get_message(&self) -> &str {
match self {
#(#variant_matches)*
}.1
}
}
};

TokenStream::from(expanded)
}
50 changes: 50 additions & 0 deletions crates/bitwarden-error-macro/tests/mod.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use bitwarden_error::prelude::*;

#[test]
fn flattens_basic_enums() {
#[derive(FlatError)]
enum Errors {
Foo,
Bar,
Baz,
}

let foo = Errors::Foo;
let bar = Errors::Bar;
let baz = Errors::Baz;

assert_eq!(foo.get_variant(), "Foo");
assert_eq!(bar.get_variant(), "Bar");
assert_eq!(baz.get_variant(), "Baz");

assert_eq!(foo.get_message(), "Foo");
assert_eq!(bar.get_message(), "Bar");
assert_eq!(baz.get_message(), "Baz");
}

#[test]
fn flattens_enums_with_fields() {
#[derive(FlatError)]
enum Errors {
#[allow(dead_code)]
Foo(String),
#[allow(dead_code)]
Bar(u32),
Baz,
}

let foo = Errors::Foo("hello".to_string());
let bar = Errors::Bar(42);
let baz = Errors::Baz;

assert_eq!(foo.get_variant(), "Foo");
assert_eq!(bar.get_variant(), "Bar");
assert_eq!(baz.get_variant(), "Baz");

// The message is always "Error: <variant>"
// TODO: Add support for getting the message from the fields
// or maybe just remove get_message and rely on ToString
assert_eq!(foo.get_message(), "Error: Foo");
assert_eq!(bar.get_message(), "Error: Bar");
assert_eq!(baz.get_message(), "Baz");
}
16 changes: 16 additions & 0 deletions crates/bitwarden-error/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[package]
name = "bitwarden-error"
version.workspace = true
authors.workspace = true
edition.workspace = true
rust-version.workspace = true
homepage.workspace = true
repository.workspace = true
license-file.workspace = true
keywords.workspace = true

[dependencies]
bitwarden-error-macro = { path = "../bitwarden-error-macro" }

[lints]
workspace = true
4 changes: 4 additions & 0 deletions crates/bitwarden-error/src/flat_error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
pub trait FlatError {
fn get_variant(&self) -> &str;
fn get_message(&self) -> &str;
}
8 changes: 8 additions & 0 deletions crates/bitwarden-error/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
mod flat_error;

pub use flat_error::FlatError;

pub mod prelude {
pub use crate::FlatError;
pub use bitwarden_error_macro::FlatError;
}
1 change: 1 addition & 0 deletions crates/bitwarden-wasm-internal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ crate-type = ["cdylib"]
[dependencies]
bitwarden-core = { workspace = true, features = ["wasm", "internal"] }
bitwarden-crypto = { workspace = true, features = ["wasm"] }
bitwarden-error = { version = "1.0.0", path = "../bitwarden-error" }
bitwarden-vault = { workspace = true, features = ["wasm"] }
console_error_panic_hook = "0.1.7"
console_log = { version = "1.0.0", features = ["color"] }
Expand Down
13 changes: 13 additions & 0 deletions crates/bitwarden-wasm-internal/src/error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/**
* Error thrown by the WASM module.
* @param {string} message - Error message.
* @extends Error
*/
class WasmError extends Error {
constructor(message, name) {
super(message);
this.name = name ?? "WasmError";
}
}

exports.WasmError = WasmError;
32 changes: 26 additions & 6 deletions crates/bitwarden-wasm-internal/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,28 +1,48 @@
use bitwarden_error::FlatError;
use wasm_bindgen::prelude::*;

// Importing an error class defined in JavaScript instead of defining it in Rust
// allows us to extend the `Error` class. It also provides much better console output.
#[wasm_bindgen]
#[wasm_bindgen(module = "/src/error.js")]
extern "C" {
#[wasm_bindgen(js_name = Error)]
type JsError;
type WasmError;

#[wasm_bindgen(constructor, js_class = Error)]
fn new(message: String) -> JsError;
#[wasm_bindgen(constructor)]
fn new(message: String, name: Option<String>) -> WasmError;
}

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

pub struct GenericError(pub String);

pub struct FlattenedError {
pub variant: String,
pub message: String,
}

impl<T: ToString> From<T> for GenericError {
fn from(error: T) -> Self {
GenericError(error.to_string())
}
}

impl<T: FlatError> From<T> for FlattenedError {
fn from(error: T) -> Self {
FlattenedError {
variant: error.get_variant().to_owned(),
message: error.get_message().to_owned(),
}
}

Check warning on line 35 in crates/bitwarden-wasm-internal/src/error.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-wasm-internal/src/error.rs#L30-L35

Added lines #L30 - L35 were not covered by tests
}

impl From<GenericError> for JsValue {
fn from(error: GenericError) -> Self {
JsError::new(error.0).into()
WasmError::new(error.0, None).into()
}

Check warning on line 41 in crates/bitwarden-wasm-internal/src/error.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-wasm-internal/src/error.rs#L40-L41

Added lines #L40 - L41 were not covered by tests
}

impl From<FlattenedError> for JsValue {
fn from(error: FlattenedError) -> Self {
WasmError::new(error.message, Some(error.variant)).into()

Check warning on line 46 in crates/bitwarden-wasm-internal/src/error.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-wasm-internal/src/error.rs#L45-L46

Added lines #L45 - L46 were not covered by tests
}
}
Loading