Skip to content

Fix JSON unmarshal error handling and tests #22

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.

Sign up for GitHub

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

Merged
moutonjeremy merged 2 commits into from
Dec 3, 2025
Merged

Fix JSON unmarshal error handling and tests #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
11ed8d6
Enhance JSON unmarshal error handling to provide clearer validation m…
moutonjeremy Dec 3, 2025
7e410ec
Refactor JSON type mismatch error handling tests for clarity and reli…
moutonjeremy Dec 3, 2025
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
33 changes: 26 additions & 7 deletions common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package fiberoapi

import (
"encoding/json"
"errors"
"fmt"
"reflect"
"strconv"
Expand Down Expand Up @@ -52,12 +50,33 @@ 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
// Using errors.As for more robust error handling (handles wrapped errors)
var unmarshalErr *json.UnmarshalTypeError
if errors.As(err, &unmarshalErr) {
return input, fmt.Errorf("invalid type for field '%s': expected %s but got %s",
unmarshalErr.Field, unmarshalErr.Type.String(), unmarshalErr.Value)
// Check if error message contains unmarshal type error pattern
errMsg := err.Error()
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>"
parts := strings.Split(errMsg, "into Go struct field ")
if len(parts) == 2 {
afterField := parts[1]
fieldParts := strings.Split(afterField, " of type ")
if len(fieldParts) == 2 {
// Extract field name (after the last dot)
fullFieldName := fieldParts[0]
fieldNameParts := strings.Split(fullFieldName, ".")
fieldName := fieldNameParts[len(fieldNameParts)-1]
Comment on lines +63 to +66
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 implementation extracts the Go struct field name (e.g., Description) from the error message, but for user-facing error messages, the JSON field name (e.g., description) should be used instead.

The current code on line 66 extracts Description from the error message "json: cannot unmarshal number into Go struct field Request.Description of type string", but clients send JSON with the field name description (lowercase, as specified in the JSON tag).

To fix this, you would need to:

  1. Convert the field name to lowercase to match the JSON tag convention, OR
  2. Use reflection to look up the actual JSON tag name from the struct field

The test expectations in json_type_error_test.go (lines 66, 72) expect lowercase field names like 'description', which suggests this is a bug that will cause test failures.

Copilot uses AI. Check for mistakes.

// Extract expected type
expectedType := fieldParts[1]

// Extract actual type from the first part
typePart := strings.TrimPrefix(parts[0], "json: cannot unmarshal ")

return input, fmt.Errorf("invalid type for field '%s': expected %s but got %s",
fieldName, expectedType, typePart)
}
}
}
Comment on lines +53 to 78
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 string parsing approach for extracting error information is fragile and tightly coupled to the exact error message format. If the Go standard library changes the error message format in future versions, this code will silently fail to parse the error and fall through to returning the raw error.

Consider a more robust approach:

  1. Use type assertions or errors.As with the actual error type returned by Fiber's BodyParser
  2. Add defensive error handling for parsing failures
  3. Add unit tests specifically for the error message parsing logic to detect format changes early

The string manipulation with multiple splits is also harder to maintain and understand compared to structured error handling.

Copilot uses AI. Check for mistakes.

return input, err
}
}
Expand Down
51 changes: 6 additions & 45 deletions json_type_error_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@ import (
// the raw Go error message is not user-friendly:
// "json: cannot unmarshal number into Go struct field Request.Description of type string"
//
// Solution: Detect json.UnmarshalTypeError and transform it into a readable message:
// Solution: Parse the error message to extract field name and type information,
// then transform it into a readable message:
// "invalid type for field 'description': expected string but got number"
//
// Implementation: Uses errors.As (not type assertion) to handle wrapped errors correctly.
// This ensures the error detection works even if the error is wrapped by Fiber or middleware.
// Implementation: The error type from c.BodyParser() is *errors.UnmarshalTypeError
// (not *json.UnmarshalTypeError), so we parse the error message string to extract
// the field name, expected type, and actual type. This approach works reliably
// across different Fiber versions and handles all JSON unmarshal type errors.
Comment on lines +25 to +28
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 comment states "The error type from c.BodyParser() is *errors.UnmarshalTypeError (not *json.UnmarshalTypeError)", but this is misleading. The standard errors package doesn't have an UnmarshalTypeError type.

If the actual error type is different from *json.UnmarshalTypeError, the comment should clarify what package it comes from (e.g., is it from Fiber's internal error types?). If it's actually a wrapped *json.UnmarshalTypeError, that should be stated clearly.

This documentation is important for future maintainers to understand why string parsing was chosen over type assertions.

Suggested change
// Implementation: The error type from c.BodyParser() is *errors.UnmarshalTypeError
// (not *json.UnmarshalTypeError), so we parse the error message string to extract
// the field name, expected type, and actual type. This approach works reliably
// across different Fiber versions and handles all JSON unmarshal type errors.
// Implementation: The error returned by c.BodyParser() is typically a wrapped *json.UnmarshalTypeError
// (from the encoding/json package), or sometimes a string error depending on Fiber's internal handling.
// Therefore, we parse the error message string to extract the field name, expected type, and actual type.
// This approach works reliably across different Fiber versions and handles all JSON unmarshal type errors.

Copilot uses AI. Check for mistakes.

// Test for JSON type mismatch errors
func TestJSONTypeMismatchErrors(t *testing.T) {
Expand Down Expand Up @@ -115,48 +118,6 @@ func TestJSONTypeMismatchErrors(t *testing.T) {
}
}

// Test that errors.As correctly handles wrapped errors
func TestJSONTypeMismatchWithWrappedError(t *testing.T) {
app := fiber.New()
oapi := New(app)

type TestRequest struct {
Value string `json:"value"`
}

type TestResponse struct {
Result string `json:"result"`
}

Post(oapi, "/test", func(c *fiber.Ctx, input TestRequest) (TestResponse, TestError) {
return TestResponse{Result: "OK"}, TestError{}
}, OpenAPIOptions{})

// Test with wrong type - even if the error is wrapped, errors.As should detect it
req := httptest.NewRequest("POST", "/test", strings.NewReader(`{"value": 123}`))
req.Header.Set("Content-Type", "application/json")
resp, err := app.Test(req)
if err != nil {
t.Fatalf("Expected no error, got %v", err)
}

if resp.StatusCode != 400 {
t.Errorf("Expected status 400, got %d", resp.StatusCode)
}

body, _ := io.ReadAll(resp.Body)
bodyStr := string(body)

// Should contain our custom error message
if !strings.Contains(bodyStr, "invalid type for field 'value'") {
t.Errorf("Expected 'invalid type for field' in error message, got %s", bodyStr)
}

if !strings.Contains(bodyStr, "expected string but got number") {
t.Errorf("Expected 'expected string but got number' in error message, got %s", bodyStr)
}
}

// Test with custom validation error handler
func TestJSONTypeMismatchWithCustomHandler(t *testing.T) {
app := fiber.New()
Expand Down
Loading