From 71419d5fb2b1708d4e71c983d008b4f5071ad1aa Mon Sep 17 00:00:00 2001 From: VirxEC Date: Wed, 8 May 2024 18:13:34 -0400 Subject: [PATCH] Raise Python exceptions instead of aborting the process Add InvalidFlatbuffer exception, raised upon invalid data into unpack Raise ValueError upon out of bounds number being passed to __init__ for enums --- Cargo.lock | 48 ++++++++++++++++++++++++------------------------ Cargo.toml | 2 +- build.rs | 51 ++++++++++++++++++++++++++++++++++----------------- pytest.py | 43 ++++++++++++++++++++++++++++++++++++++----- src/lib.rs | 20 +++++++++++++++++--- 5 files changed, 114 insertions(+), 50 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 398dbb5..5139f43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11,7 +11,7 @@ dependencies = [ "attribute-derive-macro", "proc-macro2", "quote", - "syn 2.0.60", + "syn 2.0.61", ] [[package]] @@ -27,14 +27,14 @@ dependencies = [ "proc-macro2", "quote", "quote-use", - "syn 2.0.60", + "syn 2.0.61", ] [[package]] name = "autocfg" -version = "1.2.0" +version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1fdabc7756949593fe60f30ec81974b613357de856987752631dea1e3394c80" +checksum = "0c4b4d0bd25bd0b74681c0ad21497610ce1b7c91b1022cd21c80c6fbdd9476b0" [[package]] name = "bitflags" @@ -68,7 +68,7 @@ checksum = "62d671cc41a825ebabc75757b62d3d168c577f9149b2d49ece1dad1f72119d25" dependencies = [ "proc-macro2", "quote", - "syn 2.0.60", + "syn 2.0.61", ] [[package]] @@ -98,7 +98,7 @@ checksum = "13a1bcfb855c1f340d5913ab542e36f25a1c56f57de79022928297632435dec2" dependencies = [ "attribute-derive", "quote", - "syn 2.0.60", + "syn 2.0.61", ] [[package]] @@ -121,9 +121,9 @@ checksum = "71dd52191aae121e8611f1e8dc3e324dd0dd1dee1e6dd91d10ee07a3cfb4d9d8" [[package]] name = "libc" -version = "0.2.153" +version = "0.2.154" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c198f91728a82281a64e1f4f9eeb25d82cb32a5de251c6bd1b5154d63a8e7bd" +checksum = "ae743338b92ff9146ce83992f766a31066a91a8c84a45e0e9f21e7cf6de6d346" [[package]] name = "lock_api" @@ -216,9 +216,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.81" +version = "1.0.82" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3d1597b0c024618f09a9c3b8655b7e430397a36d23fdafec26d6965e9eec3eba" +checksum = "8ad3d49ab951a01fbaafe34f2ec74122942fe18a3f9814c3268f1bb72042131b" dependencies = [ "unicode-ident", ] @@ -270,7 +270,7 @@ dependencies = [ "proc-macro2", "pyo3-macros-backend", "quote", - "syn 2.0.60", + "syn 2.0.61", ] [[package]] @@ -283,7 +283,7 @@ dependencies = [ "proc-macro2", "pyo3-build-config", "quote", - "syn 2.0.60", + "syn 2.0.61", ] [[package]] @@ -303,7 +303,7 @@ checksum = "a7b5abe3fe82fdeeb93f44d66a7b444dedf2e4827defb0a8e69c437b2de2ef94" dependencies = [ "quote", "quote-use-macros", - "syn 2.0.60", + "syn 2.0.61", ] [[package]] @@ -315,7 +315,7 @@ dependencies = [ "derive-where", "proc-macro2", "quote", - "syn 2.0.60", + "syn 2.0.61", ] [[package]] @@ -329,7 +329,7 @@ dependencies = [ [[package]] name = "rlbot-flatbuffers-py" -version = "0.3.3" +version = "0.3.4" dependencies = [ "flatbuffers", "get-size", @@ -354,28 +354,28 @@ checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" [[package]] name = "semver" -version = "1.0.22" +version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "92d43fe69e652f3df9bdc2b85b2854a0825b86e4fb76bc44d945137d053639ca" +checksum = "61697e0a1c7e512e84a621326239844a24d8207b4669b41bc18b32ea5cbf988b" [[package]] name = "serde" -version = "1.0.199" +version = "1.0.201" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0c9f6e76df036c77cd94996771fb40db98187f096dd0b9af39c6c6e452ba966a" +checksum = "780f1cebed1629e4753a1a38a3c72d30b97ec044f0aef68cb26650a3c5cf363c" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.199" +version = "1.0.201" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "11bd257a6541e141e42ca6d24ae26f7714887b47e89aa739099104c7e4d3b7fc" +checksum = "c5e405930b9796f1c00bee880d03fc7e0bb4b9a11afc776885ffe84320da2865" dependencies = [ "proc-macro2", "quote", - "syn 2.0.60", + "syn 2.0.61", ] [[package]] @@ -396,9 +396,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.60" +version = "2.0.61" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "909518bc7b1c9b779f1bbf07f2929d35af9f0f37e47c6e9ef7f9dddc1e1821f3" +checksum = "c993ed8ccba56ae856363b1845da7266a7cb78e1d146c8a32d54b45a8b831fc9" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index f0271af..9590168 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rlbot-flatbuffers-py" -version = "0.3.3" +version = "0.3.4" edition = "2021" description = "A Python module implemented in Rust for serializing and deserializing RLBot's flatbuffers" repository = "https://github.com/VirxEC/rlbot-flatbuffers-py" diff --git a/build.rs b/build.rs index 529848c..4b6ce77 100644 --- a/build.rs +++ b/build.rs @@ -65,8 +65,9 @@ impl PythonBindGenerator { let mut file_contents = vec![]; file_contents.push(Cow::Borrowed(match bind_type { - PythonBindType::Struct | PythonBindType::Union => "use crate::{generated::rlbot::flat, FromGil, IntoGil};", - PythonBindType::Enum => "use crate::generated::rlbot::flat;", + PythonBindType::Struct => "use crate::{flat_err_to_py, generated::rlbot::flat, FromGil, IntoGil};", + PythonBindType::Union => "use crate::{generated::rlbot::flat, FromGil, IntoGil};", + PythonBindType::Enum => "use crate::{flat_err_to_py, generated::rlbot::flat};", })); if bind_type != PythonBindType::Union { @@ -82,8 +83,10 @@ impl PythonBindGenerator { } file_contents.push(Cow::Borrowed(match bind_type { - PythonBindType::Struct => "use pyo3::{pyclass, pymethods, types::PyBytes, Bound, Py, Python};", - PythonBindType::Enum => "use pyo3::{pyclass, pymethods, types::PyBytes, Bound, Python};", + PythonBindType::Struct => "use pyo3::{pyclass, pymethods, types::PyBytes, Bound, Py, PyResult, Python};", + PythonBindType::Enum => { + "use pyo3::{exceptions::PyValueError, pyclass, pymethods, types::PyBytes, Bound, PyResult, Python};" + } PythonBindType::Union => "use pyo3::{pyclass, pymethods, Py, PyObject, Python, ToPyObject};", })); @@ -809,19 +812,20 @@ impl PythonBindGenerator { assert!(u8::try_from(self.types.len()).is_ok()); self.write_str(" #[pyo3(signature = (value=Default::default()))]"); - self.write_str(" pub fn new(value: u8) -> Self {"); + self.write_str(" pub fn new(value: u8) -> PyResult {"); self.write_str(" match value {"); for variable_info in &self.types { let variable_name = &variable_info[0]; let variable_value = &variable_info[1]; - self.file_contents - .push(Cow::Owned(format!(" {variable_value} => Self::{variable_name},"))); + self.file_contents.push(Cow::Owned(format!( + " {variable_value} => Ok(Self::{variable_name})," + ))); } if self.types.len() != usize::from(u8::MAX) { - self.write_str(" v => panic!(\"Unknown value: {v}\"),"); + self.write_str(" v => Err(PyValueError::new_err(format!(\"Unknown value of {v}\"))),"); } self.write_str(" }"); @@ -1118,14 +1122,17 @@ impl PythonBindGenerator { self.write_str(" #[staticmethod]"); if self.bind_type == PythonBindType::Enum { - self.write_str(" fn unpack(data: &[u8]) -> Self {"); - self.write_string(format!(" root::(data).unwrap().into()", self.struct_name)); + self.write_str(" fn unpack(data: &[u8]) -> PyResult {"); + self.write_string(format!(" match root::(data) {{", self.struct_name)); + self.write_str(" Ok(flat_t) => Ok(flat_t.into()),"); + self.write_str(" Err(e) => Err(flat_err_to_py(e)),"); + self.write_str(" }"); } else { - self.write_str(" fn unpack(py: Python, data: &[u8]) -> Py {"); - self.write_string(format!( - " root::(data).unwrap().unpack().into_gil(py)", - self.struct_name - )); + self.write_str(" fn unpack(py: Python, data: &[u8]) -> PyResult> {"); + self.write_string(format!(" match root::(data) {{", self.struct_name)); + self.write_str(" Ok(flat_t) => Ok(flat_t.unpack().into_gil(py)),"); + self.write_str(" Err(e) => Err(flat_err_to_py(e)),"); + self.write_str(" }"); } self.write_str(" }"); @@ -1226,6 +1233,8 @@ fn pyi_generator(type_data: &[(String, String, Vec>)]) -> io::Result Cow::Borrowed("__doc__: str"), Cow::Borrowed("__version__: str"), Cow::Borrowed(""), + Cow::Borrowed("class InvalidFlatbuffer(ValueError): ..."), + Cow::Borrowed(""), ]; let primitive_map = [ @@ -1347,7 +1356,10 @@ fn pyi_generator(type_data: &[(String, String, Vec>)]) -> io::Result file_contents.push(Cow::Borrowed("")); if is_enum { - file_contents.push(Cow::Borrowed(" def __init__(self, value: int = 0): ...")); + file_contents.push(Cow::Borrowed(" def __init__(self, value: int = 0):")); + file_contents.push(Cow::Borrowed(" \"\"\"")); + file_contents.push(Cow::Borrowed(" :raises ValueError: If the `value` is not a valid enum value")); + file_contents.push(Cow::Borrowed(" \"\"\"")); } else { file_contents.push(Cow::Borrowed(" def __init__(")); file_contents.push(Cow::Borrowed(" self,")); @@ -1404,7 +1416,12 @@ fn pyi_generator(type_data: &[(String, String, Vec>)]) -> io::Result if !is_union { file_contents.push(Cow::Borrowed(" def pack(self) -> bytes: ...")); file_contents.push(Cow::Borrowed(" @staticmethod")); - file_contents.push(Cow::Owned(format!(" def unpack(data: bytes) -> {type_name}: ..."))); + file_contents.push(Cow::Owned(format!(" def unpack(data: bytes) -> {type_name}:"))); + file_contents.push(Cow::Borrowed(" \"\"\"")); + file_contents.push(Cow::Borrowed( + " :raises InvalidFlatbuffer: If the `data` is invalid for this type", + )); + file_contents.push(Cow::Borrowed(" \"\"\"")); } file_contents.push(Cow::Borrowed("")); diff --git a/pytest.py b/pytest.py index be93fc7..b0c0030 100644 --- a/pytest.py +++ b/pytest.py @@ -24,7 +24,7 @@ def __add__(self, other): ) dgs.game_info_state.world_gravity_z = Float(-650) dgs.game_info_state.end_match.val = True - dgs.console_commands = [ConsoleCommand("freeze")] + dgs.console_commands = [ConsoleCommand("dump_items")] dgs.ball_state = DesiredBallState() print(repr(dgs)) @@ -52,7 +52,22 @@ def __add__(self, other): print(comm.content.decode("utf-8")) print() - num_trials = 1_000_000 + try: + AirState(8) + except ValueError as e: + print(e) + print() + + invalid_data = comm.pack() + + try: + RenderMessage.unpack(invalid_data) + except InvalidFlatbuffer as e: + print(e) + + print("Running quick benchmark...") + + num_trials = 100_000 total_make_time = 0 total_pack_time = 0 @@ -60,12 +75,30 @@ def __add__(self, other): for _ in range(num_trials): start = time_ns() desired_game_state = DesiredGameState( - DesiredBallState(DesiredPhysics()), - car_states=[DesiredCarState(boost_amount=100)], + DesiredBallState( + DesiredPhysics( + Vector3Partial(0, 0, 0), + RotatorPartial(0, 0, 0), + Vector3Partial(0, 0, 0), + Vector3Partial(0, 0, 0), + ) + ), + car_states=[ + DesiredCarState( + DesiredPhysics( + Vector3Partial(0, 0, 0), + RotatorPartial(0, 0, 0), + Vector3Partial(0, 0, 0), + Vector3Partial(0, 0, 0), + ), + 100, + ) + for _ in range(8) + ], game_info_state=DesiredGameInfoState( game_speed=1, world_gravity_z=-650, end_match=True ), - console_commands=[ConsoleCommand("freeze")], + console_commands=[ConsoleCommand("dump_items")], ) total_make_time += time_ns() - start diff --git a/src/lib.rs b/src/lib.rs index 990cf26..4694cbb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,8 +12,18 @@ pub mod generated; #[allow(clippy::enum_variant_names)] mod python; -use pyo3::{prelude::*, types::PyBytes, PyClass}; +use pyo3::{create_exception, exceptions::PyValueError, prelude::*, types::PyBytes, PyClass}; use python::*; +use std::panic::Location; + +create_exception!(rlbot_flatbuffers, InvalidFlatbuffer, PyValueError, "Invalid FlatBuffer"); + +#[track_caller] +pub fn flat_err_to_py(err: flatbuffers::InvalidFlatbuffer) -> PyErr { + let caller = Location::caller(); + let err_msg = format!("Can't make flatbuffer @ \"rlbot_flatbuffers/{}\":\n {err}", caller.file()); + InvalidFlatbuffer::new_err(err_msg) +} pub trait FromGil { fn from_gil(py: Python, obj: T) -> Self; @@ -108,13 +118,14 @@ impl FromGil for Py { } macro_rules! pynamedmodule { - (doc: $doc:literal, name: $name:tt, classes: [$($class_name:ident),*], vars: [$(($var_name:literal, $value:expr)),*]) => { + (doc: $doc:literal, name: $name:tt, classes: [$($class_name:ident),*], vars: [$(($var_name:literal, $value:expr)),*], exceptions: [$($except:expr),*]) => { #[doc = $doc] #[pymodule] #[allow(redundant_semicolons)] - fn $name(m: Bound) -> PyResult<()> { + fn $name(py: Python, m: Bound) -> PyResult<()> { $(m.add_class::<$class_name>()?);*; $(m.add($var_name, $value)?);*; + $(m.add(stringify!($except), py.get_type_bound::<$except>())?);*; Ok(()) } }; @@ -213,5 +224,8 @@ pynamedmodule! { ], vars: [ ("__version__", env!("CARGO_PKG_VERSION")) + ], + exceptions: [ + InvalidFlatbuffer ] }