Skip to content
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

Add Unwrap to ErrorWithStatus interface #225

Open
dylanhitt opened this issue Nov 15, 2024 · 3 comments · May be fixed by #228
Open

Add Unwrap to ErrorWithStatus interface #225

dylanhitt opened this issue Nov 15, 2024 · 3 comments · May be fixed by #228
Labels
enhancement New feature or request

Comments

@dylanhitt
Copy link
Collaborator

dylanhitt commented Nov 15, 2024

The current interface is:

// ErrorWithStatus is an interface that can be implemented by an error to provide
// additional information about the error.
type ErrorWithStatus interface {
	error
	StatusCode() int
}

However in order to supply a user defined error the user has to supply something like this:

type MyError fuego.HTTPError

var _ fuego.ErrorWithStatus = MyError{}

func (e MyError) Error() string { return e.Err.Error() }

func (e MyError) StatusCode() int { return http.StatusBadRequest }

func (e MyError) Unwrap() error { return fuego.HTTPError(e) }

If you remove the Unwrap this will still compile but it will return this:

400 Bad Request: 

Vs

400 Bad Request:  <the err>

Unwrap is requited so errors.As can infer the type which we want to always be HTTPError.


However, this code is somewhat brittle, maybe we look at the overall design of custom errors. It feels bad to say here is the interface we need you to adhere too, but basically they have to implement Unwrap always as: func (e MyError) Unwrap() error { return fuego.HTTPError(e) }. I do kinda like that we want everything to be HTTPError as does enforce users to adhere too: RFC 9457. Maybe we just update the docs and explicitly explain that Unwrap must be func (e MyError) Unwrap() error { return fuego.HTTPError(e) } or something along those lines? 🤷

Looking for overall thoughts.

P.S -- I could just be completely missing how to properly do this 😅

@dylanhitt dylanhitt added the enhancement New feature or request label Nov 15, 2024
@dylanhitt
Copy link
Collaborator Author

The following also works if a user needs to a carry additional data with in their error type for some reason

type MyError struct {
	err fuego.HTTPError
}

var _ fuego.ErrorWithStatus = MyError{}

func (e MyError) Error() string { return e.err.Error() }

func (e MyError) StatusCode() int { return http.StatusBadRequest }

func (e MyError) Unwrap() error { return fuego.HTTPError(e.err) }

@dylanhitt
Copy link
Collaborator Author

dylanhitt commented Nov 20, 2024

update: need to figure out how to interface unwrap for errors.As/Is... to be continued.

@dylanhitt
Copy link
Collaborator Author

Or. Another thought. Maybe the concrete type of HTTPError should be a requirement to create a custom error.

@dylanhitt dylanhitt linked a pull request Nov 20, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant