Skip to content
This repository was archived by the owner on Mar 23, 2020. It is now read-only.

Use osin.ErrNotFound for notfound errors #1

Merged
merged 1 commit into from
Jun 28, 2018

Conversation

henriquechehad
Copy link
Contributor

@henriquechehad henriquechehad commented Jun 28, 2018

For Not Found errors it's returning server_error message.

It should use the osin.ErrNotFound as default error.

Like here: https://github.com/RangelReale/osin/blob/master/access.go#L529

That is actually returning the server error message:
https://github.com/RangelReale/osin/blob/master/access.go#L534

As defined in Osin's storage interface:
https://github.com/RangelReale/osin/blob/master/storage.go#L12

@henriquechehad
Copy link
Contributor Author

henriquechehad commented Jun 28, 2018

@uniplaces @arvenil could you review please?

@henriquechehad
Copy link
Contributor Author

Thanks @arvenil. Also can you merge?

@arvenil
Copy link
Contributor

arvenil commented Jun 28, 2018

@henriquechehad Nope, I don't own this repository anymore. Are you using osin and this uniplaces/osin-dynamodb ? In open source or closed project? I have a fork in which I could do it https://github.com/arvenil/osin-dynamodb but in 2 years nobody was interested in this lib ;) And I don't use it at all. But feel free to make PR there.

If today I would need to implement OAuth 2.0 server I would probably try https://github.com/ory/hydra and https://github.com/ory/fosite or anything else that came in those 2 long years (unless really osin got better quality over those 2 years).

@henriquechehad
Copy link
Contributor Author

@arvenil I'll send the PR there. it's a private project.

Are those others with dynamodb support ready?

@henriquechehad
Copy link
Contributor Author

Created new PR arvenil#1 for arvenil/osin-dynamodb

henriquechehad pushed a commit to henriquechehad/osin-dynamodb that referenced this pull request Jun 28, 2018
…-errors

Use osin.ErrNotFound for not found errors
@jsaguiar jsaguiar merged commit 365d46d into uniplaces:master Jun 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants