@jsxs0 Thank you for your contribution to implementing RFC 7807 for standardized error responses. This is a valuable addition to the GoFr framework. However, I have a few suggestions to improve the implementation:

1. Chaining Methods for Errors: The current approach of chaining methods for creating errors (e.g., WithInstance, WithExtension) can make the code more complex and harder to maintain. It would be beneficial to simplify the error creation process.
2. Factory Functions: Using factory functions like NewValidationProblem and NewNotFoundProblem to create custom errors is not the most convenient way. It limits flexibility and can lead to code duplication.

Suggested Approach

I propose an alternative approach that leverages configuration to determine the error response format. This approach allows for more flexibility and extensibility.

Environment Variable Configuration

We introduce an ERROR_RESPONSE (or similar) environment variable with two possible values: V1 and V2.

• V1: Uses the existing error response format.
• V2: Uses the RFC 7807 format with all fields serialized as JSON.

Implementation

In the Responder, we check the ERROR_RESPONSE environment variable and adjust the error response accordingly. If ERROR_RESPONSE is set to V2, we use the MarshalJSON method to provide a detailed JSON response. Otherwise, we use the Error method as before.

Example Usage

package main

import (
    "gofr.dev/pkg/gofr"
    "net/http"
)

type MyCustomError struct {
    Type     string `json:"type"`
    Title    string `json:"title"`
    Status   int    `json:"status"`
    Detail   string `json:"detail"`
    Instance string `json:"instance"`
}


func (e MyCustomError) MarshalJSON() ([]byte, error) {
   // marshal the struct
}

func main() {
    app := gofr.New()

    app.GET("/users/:id", func(c *gofr.Context) (interface{}, error) {
        id := c.PathParam("id")
        if id == "invalid" {
            return nil, MyCustomError{
                Type:   "https://api.example.com/problems/validation",
                Title:  "Validation Error",
                Status: http.StatusBadRequest,
                Detail: "Invalid user ID format",
            }
        }

        if id == "999" {
            return nil, MyCustomError{
                Type:     "https://api.example.com/problems/not-found",
                Title:    "Resource Not Found",
                Status:   http.StatusNotFound,
                Detail:   "User not found",
                Instance: "/users/999",
            }
        }

        return map[string]string{"id": id, "name": "John Doe"}, nil
    })

    app.Run()
}

Please do let me know your thoughts on this.

ERROR_RESPONSE env variable based error outputs does not feel like a good usecase for a configuration. Considering the framework calls itself opinionated, it should decide the error format and stick to it.

Users who want different formats for some reason, should be able to implement a custom error struct.

@Umang01-hash, @vikash Thank you both for your feedback. I've revised the code to address both perspectives while maintaining the framework's opinionated nature:

1. Replaced method chaining with functional options pattern - This makes the code more readable and maintainable while preserving flexibility:

   // Before
   return errors.NewNotFoundProblem("User not found").
       WithInstance("/users/999").
       WithExtension("userId", "999")
   
   // After
   return errors.NewValidationProblem(
       "Invalid user ID format",
       errors.WithInstance("/users/invalid"),
       errors.WithExtension("userId", id),
   )

2. Maintained factory functions with enhanced flexibility - Factory functions now accept optional parameters for customisation:

   // Simple case
   errors.NewNotFoundProblem("User not found")
   
   // With customisation
   errors.NewValidationProblem(
       "Invalid input",
       errors.WithInstance("/path"),
       errors.WithExtension("field", "username")
   )

3. Direct creation for maximum flexibility - For completely custom errors:

   errors.NewProblemDetails(
       errors.WithType("https://api.example.com/problems/custom"),
       errors.WithStatus(http.StatusForbidden),
       errors.WithTitle("Access Denied"),
       errors.WithDetail("You don't have permission to access this user")
   )

4. Consistent error format—I've kept RFC 7807 as the standard format without toggling environment variables, which aligns with the framework's opinionated nature.

I hope this approach provides a good balance. It maintains a consistent error-handling strategy while offering flexibility for various use cases. Please check the updated example in examples/error_handling/main.go, which demonstrates these different approaches.

@Umang01-hash, @vikash Thank you both for your feedback. I've revised the code to address both perspectives while maintaining the framework's opinionated nature:

1. Replaced method chaining with functional options pattern - This makes the code more readable and maintainable while preserving flexibility:

   // Before
   return errors.NewNotFoundProblem("User not found").
       WithInstance("/users/999").
       WithExtension("userId", "999")
   
   // After
   return errors.NewValidationProblem(
       "Invalid user ID format",
       errors.WithInstance("/users/invalid"),
       errors.WithExtension("userId", id),
   )

2. Maintained factory functions with enhanced flexibility - Factory functions now accept optional parameters for customisation:

   // Simple case
   errors.NewNotFoundProblem("User not found")
   
   // With customisation
   errors.NewValidationProblem(
       "Invalid input",
       errors.WithInstance("/path"),
       errors.WithExtension("field", "username")
   )

3. Direct creation for maximum flexibility - For completely custom errors:

   errors.NewProblemDetails(
       errors.WithType("https://api.example.com/problems/custom"),
       errors.WithStatus(http.StatusForbidden),
       errors.WithTitle("Access Denied"),
       errors.WithDetail("You don't have permission to access this user")
   )

4. Consistent error format—I've kept RFC 7807 as the standard format without toggling environment variables, which aligns with the framework's opinionated nature.

I hope this approach provides a good balance. It maintains a consistent error-handling strategy while offering flexibility for various use cases. Please check the updated example in examples/error_handling/main.go, which demonstrates these different approaches.

I believe there's a better alternative to this. Adding the Optional pattern still restricts the error fields to what we define, limiting flexibility. I recently discussed this with @Umang01-hash, and I found his approach more effective. It would be great if you could have a discussion with him on Discord and collaborate on this PR as co-authors.

With the current implementation, handlers are expected to return 'error' - which is an interface. That can be used to provide any kind of information about the error.

Besides RFC is not a standard, yet. Although there exist a need to provide structured error, the direction of the PR is not consistent with how errors are handled in the framework.

Considering these, we are closing the PR for now. We need to have a discussion on this.