Skip to content

Commit

Permalink
Fix NewErrors() and Improve NewError() (#1728)
Browse files Browse the repository at this point in the history
* fix new errors

* fix tests

* removed new errors tests

* changed error message type

* fixed NewError test

* added NewErrors function

* added Test_NewErrors

* added comment line

* refactor: Sprintf changed with Sprint
  • Loading branch information
balcieren committed Jan 27, 2022
1 parent a51ec9b commit 7cf1886
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 33 deletions.
40 changes: 14 additions & 26 deletions app.go
Expand Up @@ -84,8 +84,8 @@ type ErrorHandler = func(*Ctx, error) error

// Error represents an error that occurred while handling a request.
type Error struct {
Code int `json:"code"`
Message string `json:"message"`
Code int `json:"code"`
Message interface{} `json:"message"`

This comment has been minimized.

Copy link
@swengineer404

swengineer404 Feb 10, 2022

This violates symantic versioning and has broken my project. MINOR version modifications are almost always to be done in a backwards-compatible manner. https://semver.org/

Treating this field as a string or using assignment with the concrete type "string" now causes an error. Please keep this in mind going forward so that others don't have to constantly downgrade their modules because of non-semantic updates.

This comment has been minimized.

Copy link
@efectn

efectn Feb 11, 2022

Member

This violates symantic versioning and has broken my project. MINOR version modifications are almost always to be done in a backwards-compatible manner. https://semver.org/

Treating this field as a string or using assignment with the concrete type "string" now causes an error. Please keep this in mind going forward so that others don't have to constantly downgrade their modules because of non-semantic updates.

Hi, can you create an issue? It seems this is critical bug

This comment has been minimized.

Copy link
@balcieren

balcieren Feb 11, 2022

Author Contributor

This violates symantic versioning and has broken my project. MINOR version modifications are almost always to be done in a backwards-compatible manner. https://semver.org/

‎Bu alanı bir dize olarak ele almak veya atamayı "dize" somut türüyle kullanmak artık bir hataya neden olur. Semantik olmayan güncellemeler nedeniyle başkalarının modüllerini sürekli olarak düşürmek zorunda kalmaması için lütfen bunu aklınızda bulundurun.‎

If we can see your problem with your codes. We can revert or solve it.

If you use with err.Message it is going to be return interface{} but if you use with err.Error(), it is going to be return err.Message type of string.

}

// App denotes the Fiber application.
Expand Down Expand Up @@ -715,43 +715,31 @@ func (app *App) Route(prefix string, fn func(router Router), name ...string) Rou

// Error makes it compatible with the `error` interface.
func (e *Error) Error() string {
return e.Message
return fmt.Sprint(e.Message)
}

// NewError creates a new Error instance with an optional message
func NewError(code int, message ...string) *Error {
func NewError(code int, message ...interface{}) *Error {
e := &Error{
Code: code,
Code: code,
Message: utils.StatusMessage(code),
}
if len(message) > 0 {
e.Message = message[0]
} else {
e.Message = utils.StatusMessage(code)
}
return e
}

// NewErrors creates multiple new Errors instance with some message
func NewErrors(code int, messages ...string) []*Error {
var errors []*Error
// NewErrors creates multiple new Error messages
func NewErrors(code int, messages ...interface{}) *Error {
e := &Error{
Code: code,
Message: utils.StatusMessage(code),
}
if len(messages) > 0 {
for _, message := range messages {
e := &Error{
Code: code,
}
e.Message = message
errors = append(errors, e)
}
} else {
// Use default messages
e := &Error{
Code: code,
}
e.Message = utils.StatusMessage(code)
errors = append(errors, e)
e.Message = messages
}

return errors
return e
}

// Listener can be used to pass a custom listener.
Expand Down
13 changes: 6 additions & 7 deletions app_test.go
Expand Up @@ -1212,16 +1212,15 @@ func Benchmark_App_ETag_Weak(b *testing.B) {
func Test_NewError(t *testing.T) {
e := NewError(StatusForbidden, "permission denied")
utils.AssertEqual(t, StatusForbidden, e.Code)
utils.AssertEqual(t, "permission denied", e.Message)
utils.AssertEqual(t, "permission denied", fmt.Sprint(e.Message))
}

func Test_NewErrors(t *testing.T) {
errors := NewErrors(StatusBadRequest, []string{"error 1", "error 2"}...)
utils.AssertEqual(t, StatusBadRequest, errors[0].Code)
utils.AssertEqual(t, "error 1", errors[0].Message)

utils.AssertEqual(t, StatusBadRequest, errors[1].Code)
utils.AssertEqual(t, "error 2", errors[1].Message)
e := NewErrors(StatusBadRequest, "error 1", "error 2")
messages := e.Message.([]interface{})
utils.AssertEqual(t, StatusBadRequest, e.Code)
utils.AssertEqual(t, "error 1", fmt.Sprint(messages[0]))
utils.AssertEqual(t, "error 2", fmt.Sprint(messages[1]))
}

// go test -run Test_Test_Timeout
Expand Down

1 comment on commit 7cf1886

@swengineer404
Copy link

Choose a reason for hiding this comment

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

Please review my comment and consider reverting the change mentioned. As it wasn't necessary and only caused backwards incompatibility.

Please sign in to comment.