Skip to content

Commit

Permalink
Replace error strings with enums
Browse files Browse the repository at this point in the history
This is useful in `no alloc` scenarios and in cases where String is not
available (such as the Linux kernel).
  • Loading branch information
ariel-miculas committed Jun 27, 2023
1 parent 0305fdf commit 5414689
Show file tree
Hide file tree
Showing 19 changed files with 548 additions and 312 deletions.
8 changes: 6 additions & 2 deletions capnp-rpc/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,12 +355,16 @@ fn to_pipeline_ops(
}

fn from_error(error: &Error, mut builder: exception::Builder) {
builder.set_reason(&error.description);
builder.set_reason(&error.to_string());
let typ = match error.kind {
::capnp::ErrorKind::Failed => exception::Type::Failed,
::capnp::ErrorKind::Overloaded => exception::Type::Overloaded,
::capnp::ErrorKind::Disconnected => exception::Type::Disconnected,
::capnp::ErrorKind::Unimplemented => exception::Type::Unimplemented,
::capnp::ErrorKind::SettingDynamicCapabilitiesIsUnsupported => {
exception::Type::Unimplemented
}
_ => exception::Type::Failed,
};
builder.set_type(typ);
}
Expand All @@ -378,7 +382,7 @@ fn remote_exception_to_error(exception: exception::Reader) -> Error {
_ => (::capnp::ErrorKind::Failed, "(malformed error)"),
};
Error {
description: format!("remote exception: {reason}"),
extra: format!("remote exception: {reason}"),
kind,
}
}
Expand Down
4 changes: 2 additions & 2 deletions capnp-rpc/test/reconnect_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ macro_rules! assert_err {
let e1 = $e1;
let e2 = $e2;
assert_eq!(e1.kind, e2.kind);
if !e1.description.ends_with(&e2.description) {
assert_eq!(e1.description, e2.description);
if !e1.extra.ends_with(&e2.extra) {
assert_eq!(e1.extra, e2.extra);
}
};
}
Expand Down
4 changes: 1 addition & 3 deletions capnp-rpc/test/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -335,9 +335,7 @@ fn pipelining_return_null() {
let cap = request.send().pipeline.get_cap();
match cap.foo_request().send().promise.await {
Err(ref e) => {
if e.description
.contains("Message contains null capability pointer")
{
if e.extra.contains("Message contains null capability pointer") {
Ok(())
} else {
Err(Error::failed(format!(
Expand Down
3 changes: 2 additions & 1 deletion capnp/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ quickcheck = { version = "1", optional = true }
quickcheck = "1"

[features]
default = ["std"]
alloc = []
default = ["std", "alloc"]

rpc_try = []

Expand Down
24 changes: 10 additions & 14 deletions capnp/src/dynamic_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::dynamic_value;
use crate::introspect::{Type, TypeVariant};
use crate::private::layout::{self, PrimitiveElement};
use crate::traits::{IndexMove, ListIter};
use crate::Result;
use crate::{Error, ErrorKind, Result};

/// A read-only dynamically-typed list.
#[derive(Copy, Clone)]
Expand Down Expand Up @@ -321,13 +321,13 @@ impl<'a> Builder<'a> {
.reborrow()
.get_pointer_element(index)
.set_list(&list.reader, false),
(TypeVariant::AnyPointer, _) => Err(crate::Error::failed(
"List(AnyPointer) not supported".into(),
)),
(TypeVariant::Capability, dynamic_value::Reader::Capability(_)) => Err(
crate::Error::failed("List(Capability) not supported".into()),
),
(_, _) => Err(crate::Error::failed("Type mismatch".into())),
(TypeVariant::AnyPointer, _) => {
Err(Error::from_kind(ErrorKind::ListAnyPointerNotSupported))
}
(TypeVariant::Capability, dynamic_value::Reader::Capability(_)) => {
Err(Error::from_kind(ErrorKind::ListCapabilityNotSupported))
}
(_, _) => Err(Error::from_kind(ErrorKind::TypeMismatch)),
}
}

Expand All @@ -348,9 +348,7 @@ impl<'a> Builder<'a> {
| TypeVariant::Float64
| TypeVariant::Enum(_)
| TypeVariant::Struct(_)
| TypeVariant::Capability => {
Err(crate::Error::failed("Expected a list or blob.".into()))
}
| TypeVariant::Capability => Err(Error::from_kind(ErrorKind::ExpectedAListOrBlob)),
TypeVariant::Text => Ok(self
.builder
.get_pointer_element(index)
Expand Down Expand Up @@ -378,9 +376,7 @@ impl<'a> Builder<'a> {
)
.into()),
},
TypeVariant::AnyPointer => Err(crate::Error::failed(
"List(AnyPointer) not supported.".into(),
)),
TypeVariant::AnyPointer => Err(Error::from_kind(ErrorKind::ListAnyPointerNotSupported)),
}
}
}
Expand Down
62 changes: 28 additions & 34 deletions capnp/src/dynamic_struct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use crate::introspect::TypeVariant;
use crate::private::layout;
use crate::schema::{Field, StructSchema};
use crate::schema_capnp::{field, node, value};
use crate::Result;
use crate::{dynamic_list, dynamic_value};
use crate::{Error, ErrorKind, Result};

fn has_discriminant_value(reader: field::Reader) -> bool {
reader.get_discriminant_value() != field::NO_DISCRIMINANT
Expand All @@ -18,7 +18,7 @@ pub(crate) fn struct_size_from_schema(schema: StructSchema) -> Result<layout::St
pointers: s.get_pointer_count(),
})
} else {
Err(crate::Error::failed("not a struct".into()))
Err(Error::from_kind(ErrorKind::NotAStruct))
}
}

Expand Down Expand Up @@ -160,16 +160,14 @@ impl<'a> Reader<'a> {
(TypeVariant::Capability, value::Interface(())) => {
Ok(dynamic_value::Reader::Capability(dynamic_value::Capability))
}
_ => Err(crate::Error::failed("field and default mismatch".into())),
_ => Err(Error::from_kind(ErrorKind::FieldAndDefaultMismatch)),
}
}
field::Group(_) => {
if let TypeVariant::Struct(schema) = ty.which() {
Ok(Reader::new(self.reader, schema.into()).into())
} else {
Err(crate::Error::failed(
"group field but type is not Struct".into(),
))
Err(Error::from_kind(ErrorKind::GroupFieldButTypeIsNotStruct))
}
}
}
Expand All @@ -184,7 +182,7 @@ impl<'a> Reader<'a> {
/// Otherwise, returns None.
pub fn which(&self) -> Result<Option<Field>> {
let node::Struct(st) = self.schema.get_proto().which()? else {
return Err(crate::Error::failed("not a struct".into()))
return Err(Error::from_kind(ErrorKind::NotAStruct))
};
if st.get_discriminant_count() == 0 {
Ok(None)
Expand All @@ -202,7 +200,7 @@ impl<'a> Reader<'a> {
let proto = field.get_proto();
if has_discriminant_value(proto) {
let node::Struct(st) = self.schema.get_proto().which()? else {
return Err(crate::Error::failed("not a struct".into()))
return Err(Error::from_kind(ErrorKind::NotAStruct))
};

let discrim = self
Expand Down Expand Up @@ -392,16 +390,14 @@ impl<'a> Builder<'a> {
(TypeVariant::Capability, value::Interface(())) => Ok(
dynamic_value::Builder::Capability(dynamic_value::Capability),
),
_ => Err(crate::Error::failed("field and default mismatch".into())),
_ => Err(Error::from_kind(ErrorKind::FieldAndDefaultMismatch)),
}
}
field::Group(_) => {
if let TypeVariant::Struct(schema) = ty.which() {
Ok(Builder::new(self.builder, schema.into()).into())
} else {
Err(crate::Error::failed(
"group field but type is not Struct".into(),
))
Err(Error::from_kind(ErrorKind::GroupFieldButTypeIsNotStruct))
}
}
}
Expand All @@ -414,7 +410,7 @@ impl<'a> Builder<'a> {

pub fn which(&self) -> Result<Option<Field>> {
let node::Struct(st) = self.schema.get_proto().which()? else {
return Err(crate::Error::failed("not a struct".into()))
return Err(Error::from_kind(ErrorKind::NotAStruct))
};
if st.get_discriminant_count() == 0 {
Ok(None)
Expand Down Expand Up @@ -518,28 +514,26 @@ impl<'a> Builder<'a> {
dynamic_value::Reader::Data(t) => target.set_as(t),
dynamic_value::Reader::Struct(s) => target.set_as(s),
dynamic_value::Reader::List(l) => target.set_as(l),
dynamic_value::Reader::Capability(_) => {
Err(crate::Error::unimplemented(
"setting dynamic capabilities is unsupported".into(),
))
}
_ => Err(crate::Error::failed(
"cannot set AnyPointer field to a primitive value".into(),
dynamic_value::Reader::Capability(_) => Err(Error::from_kind(
ErrorKind::SettingDynamicCapabilitiesIsUnsupported,
)),
_ => Err(Error::from_kind(
ErrorKind::CannotSetAnyPointerFieldToAPrimitiveValue,
)),
}
}
(TypeVariant::Capability, _, _) => Err(crate::Error::unimplemented(
"setting dynamic capabilities is unsupported".into(),
(TypeVariant::Capability, _, _) => Err(Error::from_kind(
ErrorKind::SettingDynamicCapabilitiesIsUnsupported,
)),
_ => Err(crate::Error::failed("type mismatch".into())),
_ => Err(Error::from_kind(ErrorKind::TypeMismatch)),
}
}
field::Group(_group) => {
let dynamic_value::Reader::Struct(src) = value else {
return Err(crate::Error::failed("not a struct".into()))
return Err(Error::from_kind(ErrorKind::NotAStruct))
};
let dynamic_value::Builder::Struct(mut dst) = self.reborrow().init(field)? else {
return Err(crate::Error::failed("not a struct".into()))
return Err(Error::from_kind(ErrorKind::NotAStruct))
};
if let Some(union_field) = src.which()? {
dst.set(union_field, src.get(union_field)?)?;
Expand Down Expand Up @@ -583,15 +577,15 @@ impl<'a> Builder<'a> {
p.clear();
Ok(crate::any_pointer::Builder::new(p).into())
}
_ => Err(crate::Error::failed(
"init() is only valid for struct and AnyPointer fields".into(),
_ => Err(Error::from_kind(
ErrorKind::InitIsOnlyValidForStructAndAnyPointerFields,
)),
}
}
field::Group(_) => {
self.clear(field)?;
let TypeVariant::Struct(schema) = ty.which() else {
return Err(crate::Error::failed("not a struct".into()))
return Err(Error::from_kind(ErrorKind::NotAStruct))
};
Ok((Builder::new(self.builder, schema.into())).into())
}
Expand Down Expand Up @@ -638,13 +632,13 @@ impl<'a> Builder<'a> {
.init_data(size)
.into()),

_ => Err(crate::Error::failed(
"initn() is only valid for list, text, or data fields".into(),
_ => Err(Error::from_kind(
ErrorKind::InitnIsOnlyValidForListTextOrDataFields,
)),
}
}
field::Group(_) => Err(crate::Error::failed(
"initn() is only valid for list, text, or data fields".into(),
field::Group(_) => Err(Error::from_kind(
ErrorKind::InitnIsOnlyValidForListTextOrDataFields,
)),
}
}
Expand Down Expand Up @@ -689,7 +683,7 @@ impl<'a> Builder<'a> {
}
field::Group(_) => {
let TypeVariant::Struct(schema) = ty.which() else {
return Err(crate::Error::failed("not a struct".into()))
return Err(Error::from_kind(ErrorKind::NotAStruct))
};
let mut group = Builder::new(self.builder.reborrow(), schema.into());

Expand All @@ -716,7 +710,7 @@ impl<'a> Builder<'a> {
fn set_in_union(&mut self, field: Field) -> Result<()> {
if has_discriminant_value(field.get_proto()) {
let node::Struct(st) = self.schema.get_proto().which()? else {
return Err(crate::Error::failed("not a struct".into()))
return Err(Error::from_kind(ErrorKind::NotAStruct))
};
self.builder.set_data_field::<u16>(
st.get_discriminant_offset() as usize,
Expand Down
2 changes: 1 addition & 1 deletion capnp/src/dynamic_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl<'a> Reader<'a> {
}
(value::Interface(()), TypeVariant::Capability) => Ok(Capability.into()),
(value::AnyPointer(a), TypeVariant::AnyPointer) => Ok(a.into()),
_ => Err(crate::Error::failed("type mismatch".into())),
_ => Err(crate::Error::from_kind(crate::ErrorKind::TypeMismatch)),
}
}

Expand Down
13 changes: 5 additions & 8 deletions capnp/src/io.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
//! Custom I/O traits that roughly mirror `std::io::{Read, BufRead, Write}`.
//! This extra layer of indirection enables support of no-std environments.
use alloc::string::ToString;

use crate::Result;
use crate::{Error, ErrorKind, Result};

/// A rough approximation of std::io::Read.
pub trait Read {
Expand All @@ -26,9 +24,7 @@ pub trait Read {
}
}
if !buf.is_empty() {
Err(crate::Error::failed(
"failed to fill the whole buffer".to_string(),
))
Err(Error::from_kind(ErrorKind::FailedToFillTheWholeBuffer))
} else {
Ok(())
}
Expand Down Expand Up @@ -92,13 +88,14 @@ mod std_impls {
#[cfg(not(feature = "std"))]
mod no_std_impls {
use crate::io::{BufRead, Read, Write};
use crate::{Error, Result};
use crate::{Error, ErrorKind, Result};
#[cfg(feature = "alloc")]
use alloc::string::ToString;

impl<'a> Write for &'a mut [u8] {
fn write_all(&mut self, buf: &[u8]) -> Result<()> {
if buf.len() > self.len() {
return Err(Error::failed("buffer is not large enough".to_string()));
return Err(Error::from_kind(ErrorKind::BufferNotLargeEnough));
}
let amt = buf.len();
let (a, b) = core::mem::take(self).split_at_mut(amt);
Expand Down
Loading

0 comments on commit 5414689

Please sign in to comment.