Skip to content

Add FieldError with field name context to parsing errors#43

Open
Cvikli wants to merge 3 commits intoJuliaServices:mainfrom
Cvikli:improve-field-error-messages
Open

Add FieldError with field name context to parsing errors#43
Cvikli wants to merge 3 commits intoJuliaServices:mainfrom
Cvikli:improve-field-error-messages

Conversation

@Cvikli
Copy link
Copy Markdown

@Cvikli Cvikli commented Mar 10, 2026

Summary

When struct parsing fails, errors like MethodError or TypeError are thrown without any indication of which field caused the failure. This makes debugging JSON parsing issues unnecessarily difficult, especially with larger structs.

This PR adds a FieldError exception type that wraps the original error with field context.

Before

# Wrong type
MethodError: Cannot `convert` an object of type Int64 to an object of type String

# Missing required field
TypeError(:typeassert, "", Int64, nothing)

# null for non-nullable
MethodError: Cannot `convert` an object of type Nothing to an object of type String

No indication of which field failed or which struct was being parsed.

After

# Wrong type
FieldError: Bar.path::String: MethodError: Cannot `convert`...

# Missing required field
FieldError: Bar.count::Int64: ArgumentError: required field is missing from input

# null for non-nullable
FieldError: Bar.path::String: MethodError: Cannot `convert`...

# Nested struct failure
FieldError: Outer.inner::Inner: ArgumentError: applyeach not applicable...

Implementation

  • FieldError <: Exception — exported, contains struct_type, field_name, field_type, and error fields for programmatic access
  • _make_field helper — wraps the make() + setval!() calls in findfield() with a try-catch that adds field context
  • Pre-construction check in makestruct — detects missing required fields before _construct is called, giving a clear error instead of a cryptic TypeError
  • Already-wrapped FieldErrors from nested structs are rethrown as-is (no double-wrapping)

Test plan

  • All 165 existing tests pass
  • Tested with JSON.jl v1.0 against all failure modes: wrong type, missing field, null→non-nullable, empty object, nested struct failure, array of structs
  • Valid parsing still works (extra fields ignored, defaults applied)

Cvikli added 3 commits March 10, 2026 10:51
Previously, when struct parsing failed, errors like MethodError or
TypeError were thrown without any indication of which field caused the
failure. For example, parsing `{"path": 123}` into a struct with
`path::String` would produce:

    MethodError: Cannot `convert` an object of type Int64 to an object of type String

Now the same error produces:

    FieldError: failed to parse field `path::String` of `Bar`: MethodError: Cannot `convert`...

This applies to all failure modes:
- Type mismatches: `FieldError: failed to parse field `count::Int64` of `Bar`: MethodError...`
- Missing required fields: `FieldError: failed to parse field `count::Int64` of `Bar`: missing required field`
- null for non-nullable: `FieldError: failed to parse field `path::String` of `Bar`: MethodError...`
- Nested struct failures: `FieldError: failed to parse field `inner::Inner` of `Outer`: ...`

The FieldError exception is exported and contains `struct_type`, `field_name`,
`field_type`, and `error` fields for programmatic access.
@quinnj
Copy link
Copy Markdown
Member

quinnj commented Mar 17, 2026

Hmmmmm............this looks like the right direction. I'm mostly worried about performance implications here. This code can be super performance-sensitive because it powers, for example, the raw JSON.parse performance. And here we're adding a try-catch to every field parsed, possibly non-inlined new function call, and a for-loop to check every field for every struct. In the JSON.jl repo, we have some benchmarks I worked with to track the performance of things as we iterated and refactored things. Will you report here the implications of these changes across a few of those benchmark cases? Let's see what the impact is and then maybe decide if there are more performant ways to achieve the same helpful behavior.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants