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

- Sometimes Twitter API return "Rate Limit Exceeded" as text. JSON pa… #73

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

spidgorny
Copy link

  • Sometimes Twitter API return "Rate Limit Exceeded" as text. JSON parsing fails in this case. Fixed by detecting content-type header and returning either {data: } or {text: string} in get() method
  • When using pagination, it's important to fetch the "meta" data with pagination token. Changed so it returns both like this: {data, meta, headers}

…rsing fails in this case. Fixed by detecting content-type header and returning either {data: <json>} or {text: string} in get() method

- When using pagination, it's important to fetch the "meta" data with pagination token. Changed so it returns both like this: {data, meta, headers}
@HunterLarco
Copy link
Owner

This change makes sense to me, however I'd prefer not to change the current return types of utility methods:

  • get(...)
  • post(...)
  • delete(...)

I'd prefer to maintain backwards compatibility. If JSON is not returned I'd imagine we can safely throw an error with the text value instead (see TwitterError.js) Thoughts?

@spidgorny
Copy link
Author

Yes, Rate-Limit is more like an exceptional case - I like the idea to throw an Error.

@hubgit
Copy link

hubgit commented Aug 13, 2021

This also happens if the path is incorrect (e.g. if the path is set to /users instead of users) - the 404 response is HTML.

Throwing an error with the contents of response.status and await response.text() would be useful.

It might be a good idea to check for response.ok before trying to parse the JSON?

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.

3 participants