Skip to content

Tests fail on master due to Twitter API #7369

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

Closed
5 of 6 tasks
dblythy opened this issue Apr 24, 2021 · 14 comments
Closed
5 of 6 tasks

Tests fail on master due to Twitter API #7369

dblythy opened this issue Apr 24, 2021 · 14 comments

Comments

@dblythy
Copy link
Member

dblythy commented Apr 24, 2021

New Issue Checklist

Issue Description

As discussed here, one test fails on master.

Steps to reproduce

Run test Should fail a GET request'.

Actual Outcome

Test should pass

Expected Outcome

Test fails

Failing Test Case / Pull Request

  • 🤩 I submitted a PR with a fix and a test case.
  • 🧐 I submitted a PR with a failing test case.

Environment

Server

  • Parse Server version: master

Database

  • System (MongoDB or Postgres): both

Logs

Failures:
1) OAuth Should fail a GET request
  Message:
    Uncaught exception: SyntaxError: Unexpected end of JSON input
  Stack:
        at <Jasmine>
        at IncomingMessage.<anonymous> (/home/runner/work/parse-server/parse-server/lib/Adapters/Auth/OAuth1Client.js:3:462)
        at IncomingMessage.emit (events.js:327:22)
        at endReadableNT (internal/streams/readable.js:1327:12)
        at processTicksAndRejections (internal/process/task_queues.js:80:21)
@mtrezza mtrezza added type:bug Impaired feature or lacking behavior that is likely assumed needs more info and removed type:bug Impaired feature or lacking behavior that is likely assumed labels Apr 24, 2021
@mtrezza
Copy link
Member

mtrezza commented Apr 24, 2021

This seems to be a temporary Twitter API issue, see comment.

@dblythy
Copy link
Member Author

dblythy commented Apr 24, 2021

Ok. So you would suggest wait until Twitter fix this issue on their end?

@mtrezza
Copy link
Member

mtrezza commented Apr 24, 2021

Do we already know for sure that it's a Twitter issue?

You wrote:

The test is expecting a response of {"errors":[{"code":215,"message":"Bad Authentication data."}]}. When the path is changed to /1.1/oauth/request_token, the correct error is thrown.

That is confusing. The original endpoint returns a config object.

@dblythy
Copy link
Member Author

dblythy commented Apr 24, 2021

No. I’m assuming it is as this issue has never been prevalent before(I.e every test has passed before) and the endpoint now returns an empty page regardless of IP, user agent, etc.

@mtrezza
Copy link
Member

mtrezza commented Apr 24, 2021

That means the test has always been using an incorrect endpoint?

That seems not possible because the existing tests checks for a specific response in validateCannotAuthenticateError, but that response is not returned by the endpoint according to the example response in the docs.

@dblythy
Copy link
Member Author

dblythy commented Apr 24, 2021

That is confusing. The original endpoint returns a config object.

Yes, the original endpoint used to return that object. It returns an empty page right now.

@mtrezza
Copy link
Member

mtrezza commented Apr 24, 2021

Even if it returned an object, the test still wouldn't pass, see the example response.

@dblythy
Copy link
Member Author

dblythy commented Apr 24, 2021

Even if it returned an object, the test still wouldn't pass

Sorry Manuel, I’m not quite sure I follow. The object returned by the Twitter oauth used to pass the tests, and now, does not. How would you suggest this be approached?

@mtrezza
Copy link
Member

mtrezza commented Apr 24, 2021

The object returned by the Twitter oauth used to pass the tests

That is what I doubt. How could it have passed the test? I want to make sure we don't overlook something here.

@dblythy
Copy link
Member Author

dblythy commented Apr 24, 2021

I don’t think we overlooked anything. My theory is that as Twitter move to newer API versions, they are removing older endpoints, and these no longer work. Admittedly this seems a little strange, however I can’t understand why the exisiting endpoint would stop working (rate limiting still happens on new devices).

@mtrezza
Copy link
Member

mtrezza commented Apr 24, 2021

I think we are talking about different aspects of this issue 🙂

Let's break it down:

  • a) Why has the test passed in the past?
    The previous endpoint returned a config object. The response does not contain an error key, because this is not an OAuth endpoint. The API did not change, it is still v1.1, and the test runs against 1.1, hardcoded in the URL. This test should have never passed, because the response does not contain an error key. We could just say, we don't care, but I want to make sure we understand what is going on, maybe there is an additional issue we discover.
  • b) What is the actual test purpose and therefore which endpoint does it require? Your PR seems to make sense, it seems to be testing for a specific error in a failing OAuth response. Maybe your PR is correct after all, but how can we say for sure if we don't understand the context of (a).

We could just care about (b) and close the book, but if we understand (a) we may actually find an additional issue.

@mtrezza
Copy link
Member

mtrezza commented Apr 24, 2021

@dblythy You were right, I looked into this and I understand now that these endpoints are just random endpoints to test the failing auth.

It doesn't matter which endpoint is used as long as it requires OAuth. That confused me, I assumed these specific endpoints were picked for a particular reason, but they were just randomly picked. The test is now failing because of a Twitter API issue it seems, where the endpoint without proper auth does not respond with an error JSON, but with a zero length response, which is not expected by the test. The solution is just to switch to any other GET endpoint that requires OAuth end responds with an auth error JSON as expected.

I will comment on your PR.

@dblythy
Copy link
Member Author

dblythy commented Apr 25, 2021

Sorry, I should've explained that to start. My mistake for not being clear enough!

@mtrezza
Copy link
Member

mtrezza commented Apr 25, 2021

No worries, I found the invalid endpoint in the PR, so the research was worth it. Thanks for tackling this issue to swiftly!

@dblythy dblythy closed this as completed Apr 25, 2021
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