Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 125 additions & 1 deletion common.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package fiberoapi

import (
"encoding/json"
"fmt"
"reflect"
"strconv"
Expand Down Expand Up @@ -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>"
Expand All @@ -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
}
}
Expand Down Expand Up @@ -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
Copy link

Copilot AI Dec 3, 2025

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:

  • 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 uses AI. Check for mistakes.
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
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 uses AI. Check for mistakes.

// 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]
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
}

// Check if this field is in the body map
if actualValue, exists := bodyMap[jsonTag]; exists {
Copy link

Copilot AI Dec 3, 2025

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
}
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 uses AI. Check for mistakes.
expectedFieldType := dereferenceType(field.Type)
actualValueType := getJSONValueType(actualValue)

// Check for type mismatch
mismatch := false
expectedTypeName := ""

switch expectedFieldType.Kind() {
case reflect.String:
expectedTypeName = "string"
if actualValueType != "string" {
mismatch = true
}
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
expectedTypeName = "integer"
if actualValueType != "number" {
mismatch = true
}
case reflect.Float32, reflect.Float64:
expectedTypeName = "number"
if actualValueType != "number" {
mismatch = true
}
case reflect.Bool:
expectedTypeName = "boolean"
if actualValueType != "boolean" {
mismatch = true
}
case reflect.Slice, reflect.Array:
expectedTypeName = fmt.Sprintf("[]%s", dereferenceType(expectedFieldType.Elem()).Kind())
Copy link

Copilot AI Dec 3, 2025

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.

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

Copilot uses AI. Check for mistakes.
if actualValueType != "array" {
mismatch = true
}
case reflect.Map:
expectedTypeName = "map"
Copy link

Copilot AI Dec 3, 2025

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
    }
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 uses AI. Check for mistakes.
if actualValueType != "object" {
mismatch = true
}
case reflect.Struct:
expectedTypeName = "object"
if actualValueType != "object" {
mismatch = true
}
}
Comment on lines +500 to +537
Copy link

Copilot AI Dec 3, 2025

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 uses AI. Check for mistakes.

if mismatch {
return field.Name, expectedTypeName, actualValueType
Copy link

Copilot AI Dec 3, 2025

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
}
Suggested change
return field.Name, expectedTypeName, actualValueType
return jsonTag, expectedTypeName, actualValueType

Copilot uses AI. Check for mistakes.
}
}
}

return "", "", ""
}

// getJSONValueType returns the JSON type name for a value parsed from JSON
func getJSONValueType(value interface{}) string {
switch value.(type) {
case string:
return "string"
case float64, int, int64:
Copy link

Copilot AI Dec 3, 2025

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"
Suggested change
case float64, int, int64:
case float64:

Copilot uses AI. Check for mistakes.
return "number"
case bool:
return "boolean"
case []interface{}:
return "array"
case map[string]interface{}:
return "object"
case nil:
return "null"
default:
return "unknown"
}
}

// mergeParameters merges auto-generated parameters with manually defined ones
// Manual parameters take precedence over auto-generated ones with the same name
func mergeParameters(autoParams []map[string]interface{}, manualParams []map[string]interface{}) []map[string]interface{} {
Expand Down
Loading
Loading