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

Do NOT check for OAuthException code #627

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

khalilovcmd
Copy link

I encountered a situation where I receive a Facebook error with code
100 and type OAuthException. I don’t think we need to check for
codes to decide if it is an AuthenticationError or ClientError.

Thanks for submitting a pull request to Koala! A huge portion of the gem has been built by awesome people (like you) from the Ruby community.

Here are a few things that will help get your pull request merged in quickly. None of this is required -- you can delete anything not relevant. If in doubt, open the PR! I want to help.

[ ] My PR has tests and they pass!
[ ] The live tests pass for my changes (LIVE=true rspec -- unrelated failures are okay).
[ ] The PR is based on the most recent master commit and has no merge conflicts.

If you have any questions, feel free to open an issue or comment on your PR!

Note: Koala has a code of conduct. Check it out.

I encountered a situation where I receive a Facebook error with code
`100` and type `OAuthException`. I don’t think we need to check for
codes to decide if it is an `AuthenticationError` or `ClientError`.
@khalilovcmd
Copy link
Author

@arsduo can you have a look please ☺️ ?

@khalilovcmd
Copy link
Author

@arsduo any updates on this?

@ianks
Copy link

ianks commented Dec 20, 2017

I agree with this. I have #630 which adds a new exception code but I think your approach is better. @arsduo Can we merge this PR?

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

Successfully merging this pull request may close these issues.

2 participants