aiohttp raises ValueError for invalid Location: in redir

[wumpus]

Long story short

I am crawling lots of sites, and I don't expect aiohttp to be raising ValueError.

Expected behaviour

Raise an exception that's documented

Actual behaviour

Yarl's ValueError is passed up to the top

Steps to reproduce

import aiohttp
import asyncio
import sys

hostname = None

async def fetch(session, url):
        async with session.get(url) as response:
            return await response.text()

async def main():
    async with aiohttp.ClientSession() as session:
        html = await fetch(session, hostname)
        print(html)

hostname = sys.argv[1]
loop = asyncio.get_event_loop()
loop.run_until_complete(main())

$ python ./bug.py http://www.oberoithreesixtywest.in/
...
  File "/home/lindahl/.pyenv/versions/3.6.4/lib/python3.6/site-packages/yarl/__init__.py", line 348, in origin
    raise ValueError("URL should be absolute")
ValueError: URL should be absolute

Note:

$ curl -D /dev/tty http://www.oberoithreesixtywest.in/
HTTP/1.1 301 Moved Permanently
Server: nginx
Date: Thu, 28 Dec 2017 06:35:33 GMT
Content-Type: text/html; charset=utf-8
Transfer-Encoding: chunked
Connection: keep-alive
X-Frame-Options: GOFORIT
Location: http://

Your environment

aiohttp==2.3.6
yarl==0.16.0

[asvetlov]

Location: http:// is invalid URL, isn't it?

[wumpus]

Yes. I was expecting a better-named exception than ValueError, and text like 'Redirect Location header is invalid'

[asvetlov]

If you need a new feature -- please make a Pull Request ;)

[wumpus]

I don't know what the exception should be named, or which library it should be named in. Those are architecture questions. Heck, for all I know you think ValueError() is a fine error for aiohttp to have, and this bug is invalid. I don't know.

[deasmhumhna]

Also encountered this problem. Shouldn't this return an instance of the aiohttp.InvalidURL exception that already exists? Is it a backward compatibility thing? Either way, it seems too general to have to catch any old ValueError to catch invalid URLs.

[mike-hart]

I've been recently similarly affected by this. Catching ValueError feels a little too broad on a call to session.get unless https://github.com/aio-libs/yarl was the only thing capable of raising ValueError?

I'm a strong second on @deasmhumhna request for InvalidURL, otherwise we have two invalid URL exceptions to manage. https://docs.aiohttp.org/en/stable/client_reference.html#aiohttp.InvalidURL seems to indicate this should occur, or perhaps something akin to InvalidRedirect should exist.

[webknjaz]

Related discussions:

• Support request_info and history in ClientError #2335
• Implement a better way of handling non-HTTP urls in client.ClientSession #2507
• ValueError when redirecting to URL with incorrect schema #3315

PR candidate:

• Implement granular URL error hierarchy in the HTTP client #6722