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

Error Code #31 not raising Koala::Facebook::APIError #695

Open
maxmir20 opened this issue Dec 4, 2024 · 3 comments
Open

Error Code #31 not raising Koala::Facebook::APIError #695

maxmir20 opened this issue Dec 4, 2024 · 3 comments

Comments

@maxmir20
Copy link

maxmir20 commented Dec 4, 2024

Hello!

We've have a rescue in our code for Koala::Facebook::APIErrors and we've noticed that when error code #31 is returned, it gets passed into our response instead of raising a Koala::Facebook::ClientError

Screenshot 2024-12-04 at 12 03 48

When a similar error code is returned #131037, then Koala returns a Koala::Facebook::ClientError

Screenshot 2024-12-04 at 12 05 06

Could this be resolved when someone gets a second?

@ylecuyer
Copy link
Collaborator

ylecuyer commented Dec 5, 2024

Hello @maxmir20, would you be able to share the http status code returned when you have error 31 ? as per https://github.com/arsduo/koala/blob/master/lib/koala/api.rb#L51 and https://github.com/arsduo/koala/blob/master/lib/koala/api/graph_error_checker.rb#L23 it looks like fb is returning 200 in spite of the error

@maxmir20
Copy link
Author

maxmir20 commented Dec 5, 2024

Unfortunately, we aren't able to get error 31 for that particular user anymore, instead we're now getting a #131037 error code instead, which is returning a 400 and raising the correct error.

If we run into it again, I can provide an update here, but it does appear that the response would probably be 200.

Do you think there should be an edge case check in graph_error_checker for this in this instance?

@ylecuyer
Copy link
Collaborator

ylecuyer commented Dec 6, 2024

Do you think there should be an edge case check in graph_error_checker for this in this instance?

No I'd prefer to close the issue for now and wait for extra info if you have it again.

Problem is that fb/graphql is known to return 200 on errors from time to time from what we see with prod load, and starting adding an exception here would mean keeping adding more and more exceptions in the future and they will be hard to maintain

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

2 participants