Pass detailed error message to python when parsing an argument fails

[ricohageman]

This pull requests includes some changes related to issue #2177

[davidhewitt]

Thank you for looking into this; sorry I never found a chance to replying further to our conversation on Gitter.

You're right that this is definitely a bug. I wonder though, instead of flattening all the causes into a single exception, is it nicer to just preserve the existing cause, if any? See comment below...

[davidhewitt]

I wonder, rather than flattening all the causes, should we be preserving the cause of the original error instead, like this:

Suggested change

	let mut reason = error
	.value(py)
	.str()
	.unwrap_or_else(|_| PyString::new(py, ""))
	.to_string();
	while let Some(cause) = error.cause(py) {
	reason.push_str(":\n\t");
	reason.push_str(&cause.to_string());
	error = cause
	}
	PyTypeError::new_err(format!("argument '{}': {}", arg_name, reason))
	let remapped_error = PyTypeError::new_err(format!("argument '{}': {}", arg_name, error.value(py)));
	remapped_error.set_cause(py, error.cause(py));
	remapped_error

Then the error seen from Python might be more like this:

TypeError: 'str' object cannot be interpreted as an integer

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/david/Dev/pyo3-scratch/test.py", line 19, in <module>
    pyo3_scratch.function_to_call_with_value_class(ValueClass("unparsable string"))
TypeError: argument 'value': failed to extract field ValueClass.value

I think preserving the cause tree like this is probably more correct. For example, the argument conversion code could run any old Python code (not just our PyO3 code) which generates some exception with its own separate cause. I think PyO3 flattening those causes into a single error message is a mistake; it might lose valuable debugging information.

[ricohageman]

You're totally right with this remark. Also, I didn't completely liked the flattening so this is a better approach while still preserving all the relevant information.

In addition, I tried to comply to rustfmt and clippy where relevant to this pull request. However, they were also marking parts that were untouched (some strict comparisons between floating points). Let's see if the workflows succeed.

[davidhewitt]

Looks great, many thanks!