-
Notifications
You must be signed in to change notification settings - Fork 0
Enhance JSON type mismatch error handling #23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,7 @@ | ||||||||||||||
| package fiberoapi | ||||||||||||||
|
|
||||||||||||||
| import ( | ||||||||||||||
| "encoding/json" | ||||||||||||||
| "fmt" | ||||||||||||||
| "reflect" | ||||||||||||||
| "strconv" | ||||||||||||||
|
|
@@ -50,8 +51,9 @@ func parseInput[TInput any](app *OApiApp, c *fiber.Ctx, path string, options *Op | |||||||||||||
| // It's OK, the POST has no body - ignore the error | ||||||||||||||
| } else { | ||||||||||||||
| // Transform JSON unmarshal type errors into readable validation errors | ||||||||||||||
| // Check if error message contains unmarshal type error pattern | ||||||||||||||
| errMsg := err.Error() | ||||||||||||||
|
|
||||||||||||||
| // Check if error message contains unmarshal type error pattern | ||||||||||||||
| if strings.Contains(errMsg, "json: cannot unmarshal") && strings.Contains(errMsg, "into Go struct field") { | ||||||||||||||
| // Parse the error message to extract field name and type info | ||||||||||||||
| // Format: "json: cannot unmarshal <type> into Go struct field <StructName>.<Field> of type <GoType>" | ||||||||||||||
|
|
@@ -75,8 +77,20 @@ func parseInput[TInput any](app *OApiApp, c *fiber.Ctx, path string, options *Op | |||||||||||||
| fieldName, expectedType, typePart) | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| } else if strings.Contains(errMsg, "json: slice") || strings.Contains(errMsg, "json: map") { | ||||||||||||||
| // Handle "json: slice unexpected end of JSON input" and similar errors | ||||||||||||||
| // This happens when sending wrong type for slice/map fields | ||||||||||||||
| // Try to identify which field caused the error by parsing the request body | ||||||||||||||
| fieldName, expectedType, actualType := detectTypeMismatchFromBody(c.Body(), input) | ||||||||||||||
| if fieldName != "" { | ||||||||||||||
| return input, fmt.Errorf("invalid type for field '%s': expected %s but got %s", | ||||||||||||||
| fieldName, expectedType, actualType) | ||||||||||||||
| } | ||||||||||||||
| // Fallback to generic message if we can't identify the field | ||||||||||||||
| return input, fmt.Errorf("invalid JSON: expected array or object but got incompatible type") | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Return original error if no pattern matched | ||||||||||||||
| return input, err | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
@@ -441,6 +455,116 @@ func getSchemaForType(t reflect.Type) map[string]interface{} { | |||||||||||||
| return schema | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // detectTypeMismatchFromBody attempts to identify which field caused a JSON type mismatch | ||||||||||||||
| // by parsing the request body and comparing against the expected struct type | ||||||||||||||
| func detectTypeMismatchFromBody(body []byte, input interface{}) (fieldName, expectedType, actualType string) { | ||||||||||||||
| // Parse the JSON body into a map to see what was actually sent | ||||||||||||||
| var bodyMap map[string]interface{} | ||||||||||||||
| if err := json.Unmarshal(body, &bodyMap); err != nil { | ||||||||||||||
| return "", "", "" | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+460
to
+465
|
||||||||||||||
|
|
||||||||||||||
| // Get the struct type using reflection | ||||||||||||||
| inputValue := reflect.ValueOf(input) | ||||||||||||||
| if inputValue.Kind() == reflect.Ptr { | ||||||||||||||
| inputValue = inputValue.Elem() | ||||||||||||||
| } | ||||||||||||||
| inputType := inputValue.Type() | ||||||||||||||
|
|
||||||||||||||
| if inputType.Kind() != reflect.Struct { | ||||||||||||||
| return "", "", "" | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| // Iterate through struct fields to find the mismatch | ||||||||||||||
| for i := 0; i < inputType.NumField(); i++ { | ||||||||||||||
| field := inputType.Field(i) | ||||||||||||||
|
|
||||||||||||||
| // Get the JSON tag name (default to field name if no tag) | ||||||||||||||
| jsonTag := field.Tag.Get("json") | ||||||||||||||
| if jsonTag == "" { | ||||||||||||||
| jsonTag = field.Name | ||||||||||||||
| } else { | ||||||||||||||
| // Remove omitempty and other options from the tag | ||||||||||||||
| jsonTag = strings.Split(jsonTag, ",")[0] | ||||||||||||||
|
||||||||||||||
| jsonTag = strings.Split(jsonTag, ",")[0] | |
| jsonTag = strings.Split(jsonTag, ",")[0] | |
| // Skip fields with json:"-" tag (they're not serialized) | |
| if jsonTag == "-" { | |
| continue | |
| } |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
}| 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
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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.
| expectedTypeName = fmt.Sprintf("[]%s", dereferenceType(expectedFieldType.Elem()).Kind()) | |
| elemType := dereferenceType(expectedFieldType.Elem()) | |
| expectedTypeName = fmt.Sprintf("[]%s", getJSONValueType(reflect.Zero(elemType).Interface())) |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
}| expectedTypeName = "map" | |
| keyType := dereferenceType(expectedFieldType.Key()).Kind().String() | |
| valueType := dereferenceType(expectedFieldType.Elem()).Kind().String() | |
| expectedTypeName = fmt.Sprintf("map[%s]%s", keyType, valueType) |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[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
}
}
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
}| return field.Name, expectedTypeName, actualValueType | |
| return jsonTag, expectedTypeName, actualValueType |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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"| case float64, int, int64: | |
| case float64: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The function documentation is minimal and doesn't explain important details about its behavior, such as:
Consider expanding the documentation: