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.

Sign up for GitHub

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

Jump to bottom

Implement a better way of handling non-HTTP urls in client.ClientSession #2507

Closed
webknjaz opened this issue Nov 10, 2017 · 14 comments · Fixed by #6722
Closed

Implement a better way of handling non-HTTP urls in client.ClientSession #2507

webknjaz opened this issue Nov 10, 2017 · 14 comments · Fixed by #6722
Labels
client enhancement Hacktoberfest We think it's good for https://hacktoberfest.digitalocean.com/ need pull request

Comments

@webknjaz
Copy link
Member

webknjaz commented Nov 10, 2017 •
edited
Loading

Long story short

This is a follow-up for #2479 (comment).
Currently, if one uses ClientSession and it reaches URL with custom schema the exception with insufficient metadata is being raised.

Expected behaviour

There should be some way to give control back to the caller once we reach valid URL, which we cannot follow because of unsupported schema. @asvetlov suggested to implement a custom exception for this.

Actual behaviour

ValueError: Can redirect only to http or https

Steps to reproduce

Query https://steamcommunity.com/mobileconf/conf or steammobile:https://lostauth/, or twitter:https://webknjaz
@webknjaz webknjaz added this to the 3.0 milestone Nov 10, 2017
This was referenced Nov 10, 2017
Closed
Closed
@asvetlov asvetlov modified the milestones: 3.0, 3.1 Feb 9, 2018
@asvetlov asvetlov modified the milestones: 3.1, 3.2 Mar 22, 2018
@asvetlov asvetlov modified the milestones: 3.2, 3.3 May 7, 2018
@asvetlov
Copy link
Member

I'm going to close the issue

@webknjaz webknjaz mentioned this issue Oct 3, 2018
Closed
@berislavlopac
Copy link

I propose that we either reopen this or create a new issue with the goal to create a better exception than ValueError for URL schemas that Client can't handle.

@webknjaz
Copy link
Member Author

webknjaz commented Oct 3, 2018

@berislavlopac the problem is that nobody has time to implement it. Are you willing to send a PR?

@webknjaz webknjaz reopened this Oct 3, 2018
@berislavlopac
Copy link

I'm certainly willing, I'll try to make some time. Just could use some guidance on which type of error would be most appropriate here -- my guess would be InvalidURL, unless there is something more appropriate?

@asvetlov
Copy link
Member

asvetlov commented Oct 5, 2018

InvalidURL sounds good

@asvetlov asvetlov closed this as completed Oct 5, 2018
@asvetlov asvetlov reopened this Oct 5, 2018
@asvetlov
Copy link
Member

asvetlov commented Oct 5, 2018

Sorry, closed accidentally.

@webknjaz
Copy link
Member Author

webknjaz commented Oct 5, 2018

@berislavlopac sounds great! Let's try out what you suggest and then have a discussion in the PR :)

@webknjaz webknjaz added Hacktoberfest We think it's good for https://hacktoberfest.digitalocean.com/ and removed wontfix labels Oct 5, 2018
@webknjaz webknjaz removed this from the 3.3 milestone Oct 5, 2018
@webknjaz
Copy link
Member Author

webknjaz commented Oct 5, 2018

@berislavlopac maybe call it NonHttpUrl?

@webknjaz
Copy link
Member Author

webknjaz commented Oct 5, 2018

It should have some attribute with yarl.URL instance attached, so that when users catch that they could process it accordingly. And it might be nice to have a helper function running xdg-open/open or its Windows analogue depending on the platform.

@webknjaz
Copy link
Member Author

webknjaz commented Oct 5, 2018

(however I'm not sure — maybe such helper should go to the yarl project and be URL's method or so)

@dubs3c
Copy link

dubs3c commented Jan 8, 2020 •
edited
Loading

Any chance this will be implemented?

aiohttp doesn't support this kind of URL either: \\example.com

@webknjaz
Copy link
Member Author

webknjaz commented Jan 9, 2020

@mjdubell feel free to submit a PR.

@berislavlopac
Copy link

I'm due an apology for not implementing it even though I said I will. Life intervened, and I have recently moved away from aiohttp and more towards httpx. Sorry! 😊

@dubs3c

This comment was marked as off-topic.

Sign in to view
@drew2a drew2a mentioned this issue Jan 10, 2024
Closed
@webknjaz webknjaz reopened this Jan 31, 2024
This was referenced Jan 31, 2024
Closed
Merged
Closed
webknjaz added a commit that referenced this issue Feb 13, 2024
@setla @webknjaz
Implement granular URL error hierarchy in the HTTP client
fb465e1 
This patch introduces 5 granular user-facing exceptions that may occur
when HTTP requests are made:
* `InvalidUrlClientError`
* `RedirectClientError`
* `NonHttpUrlClientError`
* `InvalidUrlRedirectClientError`
* `NonHttpUrlRedirectClientError`

Previously `ValueError` or `InvalidURL` was raised and screening out was
complicated (a valid URL that redirects to invalid one raised the same error
as an invalid URL).

Ref: #6722 (comment)

PR #6722

Resolves #2507
Resolves #2630
Resolves #3315

Co-authored-by: Sviatoslav Sydorenko <[email protected]>
@setla setla mentioned this issue Feb 13, 2024
Closed
5 tasks
setla added a commit to setla/aiohttp that referenced this issue Feb 14, 2024
@setla
Implement granular URL error hierarchy in the HTTP client
6b9f421 
This patch introduces 5 granular user-facing exceptions that may occur
when HTTP requests are made:
* `InvalidUrlClientError`
* `RedirectClientError`
* `NonHttpUrlClientError`
* `InvalidUrlRedirectClientError`
* `NonHttpUrlRedirectClientError`

Previously `ValueError` or `InvalidURL` was raised and screening out was
complicated (a valid URL that redirects to invalid one raised the same error
as an invalid URL).

Ref: aio-libs#6722 (comment)

PR aio-libs#6722

Resolves aio-libs#2507
Resolves aio-libs#2630
Resolves aio-libs#3315

Co-authored-by: Sviatoslav Sydorenko <[email protected]>
(cherry picked from commit fb465e1)
@setla setla mentioned this issue Feb 14, 2024
Merged
webknjaz pushed a commit that referenced this issue Feb 14, 2024
@setla
[PR #6722/fb465e15 backport][3.10] Implement granular URL error hiera…
cda4a8b 
…rchy in the HTTP client (#8158)

**This is a backport of PR #6722 as merged into master
(fb465e1).**

This patch introduces 5 granular user-facing exceptions that may occur
when HTTP requests are made:
* `InvalidUrlClientError`
* `RedirectClientError`
* `NonHttpUrlClientError`
* `InvalidUrlRedirectClientError`
* `NonHttpUrlRedirectClientError`

Previously `ValueError` or `InvalidURL` was raised and screening out was
complicated (a valid URL that redirects to invalid one raised the same
error
as an invalid URL).

Ref:
#6722 (comment)

PR #6722

Resolves #2507
Resolves #2630
Resolves #3315

Co-authored-by: Sviatoslav Sydorenko <[email protected]>
(cherry picked from commit fb465e1)
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
client enhancement Hacktoberfest We think it's good for https://hacktoberfest.digitalocean.com/ need pull request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement granular URL error hierarchy in the HTTP client setla/aiohttp
4 participants
@asvetlov @webknjaz @berislavlopac @dubs3c