Repository: mitsuhiko/minijinja (language: rust)

Issue:
Make Value::from_serializable not panic
Currently `Value::from_serializable` intentionally panics when an invalid object is encountered. It might be interesting to at least in some cases not panic. For instance invalid keys in maps are sometimes encountered but then it might be preferrable to have a partially broken object in the context rather than the entire operation failing.


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index faf34d23..05bfcb52 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,9 @@ All notable changes to MiniJinja are documented here.
 ## 0.31.0
 
 - Changed the closure behavior of macros to match the one of Jinja2. (#233)
+- `Value::from_serializable` will no longer panic on invalid values.  Instead
+  the error is deferred to runtime which makes working with objects possible
+  that are only partially valid for the runtime environment. (#234)
 
 ## 0.30.7
 
diff --git a/minijinja-py/src/typeconv.rs b/minijinja-py/src/typeconv.rs
index 94b95461..f56c8c81 100644
--- a/minijinja-py/src/typeconv.rs
+++ b/minijinja-py/src/typeconv.rs
@@ -247,7 +247,7 @@ fn to_python_value_impl(py: Python<'_>, value: Value) -> PyResult<Py<PyAny>> {
         Ok(rv.into())
     } else {
         match value.kind() {
-            ValueKind::Undefined | ValueKind::None => Ok(().into_py(py)),
+            ValueKind::Undefined | ValueKind::None | ValueKind::Invalid => Ok(().into_py(py)),
             ValueKind::Bool => Ok(value.is_true().into_py(py)),
             ValueKind::Number => {
                 if let Ok(rv) = TryInto::<i64>::try_into(value.clone()) {
diff --git a/minijinja/src/key/serialize.rs b/minijinja/src/key/serialize.rs
index 97de3b92..faa1b89d 100644
--- a/minijinja/src/key/serialize.rs
+++ b/minijinja/src/key/serialize.rs
@@ -1,9 +1,33 @@
 use std::convert::TryFrom;
+use std::fmt;
 
 use serde::ser::{Impossible, Serialize, Serializer};
 
 use crate::key::{Key, StaticKey};
-use crate::utils::SerializationFailed;
+
+/// Value is not a valid key.
+#[derive(Debug)]
+pub struct InvalidKey;
+
+impl std::error::Error for InvalidKey {}
+
+impl fmt::Display for InvalidKey {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "invalid key")
+    }
+}
+
+impl serde::ser::Error for InvalidKey {
+    #[track_caller]
+    #[cold]
+    fn custom<T>(msg: T) -> Self
+    where
+        T: fmt::Display,
+    {
+        let _ = msg;
+        InvalidKey
+    }
+}
 
 impl<'a> Serialize for Key<'a> {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
@@ -24,94 +48,94 @@ pub struct KeySerializer;
 
 impl Serializer for KeySerializer {
     type Ok = StaticKey;
-    type Error = SerializationFailed;
+    type Error = InvalidKey;
 
-    type SerializeSeq = Impossible<StaticKey, SerializationFailed>;
-    type SerializeTuple = Impossible<StaticKey, SerializationFailed>;
-    type SerializeTupleStruct = Impossible<StaticKey, SerializationFailed>;
-    type SerializeTupleVariant = Impossible<StaticKey, SerializationFailed>;
-    type SerializeMap = Impossible<StaticKey, SerializationFailed>;
-    type SerializeStruct = Impossible<StaticKey, SerializationFailed>;
-    type SerializeStructVariant = Impossible<StaticKey, SerializationFailed>;
+    type SerializeSeq = Impossible<StaticKey, InvalidKey>;
+    type SerializeTuple = Impossible<StaticKey, InvalidKey>;
+    type SerializeTupleStruct = Impossible<StaticKey, InvalidKey>;
+    type SerializeTupleVariant = Impossible<StaticKey, InvalidKey>;
+    type SerializeMap = Impossible<StaticKey, InvalidKey>;
+    type SerializeStruct = Impossible<StaticKey, InvalidKey>;
+    type SerializeStructVariant = Impossible<StaticKey, InvalidKey>;
 
-    fn serialize_bool(self, v: bool) -> Result<StaticKey, SerializationFailed> {
+    fn serialize_bool(self, v: bool) -> Result<StaticKey, InvalidKey> {
         Ok(Key::Bool(v))
     }
 
-    fn serialize_i8(self, v: i8) -> Result<StaticKey, SerializationFailed> {
+    fn serialize_i8(self, v: i8) -> Result<StaticKey, InvalidKey> {
         Ok(Key::I64(v as i64))
     }
 
-    fn serialize_i16(self, v: i16) -> Result<StaticKey, SerializationFailed> {
+    fn serialize_i16(self, v: i16) -> Result<StaticKey, InvalidKey> {
         Ok(Key::I64(v as i64))
     }
 
-    fn serialize_i32(self, v: i32) -> Result<StaticKey, SerializationFailed> {
+    fn serialize_i32(self, v: i32) -> Result<StaticKey, InvalidKey> {
         Ok(Key::I64(v as i64))
     }
 
-    fn serialize_i64(self, v: i64) -> Result<StaticKey, SerializationFailed> {
+    fn serialize_i64(self, v: i64) -> Result<StaticKey, InvalidKey> {
         Ok(Key::I64(v))
     }
 
     #[cold]
-    fn serialize_i128(self, _: i128) -> Result<StaticKey, SerializationFailed> {
-        panic!("unsupported key type i128")
+    fn serialize_i128(self, _: i128) -> Result<StaticKey, InvalidKey> {
+        Err(InvalidKey)
     }
 
-    fn serialize_u8(self, v: u8) -> Result<StaticKey, SerializationFailed> {
+    fn serialize_u8(self, v: u8) -> Result<StaticKey, InvalidKey> {
         Ok(Key::I64(v as i64))
     }
 
-    fn serialize_u16(self, v: u16) -> Result<StaticKey, SerializationFailed> {
+    fn serialize_u16(self, v: u16) -> Result<StaticKey, InvalidKey> {
         Ok(Key::I64(v as i64))
     }
 
-    fn serialize_u32(self, v: u32) -> Result<StaticKey, SerializationFailed> {
+    fn serialize_u32(self, v: u32) -> Result<StaticKey, InvalidKey> {
         Ok(Key::I64(v as i64))
     }
 
-    fn serialize_u64(self, v: u64) -> Result<StaticKey, SerializationFailed> {
+    fn serialize_u64(self, v: u64) -> Result<StaticKey, InvalidKey> {
         match Key::try_from(v) {
             Ok(rv) => Ok(rv),
-            Err(_) => panic!("out of bounds for i64"),
+            Err(_) => Err(InvalidKey),
         }
     }
 
     #[cold]
-    fn serialize_u128(self, _: u128) -> Result<StaticKey, SerializationFailed> {
-        panic!("unsupported key type u128")
+    fn serialize_u128(self, _: u128) -> Result<StaticKey, InvalidKey> {
+        Err(InvalidKey)
     }
 
     #[cold]
-    fn serialize_f32(self, _: f32) -> Result<StaticKey, SerializationFailed> {
-        panic!("unsupported key type f32")
+    fn serialize_f32(self, _: f32) -> Result<StaticKey, InvalidKey> {
+        Err(InvalidKey)
     }
 
     #[cold]
-    fn serialize_f64(self, _: f64) -> Result<StaticKey, SerializationFailed> {
-        panic!("unsupported key type f64")
+    fn serialize_f64(self, _: f64) -> Result<StaticKey, InvalidKey> {
+        Err(InvalidKey)
     }
 
-    fn serialize_char(self, v: char) -> Result<StaticKey, SerializationFailed> {
+    fn serialize_char(self, v: char) -> Result<StaticKey, InvalidKey> {
         Ok(Key::Char(v))
     }
 
-    fn serialize_str(self, value: &str) -> Result<StaticKey, SerializationFailed> {
+    fn serialize_str(self, value: &str) -> Result<StaticKey, InvalidKey> {
         Ok(Key::make_string_key(value))
     }
 
     #[cold]
-    fn serialize_bytes(self, _value: &[u8]) -> Result<StaticKey, SerializationFailed> {
-        panic!("unsupported key type bytes")
+    fn serialize_bytes(self, _value: &[u8]) -> Result<StaticKey, InvalidKey> {
+        Err(InvalidKey)
     }
 
     #[cold]
-    fn serialize_none(self) -> Result<StaticKey, SerializationFailed> {
-        panic!("unsupported key type unit")
+    fn serialize_none(self) -> Result<StaticKey, InvalidKey> {
+        Err(InvalidKey)
     }
 
-    fn serialize_some<T: ?Sized>(self, value: &T) -> Result<StaticKey, SerializationFailed>
+    fn serialize_some<T: ?Sized>(self, value: &T) -> Result<StaticKey, InvalidKey>
     where
         T: Serialize,
     {
@@ -119,13 +143,13 @@ impl Serializer for KeySerializer {
     }
 
     #[cold]
-    fn serialize_unit(self) -> Result<StaticKey, SerializationFailed> {
-        panic!("unsupported key type unit")
+    fn serialize_unit(self) -> Result<StaticKey, InvalidKey> {
+        Err(InvalidKey)
     }
 
     #[cold]
-    fn serialize_unit_struct(self, _name: &'static str) -> Result<StaticKey, SerializationFailed> {
-        panic!("unsupported key type unit")
+    fn serialize_unit_struct(self, _name: &'static str) -> Result<StaticKey, InvalidKey> {
+        Err(InvalidKey)
     }
 
     fn serialize_unit_variant(
@@ -133,7 +157,7 @@ impl Serializer for KeySerializer {
         _name: &'static str,
         _variant_index: u32,
         variant: &'static str,
-    ) -> Result<StaticKey, SerializationFailed> {
+    ) -> Result<StaticKey, InvalidKey> {
         Ok(Key::Str(variant))
     }
 
@@ -141,7 +165,7 @@ impl Serializer for KeySerializer {
         self,
         _name: &'static str,
         value: &T,
-    ) -> Result<StaticKey, SerializationFailed>
+    ) -> Result<StaticKey, InvalidKey>
     where
         T: Serialize,
     {
@@ -154,7 +178,7 @@ impl Serializer for KeySerializer {
         _variant_index: u32,
         _variant: &'static str,
         value: &T,
-    ) -> Result<StaticKey, SerializationFailed>
+    ) -> Result<StaticKey, InvalidKey>
     where
         T: Serialize,
     {
@@ -162,13 +186,13 @@ impl Serializer for KeySerializer {
     }
 
     #[cold]
-    fn serialize_seq(self, _len: Option<usize>) -> Result<Self::SerializeSeq, SerializationFailed> {
-        panic!("sequences as keys are not supported")
+    fn serialize_seq(self, _len: Option<usize>) -> Result<Self::SerializeSeq, InvalidKey> {
+        Err(InvalidKey)
     }
 
     #[cold]
-    fn serialize_tuple(self, _len: usize) -> Result<Self::SerializeTuple, SerializationFailed> {
-        panic!("tuples as keys are not supported")
+    fn serialize_tuple(self, _len: usize) -> Result<Self::SerializeTuple, InvalidKey> {
+        Err(InvalidKey)
     }
 
     #[cold]
@@ -176,8 +200,8 @@ impl Serializer for KeySerializer {
         self,
         _name: &'static str,
         _len: usize,
-    ) -> Result<Self::SerializeTupleStruct, SerializationFailed> {
-        panic!("tuple structs as keys are not supported")
+    ) -> Result<Self::SerializeTupleStruct, InvalidKey> {
+        Err(InvalidKey)
     }
 
     #[cold]
@@ -187,16 +211,13 @@ impl Serializer for KeySerializer {
         _variant_index: u32,
         _variant: &'static str,
         _len: usize,
-    ) -> Result<Self::SerializeTupleVariant, SerializationFailed> {
-        panic!("tuple variants as keys are not supported",)
+    ) -> Result<Self::SerializeTupleVariant, InvalidKey> {
+        Err(InvalidKey)
     }
 
     #[cold]
-    fn serialize_map(
-        self,
-        _jlen: Option<usize>,
-    ) -> Result<Self::SerializeMap, SerializationFailed> {
-        panic!("maps as keys are not supported")
+    fn serialize_map(self, _jlen: Option<usize>) -> Result<Self::SerializeMap, InvalidKey> {
+        Err(InvalidKey)
     }
 
     #[cold]
@@ -204,8 +225,8 @@ impl Serializer for KeySerializer {
         self,
         _name: &'static str,
         _len: usize,
-    ) -> Result<Self::SerializeStruct, SerializationFailed> {
-        panic!("structs as keys are not supported")
+    ) -> Result<Self::SerializeStruct, InvalidKey> {
+        Err(InvalidKey)
     }
 
     #[cold]
@@ -215,7 +236,7 @@ impl Serializer for KeySerializer {
         _variant_index: u32,
         _variant: &'static str,
         _len: usize,
-    ) -> Result<Self::SerializeStructVariant, SerializationFailed> {
-        panic!("structs as keys are not supported")
+    ) -> Result<Self::SerializeStructVariant, InvalidKey> {
+        Err(InvalidKey)
     }
 }
diff --git a/minijinja/src/utils.rs b/minijinja/src/utils.rs
index 661619ee..8eefa636 100644
--- a/minijinja/src/utils.rs
+++ b/minijinja/src/utils.rs
@@ -14,32 +14,6 @@ use similar_asserts::assert_eq;
 /// internal marker to seal up some trait methods
 pub struct SealedMarker;
 
-/// Internal dummy error type for the internal serialization system.
-///
-/// This error does not actually "error", it instead panics.  For more
-/// information see `Value::from_serializable`.
-#[derive(Debug)]
-pub enum SerializationFailed {}
-
-impl std::error::Error for SerializationFailed {}
-
-impl fmt::Display for SerializationFailed {
-    fn fmt(&self, _f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        unreachable!();
-    }
-}
-
-impl serde::ser::Error for SerializationFailed {
-    #[track_caller]
-    #[cold]
-    fn custom<T>(msg: T) -> Self
-    where
-        T: fmt::Display,
-    {
-        panic!("{}", msg)
-    }
-}
-
 pub fn memchr(haystack: &[u8], needle: u8) -> Option<usize> {
     haystack.iter().position(|&x| x == needle)
 }
diff --git a/minijinja/src/value/mod.rs b/minijinja/src/value/mod.rs
index 9ebdd0e1..d9a7b819 100644
--- a/minijinja/src/value/mod.rs
+++ b/minijinja/src/value/mod.rs
@@ -116,7 +116,7 @@ use crate::functions;
 use crate::key::{Key, StaticKey};
 use crate::utils::OnDrop;
 use crate::value::object::{SimpleSeqObject, SimpleStructObject};
-use crate::value::serialize::ValueSerializer;
+use crate::value::serialize::transform;
 use crate::vm::State;
 
 pub use crate::value::argtypes::{from_args, ArgType, FunctionArgs, FunctionResult, Rest};
@@ -216,6 +216,8 @@ pub enum ValueKind {
     Undefined,
     /// The value is the none singleton ([`()`])
     None,
+    /// The value is invalid
+    Invalid,
     /// The value is a [`bool`]
     Bool,
     /// The value is a number of a supported type.
@@ -237,6 +239,7 @@ impl fmt::Display for ValueKind {
         let ty = match *self {
             ValueKind::Undefined => "undefined",
             ValueKind::None => "none",
+            ValueKind::Invalid => "invalid",
             ValueKind::Bool => "bool",
             ValueKind::Number => "number",
             ValueKind::Char => "char",
@@ -288,6 +291,7 @@ pub(crate) enum ValueRepr {
     F64(f64),
     Char(char),
     None,
+    Invalid(Arc<String>),
     U128(Packed<u128>),
     I128(Packed<i128>),
     String(Arc<String>, StringType),
@@ -307,6 +311,7 @@ impl fmt::Debug for ValueRepr {
             ValueRepr::F64(val) => fmt::Debug::fmt(val, f),
             ValueRepr::Char(val) => fmt::Debug::fmt(val, f),
             ValueRepr::None => write!(f, "None"),
+            ValueRepr::Invalid(ref val) => write!(f, "<invalid value: {}>", val),
             ValueRepr::U128(val) => fmt::Debug::fmt(&{ val.0 }, f),
             ValueRepr::I128(val) => fmt::Debug::fmt(&{ val.0 }, f),
             ValueRepr::String(val, _) => fmt::Debug::fmt(val, f),
@@ -391,6 +396,7 @@ impl fmt::Display for Value {
             }
             ValueRepr::Char(val) => write!(f, "{val}"),
             ValueRepr::None => write!(f, "none"),
+            ValueRepr::Invalid(ref val) => write!(f, "<invalid value: {}>", val),
             ValueRepr::I128(val) => write!(f, "{}", { val.0 }),
             ValueRepr::String(val, _) => write!(f, "{val}"),
             ValueRepr::Bytes(val) => write!(f, "{}", String::from_utf8_lossy(val)),
@@ -467,22 +473,17 @@ impl Value {
     /// let val = Value::from_serializable(&vec![1, 2, 3]);
     /// ```
     ///
-    /// # Panics
-    ///
-    /// This method panics if types are passed which are not supported by the
-    /// underlying rendering system.  Today this is the case for value types
-    /// that have unrepresentable keys, when custom serialize implementation
-    /// fail (for instance when invalid types are attempted to be flattened).
+    /// This method does not fail but it might return a value that is not valid.  Such
+    /// values will when operated on fail in the template engine in most situations.
+    /// This for instance can happen if the underlying implementation of [`Serialize`]
+    /// fails.  There are also cases where invalid objects are silently hidden in the
+    /// engine today.  This is for instance the case for when keys are used in hash maps
+    /// that the engine cannot deal with.  Invalid values are considered an implementation
+    /// detail.  There is currently no API to validate a value.
     pub fn from_serializable<T: Serialize>(value: &T) -> Value {
         let _serialization_guard = mark_internal_serialization();
         let _optimization_guard = value_optimization();
-
-        // note on `SerializationFailed` behaves like `Infallible` in that it should
-        // never happen.  Instead the creation of this error itself will panic instead.
-        match Serialize::serialize(value, ValueSerializer) {
-            Ok(rv) => rv,
-            Err(infallible) => match infallible {},
-        }
+        transform(value)
     }
 
     /// Creates a value from a safe string.
@@ -593,6 +594,7 @@ impl Value {
             ValueRepr::U64(_) | ValueRepr::I64(_) | ValueRepr::F64(_) => ValueKind::Number,
             ValueRepr::Char(_) => ValueKind::Char,
             ValueRepr::None => ValueKind::None,
+            ValueRepr::Invalid(_) => ValueKind::Invalid,
             ValueRepr::I128(_) => ValueKind::Number,
             ValueRepr::String(..) => ValueKind::String,
             ValueRepr::Bytes(_) => ValueKind::Bytes,
@@ -625,7 +627,7 @@ impl Value {
             ValueRepr::Char(x) => x != '\x00',
             ValueRepr::String(ref x, _) => !x.is_empty(),
             ValueRepr::Bytes(ref x) => !x.is_empty(),
-            ValueRepr::None | ValueRepr::Undefined => false,
+            ValueRepr::None | ValueRepr::Undefined | ValueRepr::Invalid(_) => false,
             ValueRepr::Seq(ref x) => !x.is_empty(),
             ValueRepr::Map(ref x, _) => !x.is_empty(),
             ValueRepr::Dynamic(ref x) => match x.kind() {
@@ -901,6 +903,15 @@ impl Value {
         }
     }
 
+    /// Asserts that the value is valid.
+    pub(crate) fn assert_valid(self) -> Result<Value, Error> {
+        if let ValueRepr::Invalid(ref err) = self.0 {
+            Err(Error::new(ErrorKind::BadSerialization, err.to_string()))
+        } else {
+            Ok(self)
+        }
+    }
+
     /// Calls the value directly.
     pub(crate) fn call(&self, state: &State, args: &[Value]) -> Result<Value, Error> {
         if let ValueRepr::Dynamic(ref dy) = self.0 {
@@ -1050,7 +1061,9 @@ impl Serialize for Value {
             ValueRepr::I64(i) => serializer.serialize_i64(i),
             ValueRepr::F64(f) => serializer.serialize_f64(f),
             ValueRepr::Char(c) => serializer.serialize_char(c),
-            ValueRepr::None | ValueRepr::Undefined => serializer.serialize_unit(),
+            ValueRepr::None | ValueRepr::Undefined | ValueRepr::Invalid(_) => {
+                serializer.serialize_unit()
+            }
             ValueRepr::U128(u) => serializer.serialize_u128(u.0),
             ValueRepr::I128(i) => serializer.serialize_i128(i.0),
             ValueRepr::String(ref s, _) => serializer.serialize_str(s),
diff --git a/minijinja/src/value/serialize.rs b/minijinja/src/value/serialize.rs
index 938d0b7f..16a3dd18 100644
--- a/minijinja/src/value/serialize.rs
+++ b/minijinja/src/value/serialize.rs
@@ -1,18 +1,50 @@
 use std::collections::BTreeMap;
+use std::fmt;
 
 use serde::{ser, Serialize, Serializer};
 
 use crate::key::{Key, KeySerializer, StaticKey};
-use crate::utils::SerializationFailed;
 use crate::value::{
     value_map_with_capacity, Arc, MapType, Packed, StringType, Value, ValueMap, ValueRepr,
     VALUE_HANDLES, VALUE_HANDLE_MARKER,
 };
+
+#[derive(Debug)]
+pub struct InvalidValue(Arc<String>);
+
+impl std::error::Error for InvalidValue {}
+
+impl fmt::Display for InvalidValue {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.0)
+    }
+}
+
+impl serde::ser::Error for InvalidValue {
+    fn custom<T>(msg: T) -> Self
+    where
+        T: fmt::Display,
+    {
+        InvalidValue(Arc::new(msg.to_string()))
+    }
+}
+
+/// Transforms a serializable value to a value object.
+///
+/// This neither fails nor panics.  For objects that cannot be represented
+/// the value might be represented as a half broken error object.
+pub fn transform<T: Serialize>(value: T) -> Value {
+    match value.serialize(ValueSerializer) {
+        Ok(rv) => rv,
+        Err(invalid) => ValueRepr::Invalid(invalid.0).into(),
+    }
+}
+
 pub struct ValueSerializer;
 
 impl Serializer for ValueSerializer {
     type Ok = Value;
-    type Error = SerializationFailed;
+    type Error = InvalidValue;
 
     type SerializeSeq = SerializeSeq;
     type SerializeTuple = SerializeTuple;
@@ -22,86 +54,86 @@ impl Serializer for ValueSerializer {
     type SerializeStruct = SerializeStruct;
     type SerializeStructVariant = SerializeStructVariant;
 
-    fn serialize_bool(self, v: bool) -> Result<Value, SerializationFailed> {
+    fn serialize_bool(self, v: bool) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::Bool(v).into())
     }
 
-    fn serialize_i8(self, v: i8) -> Result<Value, SerializationFailed> {
+    fn serialize_i8(self, v: i8) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::I64(v as i64).into())
     }
 
-    fn serialize_i16(self, v: i16) -> Result<Value, SerializationFailed> {
+    fn serialize_i16(self, v: i16) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::I64(v as i64).into())
     }
 
-    fn serialize_i32(self, v: i32) -> Result<Value, SerializationFailed> {
+    fn serialize_i32(self, v: i32) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::I64(v as i64).into())
     }
 
-    fn serialize_i64(self, v: i64) -> Result<Value, SerializationFailed> {
+    fn serialize_i64(self, v: i64) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::I64(v).into())
     }
 
-    fn serialize_i128(self, v: i128) -> Result<Value, SerializationFailed> {
+    fn serialize_i128(self, v: i128) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::I128(Packed(v)).into())
     }
 
-    fn serialize_u8(self, v: u8) -> Result<Value, SerializationFailed> {
+    fn serialize_u8(self, v: u8) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::U64(v as u64).into())
     }
 
-    fn serialize_u16(self, v: u16) -> Result<Value, SerializationFailed> {
+    fn serialize_u16(self, v: u16) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::U64(v as u64).into())
     }
 
-    fn serialize_u32(self, v: u32) -> Result<Value, SerializationFailed> {
+    fn serialize_u32(self, v: u32) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::U64(v as u64).into())
     }
 
-    fn serialize_u64(self, v: u64) -> Result<Value, SerializationFailed> {
+    fn serialize_u64(self, v: u64) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::U64(v).into())
     }
 
-    fn serialize_u128(self, v: u128) -> Result<Value, SerializationFailed> {
+    fn serialize_u128(self, v: u128) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::U128(Packed(v)).into())
     }
 
-    fn serialize_f32(self, v: f32) -> Result<Value, SerializationFailed> {
+    fn serialize_f32(self, v: f32) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::F64(v as f64).into())
     }
 
-    fn serialize_f64(self, v: f64) -> Result<Value, SerializationFailed> {
+    fn serialize_f64(self, v: f64) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::F64(v).into())
     }
 
-    fn serialize_char(self, v: char) -> Result<Value, SerializationFailed> {
+    fn serialize_char(self, v: char) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::Char(v).into())
     }
 
-    fn serialize_str(self, value: &str) -> Result<Value, SerializationFailed> {
+    fn serialize_str(self, value: &str) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::String(Arc::new(value.to_owned()), StringType::Normal).into())
     }
 
-    fn serialize_bytes(self, value: &[u8]) -> Result<Value, SerializationFailed> {
+    fn serialize_bytes(self, value: &[u8]) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::Bytes(Arc::new(value.to_owned())).into())
     }
 
-    fn serialize_none(self) -> Result<Value, SerializationFailed> {
+    fn serialize_none(self) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::None.into())
     }
 
-    fn serialize_some<T: ?Sized>(self, value: &T) -> Result<Value, SerializationFailed>
+    fn serialize_some<T: ?Sized>(self, value: &T) -> Result<Value, InvalidValue>
     where
         T: Serialize,
     {
-        value.serialize(self)
+        Ok(transform(value))
     }
 
-    fn serialize_unit(self) -> Result<Value, SerializationFailed> {
+    fn serialize_unit(self) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::None.into())
     }
 
-    fn serialize_unit_struct(self, _name: &'static str) -> Result<Value, SerializationFailed> {
+    fn serialize_unit_struct(self, _name: &'static str) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::None.into())
     }
 
@@ -110,7 +142,7 @@ impl Serializer for ValueSerializer {
         name: &'static str,
         variant_index: u32,
         variant: &'static str,
-    ) -> Result<Value, SerializationFailed> {
+    ) -> Result<Value, InvalidValue> {
         if name == VALUE_HANDLE_MARKER && variant == VALUE_HANDLE_MARKER {
             Ok(VALUE_HANDLES.with(|handles| {
                 let mut handles = handles.borrow_mut();
@@ -127,11 +159,11 @@ impl Serializer for ValueSerializer {
         self,
         _name: &'static str,
         value: &T,
-    ) -> Result<Value, SerializationFailed>
+    ) -> Result<Value, InvalidValue>
     where
         T: Serialize,
     {
-        value.serialize(self)
+        Ok(transform(value))
     }
 
     fn serialize_newtype_variant<T: ?Sized>(
@@ -140,22 +172,22 @@ impl Serializer for ValueSerializer {
         _variant_index: u32,
         variant: &'static str,
         value: &T,
-    ) -> Result<Value, SerializationFailed>
+    ) -> Result<Value, InvalidValue>
     where
         T: Serialize,
     {
         let mut map = value_map_with_capacity(1);
-        map.insert(Key::Str(variant), ok!(value.serialize(self)));
+        map.insert(Key::Str(variant), transform(value));
         Ok(ValueRepr::Map(Arc::new(map), MapType::Normal).into())
     }
 
-    fn serialize_seq(self, len: Option<usize>) -> Result<Self::SerializeSeq, SerializationFailed> {
+    fn serialize_seq(self, len: Option<usize>) -> Result<Self::SerializeSeq, InvalidValue> {
         Ok(SerializeSeq {
             elements: Vec::with_capacity(len.unwrap_or(0)),
         })
     }
 
-    fn serialize_tuple(self, len: usize) -> Result<Self::SerializeTuple, SerializationFailed> {
+    fn serialize_tuple(self, len: usize) -> Result<Self::SerializeTuple, InvalidValue> {
         Ok(SerializeTuple {
             elements: Vec::with_capacity(len),
         })
@@ -165,7 +197,7 @@ impl Serializer for ValueSerializer {
         self,
         _name: &'static str,
         len: usize,
-    ) -> Result<Self::SerializeTupleStruct, SerializationFailed> {
+    ) -> Result<Self::SerializeTupleStruct, InvalidValue> {
         Ok(SerializeTupleStruct {
             fields: Vec::with_capacity(len),
         })
@@ -177,14 +209,14 @@ impl Serializer for ValueSerializer {
         _variant_index: u32,
         variant: &'static str,
         len: usize,
-    ) -> Result<Self::SerializeTupleVariant, SerializationFailed> {
+    ) -> Result<Self::SerializeTupleVariant, InvalidValue> {
         Ok(SerializeTupleVariant {
             name: variant,
             fields: Vec::with_capacity(len),
         })
     }
 
-    fn serialize_map(self, len: Option<usize>) -> Result<Self::SerializeMap, SerializationFailed> {
+    fn serialize_map(self, len: Option<usize>) -> Result<Self::SerializeMap, InvalidValue> {
         Ok(SerializeMap {
             entries: value_map_with_capacity(len.unwrap_or(0)),
             key: None,
@@ -195,7 +227,7 @@ impl Serializer for ValueSerializer {
         self,
         _name: &'static str,
         len: usize,
-    ) -> Result<Self::SerializeStruct, SerializationFailed> {
+    ) -> Result<Self::SerializeStruct, InvalidValue> {
         Ok(SerializeStruct {
             fields: value_map_with_capacity(len),
         })
@@ -207,7 +239,7 @@ impl Serializer for ValueSerializer {
         _variant_index: u32,
         variant: &'static str,
         len: usize,
-    ) -> Result<Self::SerializeStructVariant, SerializationFailed> {
+    ) -> Result<Self::SerializeStructVariant, InvalidValue> {
         Ok(SerializeStructVariant {
             variant,
             map: value_map_with_capacity(len),
@@ -221,18 +253,17 @@ pub struct SerializeSeq {
 
 impl ser::SerializeSeq for SerializeSeq {
     type Ok = Value;
-    type Error = SerializationFailed;
+    type Error = InvalidValue;
 
-    fn serialize_element<T: ?Sized>(&mut self, value: &T) -> Result<(), SerializationFailed>
+    fn serialize_element<T: ?Sized>(&mut self, value: &T) -> Result<(), InvalidValue>
     where
         T: Serialize,
     {
-        let value = ok!(value.serialize(ValueSerializer));
-        self.elements.push(value);
+        self.elements.push(transform(value));
         Ok(())
     }
 
-    fn end(self) -> Result<Value, SerializationFailed> {
+    fn end(self) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::Seq(Arc::new(self.elements)).into())
     }
 }
@@ -243,18 +274,17 @@ pub struct SerializeTuple {
 
 impl ser::SerializeTuple for SerializeTuple {
     type Ok = Value;
-    type Error = SerializationFailed;
+    type Error = InvalidValue;
 
-    fn serialize_element<T: ?Sized>(&mut self, value: &T) -> Result<(), SerializationFailed>
+    fn serialize_element<T: ?Sized>(&mut self, value: &T) -> Result<(), InvalidValue>
     where
         T: Serialize,
     {
-        let value = ok!(value.serialize(ValueSerializer));
-        self.elements.push(value);
+        self.elements.push(transform(value));
         Ok(())
     }
 
-    fn end(self) -> Result<Value, SerializationFailed> {
+    fn end(self) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::Seq(Arc::new(self.elements)).into())
     }
 }
@@ -265,18 +295,17 @@ pub struct SerializeTupleStruct {
 
 impl ser::SerializeTupleStruct for SerializeTupleStruct {
     type Ok = Value;
-    type Error = SerializationFailed;
+    type Error = InvalidValue;
 
-    fn serialize_field<T: ?Sized>(&mut self, value: &T) -> Result<(), SerializationFailed>
+    fn serialize_field<T: ?Sized>(&mut self, value: &T) -> Result<(), InvalidValue>
     where
         T: Serialize,
     {
-        let value = ok!(value.serialize(ValueSerializer));
-        self.fields.push(value);
+        self.fields.push(transform(value));
         Ok(())
     }
 
-    fn end(self) -> Result<Value, SerializationFailed> {
+    fn end(self) -> Result<Value, InvalidValue> {
         Ok(Value(ValueRepr::Seq(Arc::new(self.fields))))
     }
 }
@@ -288,18 +317,17 @@ pub struct SerializeTupleVariant {
 
 impl ser::SerializeTupleVariant for SerializeTupleVariant {
     type Ok = Value;
-    type Error = SerializationFailed;
+    type Error = InvalidValue;
 
-    fn serialize_field<T: ?Sized>(&mut self, value: &T) -> Result<(), SerializationFailed>
+    fn serialize_field<T: ?Sized>(&mut self, value: &T) -> Result<(), InvalidValue>
     where
         T: Serialize,
     {
-        let value = ok!(value.serialize(ValueSerializer));
-        self.fields.push(value);
+        self.fields.push(transform(value));
         Ok(())
     }
 
-    fn end(self) -> Result<Value, SerializationFailed> {
+    fn end(self) -> Result<Value, InvalidValue> {
         let mut map = value_map_with_capacity(1);
         map.insert(
             Key::Str(self.name),
@@ -316,31 +344,30 @@ pub struct SerializeMap {
 
 impl ser::SerializeMap for SerializeMap {
     type Ok = Value;
-    type Error = SerializationFailed;
+    type Error = InvalidValue;
 
-    fn serialize_key<T: ?Sized>(&mut self, key: &T) -> Result<(), SerializationFailed>
+    fn serialize_key<T: ?Sized>(&mut self, key: &T) -> Result<(), InvalidValue>
     where
         T: Serialize,
     {
-        let key = ok!(key.serialize(KeySerializer));
-        self.key = Some(key);
+        match key.serialize(KeySerializer) {
+            Ok(key) => self.key = Some(key),
+            Err(_) => self.key = None,
+        }
         Ok(())
     }
 
-    fn serialize_value<T: ?Sized>(&mut self, value: &T) -> Result<(), SerializationFailed>
+    fn serialize_value<T: ?Sized>(&mut self, value: &T) -> Result<(), InvalidValue>
     where
         T: Serialize,
     {
-        let key = self
-            .key
-            .take()
-            .expect("serialize_value called before serialize_key");
-        let value = ok!(value.serialize(ValueSerializer));
-        self.entries.insert(key, value);
+        if let Some(key) = self.key.take() {
+            self.entries.insert(key, transform(value));
+        }
         Ok(())
     }
 
-    fn end(self) -> Result<Value, SerializationFailed> {
+    fn end(self) -> Result<Value, InvalidValue> {
         Ok(Value(ValueRepr::Map(
             Arc::new(self.entries),
             MapType::Normal,
@@ -351,14 +378,14 @@ impl ser::SerializeMap for SerializeMap {
         &mut self,
         key: &K,
         value: &V,
-    ) -> Result<(), SerializationFailed>
+    ) -> Result<(), InvalidValue>
     where
         K: Serialize,
         V: Serialize,
     {
-        let key = ok!(key.serialize(KeySerializer));
-        let value = ok!(value.serialize(ValueSerializer));
-        self.entries.insert(key, value);
+        if let Ok(key) = key.serialize(KeySerializer) {
+            self.entries.insert(key, transform(value));
+        }
         Ok(())
     }
 }
@@ -369,22 +396,21 @@ pub struct SerializeStruct {
 
 impl ser::SerializeStruct for SerializeStruct {
     type Ok = Value;
-    type Error = SerializationFailed;
+    type Error = InvalidValue;
 
     fn serialize_field<T: ?Sized>(
         &mut self,
         key: &'static str,
         value: &T,
-    ) -> Result<(), SerializationFailed>
+    ) -> Result<(), InvalidValue>
     where
         T: Serialize,
     {
-        let value = ok!(value.serialize(ValueSerializer));
-        self.fields.insert(Key::Str(key), value);
+        self.fields.insert(Key::Str(key), transform(value));
         Ok(())
     }
 
-    fn end(self) -> Result<Value, SerializationFailed> {
+    fn end(self) -> Result<Value, InvalidValue> {
         Ok(ValueRepr::Map(Arc::new(self.fields), MapType::Normal).into())
     }
 }
@@ -396,22 +422,21 @@ pub struct SerializeStructVariant {
 
 impl ser::SerializeStructVariant for SerializeStructVariant {
     type Ok = Value;
-    type Error = SerializationFailed;
+    type Error = InvalidValue;
 
     fn serialize_field<T: ?Sized>(
         &mut self,
         key: &'static str,
         value: &T,
-    ) -> Result<(), SerializationFailed>
+    ) -> Result<(), InvalidValue>
     where
         T: Serialize,
     {
-        let value = ok!(value.serialize(ValueSerializer));
-        self.map.insert(Key::Str(key), value);
+        self.map.insert(Key::Str(key), transform(value));
         Ok(())
     }
 
-    fn end(self) -> Result<Value, SerializationFailed> {
+    fn end(self) -> Result<Value, InvalidValue> {
         let mut rv = BTreeMap::new();
         rv.insert(
             self.variant,
diff --git a/minijinja/src/vm/mod.rs b/minijinja/src/vm/mod.rs
index 9104d49e..fc219c3c 100644
--- a/minijinja/src/vm/mod.rs
+++ b/minijinja/src/vm/mod.rs
@@ -90,7 +90,7 @@ impl<'env> Vm<'env> {
         self.eval_state(
             &mut State {
                 env: self.env,
-                ctx: Context::new(Frame::new(root)),
+                ctx: Context::new(Frame::new(ok!(root.assert_valid()))),
                 current_block: None,
                 current_call: None,
                 auto_escape,
@@ -253,7 +253,10 @@ impl<'env> Vm<'env> {
                     state.ctx.store(name, stack.pop());
                 }
                 Instruction::Lookup(name) => {
-                    stack.push(state.lookup(name).unwrap_or(Value::UNDEFINED));
+                    stack.push(ok!(state
+                        .lookup(name)
+                        .unwrap_or(Value::UNDEFINED)
+                        .assert_valid()));
                 }
                 Instruction::GetAttr(name) => {
                     a = stack.pop();
@@ -263,7 +266,7 @@ impl<'env> Vm<'env> {
                     // Only when we cannot look up something, we start to consider the undefined
                     // special case.
                     stack.push(match a.get_attr_fast(name) {
-                        Some(value) => value,
+                        Some(value) => ctx_ok!(value.assert_valid()),
                         None => ctx_ok!(undefined_behavior.handle_undefined(a.is_undefined())),
                     });
                 }
@@ -271,7 +274,7 @@ impl<'env> Vm<'env> {
                     a = stack.pop();
                     b = stack.pop();
                     stack.push(match b.get_item_opt(&a) {
-                        Some(value) => value,
+                        Some(value) => ctx_ok!(value.assert_valid()),
                         None => ctx_ok!(undefined_behavior.handle_undefined(b.is_undefined())),
                     });
                 }
@@ -404,7 +407,7 @@ impl<'env> Vm<'env> {
                         }
                     };
                     match next {
-                        Some(item) => stack.push(item),
+                        Some(item) => stack.push(ctx_ok!(item.assert_valid())),
                         None => {
                             pc = *jump_target;
                             continue;


Test patch:
diff --git a/minijinja/tests/test_templates.rs b/minijinja/tests/test_templates.rs
index 5cd51c41..16142bbb 100644
--- a/minijinja/tests/test_templates.rs
+++ b/minijinja/tests/test_templates.rs
@@ -339,25 +339,88 @@ fn test_current_call_state() {
     );
 }
 
+// ideally this would work, but unfortuantely the way serde flatten works makes it
+// impossible for us to support with the internal optimizations in the value model.
+// see https://github.com/mitsuhiko/minijinja/issues/222
+#[derive(Debug, serde::Serialize)]
+struct Bad {
+    a: i32,
+    #[serde(flatten)]
+    more: Value,
+}
+
 #[test]
 #[should_panic = "can only flatten structs and maps"]
 fn test_flattening() {
-    // ideally this would work, but unfortuantely the way serde flatten works makes it
-    // impossible for us to support with the internal optimizations in the value model.
-    // see https://github.com/mitsuhiko/minijinja/issues/222
-    #[derive(Debug, serde::Serialize)]
-    struct Context {
-        a: i32,
-        #[serde(flatten)]
-        more: Value,
-    }
+    let ctx = Bad {
+        a: 42,
+        more: Value::from(BTreeMap::from([("b", 23)])),
+    };
+
+    let env = Environment::new();
+    env.render_str("{{ debug() }}", ctx).unwrap();
+}
+
+#[test]
+fn test_flattening_sub_item_good() {
+    let bad = Bad {
+        a: 42,
+        more: Value::from(BTreeMap::from([("b", 23)])),
+    };
 
-    let ctx = Context {
+    let ctx = context!(bad, good => "good");
+    let env = Environment::new();
+
+    // we are not touching a bad value, so we are good
+    let rv = env.render_str("{{ good }}", ctx).unwrap();
+    assert_eq!(rv, "good");
+}
+
+#[test]
+#[should_panic = "can only flatten structs and maps"]
+fn test_flattening_sub_item_bad_lookup() {
+    let bad = Bad {
         a: 42,
         more: Value::from(BTreeMap::from([("b", 23)])),
     };
 
+    let ctx = context!(bad, good => "good");
     let env = Environment::new();
-    let rv = env.render_str("{{ debug() }}", ctx).unwrap();
-    assert_eq!(rv, "42|23");
+
+    // resolving an invalid value will fail
+    env.render_str("{{ bad }}", ctx).unwrap();
+}
+
+#[test]
+#[should_panic = "can only flatten structs and maps"]
+fn test_flattening_sub_item_bad_attr() {
+    let bad = Bad {
+        a: 42,
+        more: Value::from(BTreeMap::from([("b", 23)])),
+    };
+
+    let ctx = context!(good => context!(bad));
+    let env = Environment::new();
+
+    // resolving an invalid value will fail, even in an attribute lookup
+    env.render_str("{% if good.bad %}...{% endif %}", ctx)
+        .unwrap();
+}
+
+#[test]
+fn test_flattening_sub_item_shielded_print() {
+    let bad = Bad {
+        a: 42,
+        more: Value::from(BTreeMap::from([("b", 23)])),
+    };
+
+    let ctx = context!(good => context!(bad));
+    let env = Environment::new();
+
+    // this on the other hand is okay
+    let value = env.render_str("{{ good }}", ctx).unwrap();
+    assert_eq!(
+        value,
+        r#"{"bad": <invalid value: can only flatten structs and maps (got an enum)>}"#
+    );
 }