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

jsonapi: Export Errors #263

Open
256dpi opened this issue Aug 30, 2016 · 2 comments
Open

jsonapi: Export Errors #263

256dpi opened this issue Aug 30, 2016 · 2 comments
Assignees
Milestone

Comments

@256dpi
Copy link
Contributor

256dpi commented Aug 30, 2016

As only two of many errors are exported, they will be removed with #261 to be concise.

This issue should track the discussion about how errors should be exported again and therefore handled in calling go applications.

From a quick scan I came up with the following classes of errors:

  • Bad Payload (wrongly formatted JSON API documents)
  • Invalid Target/Source (required implementation of interfaces and type)
  • Nil Argument (some arguments must not be nil)

I would suggest that we exports these classes of errors:

var ErrBadPayload = errors.New("bad payload")
var ErrInvalidSource = errors.New("invalid source")
var ErrInvalidTarget = errors.New("invalid target")
var ErrNilArgument = errors.New("nil argument")

Afterwards we could wrap the errors and add additional context for logging:

return errors.Wrap(ErrInvalidSource, "all slice elements must implement the jsonapi.MarshalIdentifier interface")

The wrapping approach ensures the clean error interface for applications that have no special interest in the errors. Applications that do want to know the cause of error can use:

if errors.Cause(err) == jsonapi.ErrInvalidSource {
  // ...
}

Some Links to this approach:

@wwwdata wwwdata added this to the Release 1.0 milestone Aug 30, 2016
@sharpner
Copy link
Member

Although I really like pkg/errors I am not sure if a library like api2go should force such a framework onto the user, even if its small and helpful. I think we should expose simple standard library errors, as you suggested in your first paragraph, but without the wrapping part.

@sharpner sharpner self-assigned this Oct 28, 2017
@sharpner sharpner mentioned this issue Oct 30, 2017
@mwmahlberg
Copy link

I agree that standard errors should be used. Additionally, helper methods like os.IsNotExists could be provided. This would make error handling very straightforward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants