-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Fix Twitter API Oauth Error #7370
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7370 +/- ##
==========================================
- Coverage 93.92% 93.92% -0.01%
==========================================
Files 181 181
Lines 13195 13195
==========================================
- Hits 12394 12393 -1
- Misses 801 802 +1
Continue to review full report at Codecov.
|
Thanks for opening this PR. I think this PR is incorrect. I don't understand why you suggest to change the endpoint, unless the test used an incorrect endpoint to begin with.
The two endpoints seem to be for completely different purposes and the test in this PR may only pass because endpoint (b) returns a JSON (albeit with different content) which makes auth fail as the test expects. My guess is that endpoint (a) is just temporarily down or has been throttled for us because:
Conclusion:
|
Hey @mtrezza, thanks for your comments. You’re correct, as the current endpoint has worked for so long, perhaps it’s a Twitter issue. The change in this PR simply changes the endpoint without attempting to understand why the existing endpoint no longer works (I’m assuming it is depreciated). Perhaps the best approach is wait until the endpoint works again? |
I'll continue in the issue thread. |
After identifying the underlying issue, I suggest the following changes:
|
Thanks for the suggestions! I'll add the changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formally requesting the changes so we don't merge with invalid endpoint by mistake.
This looks good! Any thing you want to add before merging? |
Nope! I'm ready to merge 😊 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Thanks for tackling this so quickly!
* bump parse 3.3.0 * Update CHANGELOG.md * update user test (PR #7464) * fix Twitter API oauth Error (PR #7370) * bumped dependencies * Revert "bumped dependencies" This reverts commit 97ad83d. * bump @parse/push-adapter 3.4.1 * bump jwks-rsa@1.12.3 * bump mongodb@3.6.11 * bump ws@7.5.3 * changed logging for circular obj (PR #7457) * Update CHANGELOG.md
🎉 This change has been released in version 5.0.0-beta.1 |
🎉 This change has been released in version 5.0.0 |
Squashed commits: [1306da7] Merge pull request from GHSA-23r4-5mxp-c7g5 [3a5c38d] revert to version 4.5.0 for testing [a3483d8] fix changelog skip 4.5.1 [3c42584] 4.5.2 [97b1dca] revert to version 4.5.0 for testing [f3133ac] Release 4.10.1 (parse-community#7508) * bump parse 3.3.0 * Update CHANGELOG.md * update user test (PR parse-community#7464) * fix Twitter API oauth Error (PR parse-community#7370) * bumped dependencies * Revert "bumped dependencies" This reverts commit 97ad83d. * bump @parse/push-adapter 3.4.1 * bump jwks-rsa@1.12.3 * bump mongodb@3.6.11 * bump ws@7.5.3 * changed logging for circular obj (PR parse-community#7457) * Update CHANGELOG.md [7e1da90] added changelog [0e3cae5] audit fix [f0d5232] bumped version [4ac4b7f] Merge pull request from GHSA-7pr3-p5fm-8r9x * fix: LQ deletes session token * add 4.10.4 * add changes [ef2ec21] ci: update docker image building (parse-community#7553) * docker * Update docker-publish.yml * Update docker-publish.yml [6ae5835] Merge pull request from GHSA-xqp8-w826-hh6x * Backport the advisory fix * Added a 4.10.3 section to CHANGELOG [0bfa6b7] Release 4.10.2 (parse-community#7513) * move graphql-tag from devDependencies to dependencies (parse-community#7183) * bump version * Update CHANGELOG.md [0be0b87] bump version
Squashed commits: [1306da7] Merge pull request from GHSA-23r4-5mxp-c7g5 [3a5c38d] revert to version 4.5.0 for testing [a3483d8] fix changelog skip 4.5.1 [3c42584] 4.5.2 [97b1dca] revert to version 4.5.0 for testing [f3133ac] Release 4.10.1 (parse-community#7508) * bump parse 3.3.0 * Update CHANGELOG.md * update user test (PR parse-community#7464) * fix Twitter API oauth Error (PR parse-community#7370) * bumped dependencies * Revert "bumped dependencies" This reverts commit 97ad83d. * bump @parse/push-adapter 3.4.1 * bump jwks-rsa@1.12.3 * bump mongodb@3.6.11 * bump ws@7.5.3 * changed logging for circular obj (PR parse-community#7457) * Update CHANGELOG.md [7e1da90] added changelog [0e3cae5] audit fix [f0d5232] bumped version [4ac4b7f] Merge pull request from GHSA-7pr3-p5fm-8r9x * fix: LQ deletes session token * add 4.10.4 * add changes [ef2ec21] ci: update docker image building (parse-community#7553) * docker * Update docker-publish.yml * Update docker-publish.yml [6ae5835] Merge pull request from GHSA-xqp8-w826-hh6x * Backport the advisory fix * Added a 4.10.3 section to CHANGELOG [0bfa6b7] Release 4.10.2 (parse-community#7513) * move graphql-tag from devDependencies to dependencies (parse-community#7183) * bump version * Update CHANGELOG.md [0be0b87] bump version
New Pull Request Checklist
Issue Description
Fixes the failing test explained in #7369.
Approach
Changes the endpoint that the GET request makes from https://api.twitter.com/1.1/help/configuration.json to https://api.twitter.com/1.1/oauth/request_token.