From 19c3e4f6dc31fd78e2793d0596d6a9cc3a30580a Mon Sep 17 00:00:00 2001 From: Yiyu Lin Date: Thu, 13 Apr 2023 08:03:56 +0800 Subject: [PATCH] refactor(serde_v8): move to `thiserror`, better error output (#18202) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ref #17318 --------- Co-authored-by: Bartek Iwańczuk --- Cargo.lock | 1 + core/runtime.rs | 2 +- serde_v8/Cargo.toml | 1 + serde_v8/de.rs | 10 ++++++---- serde_v8/error.rs | 41 ++++++++++++++++++++++------------------- serde_v8/tests/de.rs | 4 ++-- 6 files changed, 33 insertions(+), 26 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 42b71b2015..132ae4c572 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3978,6 +3978,7 @@ dependencies = [ "serde_bytes", "serde_json", "smallvec", + "thiserror", "v8", ] diff --git a/core/runtime.rs b/core/runtime.rs index 84a72c02ba..ef65d2192b 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -4373,7 +4373,7 @@ Deno.core.opAsync("op_async_serialize_object_with_numbers_as_keys", { let sum = Deno.core.ops.op_sum_take(w32.subarray(0, 2)); return false; } catch(e) { - return e.message.includes('ExpectedDetachable'); + return e.message.includes('invalid type, expected: detachable'); } }); if (!assertWasmThrow()) { diff --git a/serde_v8/Cargo.toml b/serde_v8/Cargo.toml index b76bdf609b..bd10f18cf7 100644 --- a/serde_v8/Cargo.toml +++ b/serde_v8/Cargo.toml @@ -20,6 +20,7 @@ num-bigint.workspace = true serde.workspace = true serde_bytes.workspace = true smallvec = { workspace = true, features = ["union"] } +thiserror.workspace = true v8.workspace = true [dev-dependencies] diff --git a/serde_v8/de.rs b/serde_v8/de.rs index 5293a705d9..d593ffbc56 100644 --- a/serde_v8/de.rs +++ b/serde_v8/de.rs @@ -281,8 +281,9 @@ impl<'de, 'a, 'b, 's, 'x> de::Deserializer<'de> if obj.is_array() { // If the obj is an array fail if it's length differs from the tuple length let array = v8::Local::::try_from(self.input).unwrap(); - if array.length() as usize != len { - return Err(Error::LengthMismatch); + let array_len = array.length() as usize; + if array_len != len { + return Err(Error::LengthMismatch(array_len, len)); } } visitor.visit_seq(SeqAccess::new(obj, self.scope, 0..len as u32)) @@ -409,8 +410,9 @@ impl<'de, 'a, 'b, 's, 'x> de::Deserializer<'de> let prop_names = obj.get_own_property_names(self.scope, Default::default()); let prop_names = prop_names.ok_or(Error::ExpectedEnum)?; - if prop_names.length() != 1 { - return Err(Error::LengthMismatch); + let prop_names_len = prop_names.length(); + if prop_names_len != 1 { + return Err(Error::LengthMismatch(prop_names_len as usize, 1)); } prop_names.get_index(self.scope, 0).unwrap() }; diff --git a/serde_v8/error.rs b/serde_v8/error.rs index e613859463..94ac3c0a54 100644 --- a/serde_v8/error.rs +++ b/serde_v8/error.rs @@ -1,56 +1,59 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. use std::fmt::Display; -use std::fmt::{self}; - -use serde::de; -use serde::ser; pub type Result = std::result::Result; -#[derive(Clone, Debug, Eq, PartialEq)] +#[derive(Clone, Debug, Eq, PartialEq, thiserror::Error)] #[non_exhaustive] pub enum Error { + #[error("{0}")] Message(String), + #[error("serde_v8 error: invalid type, expected: boolean")] ExpectedBoolean, + #[error("serde_v8 error: invalid type, expected: integer")] ExpectedInteger, + #[error("serde_v8 error: invalid type, expected: number")] ExpectedNumber, + #[error("serde_v8 error: invalid type, expected: string")] ExpectedString, + #[error("serde_v8 error: invalid type, expected: array")] ExpectedArray, + #[error("serde_v8 error: invalid type, expected: map")] ExpectedMap, + #[error("serde_v8 error: invalid type, expected: enum")] ExpectedEnum, + #[error("serde_v8 error: invalid type, expected: object")] ExpectedObject, + #[error("serde_v8 error: invalid type, expected: buffer")] ExpectedBuffer, + #[error("serde_v8 error: invalid type, expected: detachable")] ExpectedDetachable, + #[error("serde_v8 error: invalid type, expected: external")] ExpectedExternal, + #[error("serde_v8 error: invalid type, expected: bigint")] ExpectedBigInt, + #[error("serde_v8 error: invalid type, expected: utf8")] ExpectedUtf8, + #[error("serde_v8 error: invalid type, expected: latin1")] ExpectedLatin1, + #[error("serde_v8 error: unsupported type")] UnsupportedType, - LengthMismatch, + + #[error("serde_v8 error: length mismatch, got: {0}, expected: {1}")] + LengthMismatch(usize, usize), } -impl ser::Error for Error { +impl serde::ser::Error for Error { fn custom(msg: T) -> Self { Error::Message(msg.to_string()) } } -impl de::Error for Error { +impl serde::de::Error for Error { fn custom(msg: T) -> Self { Error::Message(msg.to_string()) } } - -impl Display for Error { - fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { - match self { - Error::Message(msg) => formatter.write_str(msg), - err => formatter.write_str(format!("serde_v8 error: {err:?}").as_ref()), - } - } -} - -impl std::error::Error for Error {} diff --git a/serde_v8/tests/de.rs b/serde_v8/tests/de.rs index a77089babb..eae30f5404 100644 --- a/serde_v8/tests/de.rs +++ b/serde_v8/tests/de.rs @@ -115,13 +115,13 @@ defail!( de_tuple_wrong_len_short, (u64, bool, ()), "[123, true]", - |e| e == Err(Error::LengthMismatch) + |e| e == Err(Error::LengthMismatch(2, 3)) ); defail!( de_tuple_wrong_len_long, (u64, bool, ()), "[123, true, null, 'extra']", - |e| e == Err(Error::LengthMismatch) + |e| e == Err(Error::LengthMismatch(4, 3)) ); detest!( de_mathop,