Enhance JSON type mismatch error handling

[moutonjeremy]

Improve error handling for JSON type mismatches by adding detailed field identification and expanding test coverage for new cases. This ensures clearer validation errors when input types do not match expected struct fields.

[Copilot]

Pull request overview

This PR enhances JSON type mismatch error handling by introducing detailed field identification in error messages and expanding test coverage. When clients send incorrect JSON types (e.g., sending a number for a string field), the system now provides clear error messages like "invalid type for field 'name': expected string but got number" instead of raw Go unmarshaling errors.

Key Changes:

• Added detectTypeMismatchFromBody() function to identify which field caused type mismatches by re-parsing the JSON body
• Added getJSONValueType() helper to determine the JSON type of values
• Expanded test coverage with 100+ new test cases covering all primitive types (string, int, float, bool), arrays, and maps

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 11 comments.

File	Description
common.go	Implements enhanced error detection logic with two new helper functions for identifying and reporting JSON type mismatches with user-friendly field names and type information
json_type_error_test.go	Adds comprehensive test coverage for type mismatch scenarios across string, int, float, bool, array, and map fields, including a new TestAllTypeMismatches test function

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The function returns the Go struct field name (e.g., "Name", "Age") instead of the JSON field name (e.g., "name", "age") that clients actually use in their requests. This makes error messages confusing since the API consumer sends JSON with lowercase field names but receives errors referencing capitalized struct field names.

The function should return jsonTag instead of field.Name to match what the client actually sent:

if mismatch {
    return jsonTag, expectedTypeName, actualValueType
}

Suggested change

	return field.Name, expectedTypeName, actualValueType
	return jsonTag, expectedTypeName, actualValueType

[Copilot]

[nitpick] The expected type format for slices (e.g., "[]string") differs from the format for maps (just "map"). The slice type includes the element type using reflection Kind() which produces types like "[]string", "[]int", but this is inconsistent with the map handling.

Additionally, using Kind() directly produces Go-style type names (like "string", "int") but for consistency with JSON/API terminology, consider using the same type mapping logic as in getJSONValueType to produce types like "[]string" for string arrays.

Suggested change

	expectedTypeName = fmt.Sprintf("[]%s", dereferenceType(expectedFieldType.Elem()).Kind())
	elemType := dereferenceType(expectedFieldType.Elem())
	expectedTypeName = fmt.Sprintf("[]%s", getJSONValueType(reflect.Zero(elemType).Interface()))

[Copilot]

The function checks if actualValue exists in bodyMap, but when a field is explicitly set to null in JSON, it will exist in the map with a nil value. The current logic would attempt to check type mismatches for null values, which are typically valid for any field and shouldn't be reported as type mismatches.

Consider skipping null values:

if actualValue, exists := bodyMap[jsonTag]; exists {
    // Skip null values as they're generally valid for optional fields
    if actualValue == nil {
        continue
    }
    expectedFieldType := dereferenceType(field.Type)
    // ... rest of the logic
}

Suggested change

	if actualValue, exists := bodyMap[jsonTag]; exists {
	if actualValue, exists := bodyMap[jsonTag]; exists {
	// Skip null values as they're generally valid for optional fields
	if actualValue == nil {
	continue
	}

[Copilot]

[nitpick] The function documentation is minimal and doesn't explain important details about its behavior, such as:

• What happens when the body can't be parsed
• The return value semantics (empty strings indicate no mismatch found)
• The type name conventions used (e.g., "integer" for int, "boolean" for bool)

Consider expanding the documentation:

// detectTypeMismatchFromBody attempts to identify which field caused a JSON type mismatch
// by parsing the request body and comparing against the expected struct type.
// It returns the field name (using JSON tag name), expected type name (using JSON-style names
// like "integer", "boolean", "number"), and actual type name. If no mismatch is found or if
// the body cannot be parsed, it returns empty strings for all three values.

Suggested change

	// by parsing the request body and comparing against the expected struct type
	// by parsing the request body and comparing against the expected struct type.
	// It returns the field name (using JSON tag name), expected type name (using JSON-style names
	// like "integer", "boolean", "number"), and actual type name. If no mismatch is found or if
	// the body cannot be parsed, it returns empty strings for all three values.

[Copilot]

The type name for maps is inconsistent with the type names used for other types. This function returns "map" for map types, but the tests expect field type names like "map[string]string". This inconsistency will cause test failures when map type mismatches occur.

Consider using a more descriptive type name that matches the actual Go type:

case reflect.Map:
    keyType := dereferenceType(expectedFieldType.Key()).Kind()
    valueType := dereferenceType(expectedFieldType.Elem()).Kind()
    expectedTypeName = fmt.Sprintf("map[%s]%s", keyType, valueType)
    if actualValueType != "object" {
        mismatch = true
    }

Suggested change

	expectedTypeName = "map"
	keyType := dereferenceType(expectedFieldType.Key()).Kind().String()
	valueType := dereferenceType(expectedFieldType.Elem()).Kind().String()
	expectedTypeName = fmt.Sprintf("map[%s]%s", keyType, valueType)

[Copilot]

JSON only has one numeric type (number), so when JSON is unmarshaled, all numbers become float64. The case statement case float64, int, int64: will only match float64 in practice since int and int64 cannot result from json.Unmarshal into map[string]interface{}.

The int and int64 cases are unreachable and can be removed:

case float64:
    return "number"

Suggested change

	case float64, int, int64:
	case float64:

[Copilot]

The test expects expectedType to be "int" (line 383), but the detectTypeMismatchFromBody function in common.go returns "integer" for int fields (line 508). This mismatch will cause the test to fail.

The test expectations should match the actual error message format. Update the expected types to:

• "integer" instead of "int" (line 383)
• "boolean" instead of "bool" (lines 390, 397)
• "number" instead of "float64" (lines 432, 439)

[Copilot]

The JSON tag parsing doesn't handle the special case where the JSON tag is "-", which indicates that the field should be ignored during JSON marshaling/unmarshaling. If a field has json:"-", it should be skipped entirely rather than using "-" as the field name.

Add a check after parsing the JSON tag:

// Remove omitempty and other options from the tag
jsonTag = strings.Split(jsonTag, ",")[0]

// Skip fields with json:"-" tag (they're not serialized)
if jsonTag == "-" {
    continue
}

Suggested change

	jsonTag = strings.Split(jsonTag, ",")[0]
	jsonTag = strings.Split(jsonTag, ",")[0]
	// Skip fields with json:"-" tag (they're not serialized)
	if jsonTag == "-" {
	continue
	}

[Copilot]

[nitpick] The detectTypeMismatchFromBody function unmarshals the request body a second time (after it already failed to unmarshal into the struct) to detect which field caused the type mismatch. This additional unmarshaling operation adds performance overhead on every type mismatch error.

While this is acceptable for error cases (which should be rare), consider documenting this trade-off. Additionally, the function could be optimized to only unmarshal when necessary by checking the error message pattern first.

[Copilot]

[nitpick] The type name mapping logic (string -> "string", int/int64/etc -> "integer", float -> "number", bool -> "boolean") is duplicated between the detectTypeMismatchFromBody function (lines 500-537) and the getSchemaForType function (lines 420-448). This duplication increases maintenance burden.

Consider extracting this mapping into a shared helper function that both can use, such as:

func getJSONTypeName(t reflect.Type) string {
    switch dereferenceType(t).Kind() {
    case reflect.String:
        return "string"
    case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
        reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
        return "integer"
    case reflect.Float32, reflect.Float64:
        return "number"
    case reflect.Bool:
        return "boolean"
    // ... etc
    }
}