nicer error when hitting recursion error while serializing (#630)

Author: samuelcolvin

src/serializers/errors.rs

@@ -37,6 +37,12 @@ pub struct PydanticSerializationError {
     message: String,
 }
 
+impl fmt::Display for PydanticSerializationError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.message)
+    }
+}
+
 impl PydanticSerializationError {
     pub(crate) fn new_err(msg: String) -> PyErr {
         PyErr::new::<Self, String>(msg)

src/serializers/type_serializers/function.rs

@@ -1,15 +1,14 @@
 use std::borrow::Cow;
 use std::str::FromStr;
 
-use pyo3::exceptions::{PyAttributeError, PyRuntimeError};
+use pyo3::exceptions::{PyAttributeError, PyRecursionError, PyRuntimeError};
 use pyo3::intern;
 use pyo3::prelude::*;
 use pyo3::types::PyDict;
 
 use pyo3::types::PyString;
-use serde::ser::Error;
 
-use crate::build_tools::{function_name, py_error_type, SchemaDict};
+use crate::build_tools::{function_name, py_err, py_error_type, SchemaDict};
 use crate::definitions::DefinitionsBuilder;
 use crate::{PydanticOmit, PydanticSerializationUnexpectedValue};
 
@@ -154,6 +153,26 @@ impl FunctionPlainSerializer {
     }
 }
 
+fn on_error(py: Python, err: PyErr, function_name: &str, extra: &Extra) -> PyResult<()> {
+    let exception = err.value(py);
+    if let Ok(ser_err) = exception.extract::<PydanticSerializationUnexpectedValue>() {
+        if extra.check.enabled() {
+            Err(err)
+        } else {
+            extra.warnings.custom_warning(ser_err.__repr__());
+            Ok(())
+        }
+    } else if let Ok(err) = exception.extract::<PydanticSerializationError>() {
+        py_err!(PydanticSerializationError; "{}", err)
+    } else if exception.is_instance_of::<PyRecursionError>().unwrap_or(false) {
+        py_err!(PydanticSerializationError; "Error calling function `{}`: RecursionError", function_name)
+    } else {
+        let new_err = py_error_type!(PydanticSerializationError; "Error calling function `{}`: {}", function_name, err);
+        new_err.set_cause(py, Some(err));
+        Err(new_err)
+    }
+}
+
 macro_rules! function_type_serializer {
     ($name:ident) => {
         impl TypeSerializer for $name {
@@ -177,21 +196,10 @@ macro_rules! function_type_serializer {
                             _ => Ok(next_value.to_object(py)),
                         }
                     }
-                    Err(err) => match err.value(py).extract::<PydanticSerializationUnexpectedValue>() {
-                        Ok(ser_err) => {
-                            if extra.check.enabled() {
-                                Err(err)
-                            } else {
-                                extra.warnings.custom_warning(ser_err.__repr__());
-                                infer_to_python(value, include, exclude, extra)
-                            }
-                        }
-                        Err(_) => {
-                            let new_err = py_error_type!(PydanticSerializationError; "Error calling function `{}`: {}", self.function_name, err);
-                            new_err.set_cause(py, Some(err));
-                            Err(new_err)
-                        }
-                    },
+                    Err(err) => {
+                        on_error(py, err, &self.function_name, extra)?;
+                        infer_to_python(value, include, exclude, extra)
+                    }
                 }
             }
 
@@ -205,21 +213,10 @@ macro_rules! function_type_serializer {
                             None => infer_json_key(next_key, extra),
                         }
                     }
-                    Err(err) => match err.value(py).extract::<PydanticSerializationUnexpectedValue>() {
-                        Ok(ser_err) => {
-                            if extra.check.enabled() {
-                                Err(err)
-                            } else {
-                                extra.warnings.custom_warning(ser_err.__repr__());
-                                infer_json_key(key, extra)
-                            }
-                        }
-                        Err(_) => {
-                            let new_err = py_error_type!(PydanticSerializationError; "Error calling function `{}`: {}", self.function_name, err);
-                            new_err.set_cause(py, Some(err));
-                            Err(new_err)
-                        }
-                    },
+                    Err(err) => {
+                        on_error(py, err, &self.function_name, extra)?;
+                        infer_json_key(key, extra)
+                    }
                 }
             }
 
@@ -235,28 +232,18 @@ macro_rules! function_type_serializer {
                 match self.call(value, include, exclude, extra) {
                     Ok(v) => {
                         let next_value = v.as_ref(py);
-                            // None for include/exclude here, as filtering should be done
+                        // None for include/exclude here, as filtering should be done
                         match self.json_return_ob_type {
                             Some(ref ob_type) => {
                                 infer_serialize_known(ob_type, next_value, serializer, None, None, extra)
                             }
                             None => infer_serialize(next_value, serializer, None, None, extra),
                         }
                     }
-                    Err(err) => match err.value(py).extract::<PydanticSerializationUnexpectedValue>() {
-                        Ok(ser_err) => {
-                            if extra.check.enabled() {
-                                Err(py_err_se_err(err))
-                            } else {
-                                extra.warnings.custom_warning(ser_err.__repr__());
-                                infer_serialize(value, serializer, include, exclude, extra)
-                            }
-                        }
-                        Err(_) => Err(Error::custom(format!(
-                            "Error calling function `{}`: {}",
-                            self.function_name, err
-                        ))),
-                    },
+                    Err(err) => {
+                        on_error(py, err, &self.function_name, extra).map_err(py_err_se_err)?;
+                        infer_serialize(value, serializer, include, exclude, extra)
+                    }
                 }
             }
 

tests/serializers/test_functions.py

@@ -1,4 +1,5 @@
 import json
+import platform
 import sys
 from collections import deque
 from pathlib import Path
@@ -406,10 +407,10 @@ def fallback(v):
     assert s.to_json('foo') == b'"result=foo"'
 
     assert s.to_python(Foobar()) == 'result=foobar!'
-    with pytest.raises(PydanticSerializationError, match='Error calling function `f`'):
-        assert s.to_python(Foobar(), mode='json') == 'result=foobar!'
-    with pytest.raises(PydanticSerializationError, match='Error calling function `f`'):
-        assert s.to_json(Foobar()) == b'"result=foobar!"'
+    with pytest.raises(PydanticSerializationError, match='Unable to serialize unknown type:'):
+        s.to_python(Foobar(), mode='json')
+    with pytest.raises(PydanticSerializationError, match='Unable to serialize unknown type:'):
+        s.to_json(Foobar())
 
     assert s.to_python(Foobar(), fallback=fallback) == 'result=fallback:foobar!'
     assert s.to_python(Foobar(), mode='json', fallback=fallback) == 'result=fallback:foobar!'
@@ -630,3 +631,34 @@ def f(value, handler, _info):
     s = SchemaSerializer(core_schema.general_wrap_validator_function(f, core_schema.int_schema()))
     with pytest.warns(UserWarning, match='Expected `int` but got `str` - serialized value may not be as expected'):
         assert s.to_python('abc') == 'abc'
+
+
+@pytest.mark.skipif(
+    platform.python_implementation() == 'PyPy' or sys.platform in {'emscripten', 'win32'},
+    reason='fails on pypy, emscripten and windows',
+)
+def test_recursive_call():
+    def bad_recursive(value):
+        return s.to_python(value)
+
+    s = SchemaSerializer(
+        core_schema.any_schema(serialization=core_schema.plain_serializer_function_ser_schema(bad_recursive))
+    )
+    with pytest.raises(PydanticSerializationError) as exc_info:
+        s.to_python(42)
+    # insert_assert(str(exc_info.value))
+    assert str(exc_info.value) == 'Error calling function `bad_recursive`: RecursionError'
+
+    with pytest.raises(PydanticSerializationError) as exc_info:
+        s.to_python(42, mode='json')
+    # insert_assert(str(exc_info.value))
+    assert str(exc_info.value) == 'Error calling function `bad_recursive`: RecursionError'
+
+    with pytest.raises(PydanticSerializationError) as exc_info:
+        s.to_json(42)
+    # insert_assert(str(exc_info.value))
+    assert str(exc_info.value) == (
+        'Error serializing to JSON: '
+        'PydanticSerializationError: Error calling function `bad_recursive`: '
+        'RuntimeError: Already mutably borrowed'
+    )