Fastapi: [BUG] RedirectResponse returns 404 for external url's with TestClient

Created on 13 Dec 2019  路  7Comments  路  Source: tiangolo/fastapi

Describe the bug

When trying to test a RedirectResponse with TestClient, it returns a 404 on any external url.

It seems related to this issue but there is an important difference: While in pure starlette, redirects work for urls in the form https://domain.tld/ and fail for https://domain.tld or any path or query parameters, with FastAPI, they fail for ANY external domain (see below).

Feel free to close this issue if it has nothing to do with FastAPI. I assume it has something to do with FastAPI as the behavior is different for FastAPI and pure starlette.

To Reproduce

  1. Create a file with:
from fastapi import FastAPI
from starlette.responses import RedirectResponse
from starlette.testclient import TestClient


def test_redirect():
    app = FastAPI()

    @app.get('/home')
    def home():
        return {}

    @app.get('/redirect_home')
    def redirect():
        return RedirectResponse(url='/home')

    @app.get('/redirect')
    def redirect():
        return RedirectResponse(url='https://www.github.com/')

    with TestClient(app) as client:
        response = client.get('/redirect_home')
        assert response.status_code == 200

        response = client.get('/redirect', allow_redirects=False)
        assert response.status_code == 307

        response = client.get('/redirect')
        assert response.status_code == 200
  1. Run the test with pytest.

The error I get:

            response = client.get('/redirect')
>           assert response.status_code == 200
E           assert 404 == 200

test_bug.py:29: AssertionError

Expected behavior

TestClient returns the correct status code for external urls.

Screenshots

If applicable, add screenshots to help explain your problem.

Environment

  • OS: macOS
  • FastAPI Version: 0.45.0
  • Python version:

Additional context

Add any other context about the problem here.

bug

Most helpful comment

I'm not really sure what behaviour we'd want from the test client for external URLs.
Or even how the test client could determine if it is an external URL?

Eg. If the service returns a redirect response to "https://api.myservice.com/users" how would we determine if that's an external URL, or the service itself?

We also don't really want the test client to make live network queries.

I can kind of see some approachs onto tightening up the behavior here, but it's a complex area.

In the meantime I guess I'd suggest that you use allow_redirects=False for any test client calls that'll result in an external host redirect.

All 7 comments

I'm not really sure what behaviour we'd want from the test client for external URLs.
Or even how the test client could determine if it is an external URL?

Eg. If the service returns a redirect response to "https://api.myservice.com/users" how would we determine if that's an external URL, or the service itself?

We also don't really want the test client to make live network queries.

I can kind of see some approachs onto tightening up the behavior here, but it's a complex area.

In the meantime I guess I'd suggest that you use allow_redirects=False for any test client calls that'll result in an external host redirect.

I am using allow_redirects=False now which helps. It took me a couple of hours to find the underlying issue here and I believe it could me more transparent for other users, maybe with a note in the docs about testing.

Also, the different behavior of FastAPI and starlette puzzled me.

For determining an external URL, I am not sure what needs to be taken into account here. I have questions, though:

  • does the test client know about potential TLD's that might be used in requests?
  • in which cases would you encourage to not use relative URL's with test client? (if the answer is none, than any TLD could be considered external)

Right now I'd recommend that the test client never makes external requests.

At some point we could have some clear configuration around which domains the test client expects the local service to be on, and raise errors on other cases by default.

Some really smart configuration would be to then allow users to either mount mock services against external URLs or mount them as real network requests, but we're not at that point just yet.

Thanks a lot for the help here @tomchristie ! :bowing_man: :cake:

I assume that answers the question, right @chbndrhnns ? If so, you can close the issue.

@tiangolo is there something that we should add to the docs based on the discussion? Or is it rather a corner case in your view?

I think this is probably more like a corner case. I don't think we should add something specific to the docs.

But still, as all the discussion is here in this thread, if someone else has the same corner case they will be able to search and find the info here.

I too scratched my head on this problem of 404's for my attempts of redirection.
I believe

response = client.get('/redirect', allow_redirects=False)
assert response.status_code == 307

Is indeed the way to go.

It seems that an assert based on the returned headers can be added

assert answer.headers["location"] == "your-external-url"
Was this page helpful?
0 / 5 - 0 ratings