Skip to content

A Parse/FromString exception is rethrown with its stack trace wiped, so the trace points at the converter instead of the code that failed #103

Description

@matt-edmondson

What's wrong

RoundTripStringJsonConverter<T>.Read and ReadAsPropertyName unwrap the reflection wrapper like this (RoundTripStringJsonConverter/RoundTripStringJsonConverter.cs, both methods):

catch (TargetInvocationException ex) when (ex.InnerException is not null)
{
	// Unwrap the inner exception to preserve the original exception type
	throw ex.InnerException;
}

throw ex.InnerException; keeps the exception type, but it resets the exception's stack trace to the rethrow site. Every frame inside the user's Parse / FromString / Create / Convert method is lost.

Reproduction

var o = new JsonSerializerOptions();
o.Converters.Add(new RoundTripStringJsonConverterFactory());
JsonSerializer.Deserialize<Widget>("\"bad\"", o);

public class Widget
{
	public static Widget Parse(string s) => Validate(s);
	static Widget Validate(string s) => throw new FormatException("bad widget");
	public override string ToString() => "w";
}

Stack trace actually reported (ktsu.RoundTripStringJsonConverter from NuGet, net10.0):

FormatException
   at ktsu.RoundTripStringJsonConverter.RoundTripStringJsonConverterFactory.RoundTripStringJsonConverter`1.Read(...)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(...)
   ...
   at Program.<Main>$(String[] args) in Program.cs:line 4

Widget.Validate and Widget.Parse, where the exception was actually thrown, are missing from the trace.

Why it matters

The converter exists to call user parsing code. When that code rejects a value, the trace is the main thing a developer has to work with. For a non-trivial Parse (several helpers, validation layers, or a semantic-string type that runs validators), the trace now ends at the converter. The developer can't tell which check failed or where.

Suggested fix

Rethrow through ExceptionDispatchInfo, which keeps both the type and the original trace:

catch (TargetInvocationException ex) when (ex.InnerException is not null)
{
	ExceptionDispatchInfo.Capture(ex.InnerException).Throw();
	throw; // unreachable
}

Alternatively, invoke with BindingFlags.DoNotWrapExceptions (.NET Core 3.0+, applied through the Invoke(object, BindingFlags, Binder, object[], CultureInfo) overload) and drop the catch. Both Read and ReadAsPropertyName need the change. The duplicated invoke block could move into one helper.

Acceptance criteria

  • A test whose Parse throws from a nested helper asserts that the caught exception's StackTrace contains the helper's method name, for both value and dictionary-key (ReadAsPropertyName) deserialization.
  • The exception type is still the original one (existing tests that expect e.g. FormatException keep passing).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingreadyFully specified; implement as written

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions