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.

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

Catch MissingSchema errors that could be raised by requests #221

Closed
non-Jedi opened this issue May 25, 2018 · 3 comments
Closed

Catch MissingSchema errors that could be raised by requests #221

non-Jedi opened this issue May 25, 2018 · 3 comments

Comments

@non-Jedi
Copy link
Collaborator

See here for example: https://github.com/matrix-org/matrix-python-sdk/pull/179/files#diff-730c26e17b5455de7bbc524b7a20dc2fR33

requests is an implementation detail, so we shouldn't allow end users to see any of its errors.

@slipeer
Copy link

slipeer commented Jul 13, 2018

Perhaps the best solution would be to check URL in the api initiation code?
Api initialization - is single place to implement this check (against a lot of places where you have to check error from requests).
For example:

from urllib3.util import parse_url
from urllib3.exceptions import LocationParseError
try:
    scheme, auth, host, port, path, query, fragment = parse_url(url)
except LocationParseError as e:
    raise MatrixError("Invalid homeserver url %s" % url)

if not scheme:
    raise MatrixError("No scheme in homeserver url")

@non-Jedi
Copy link
Collaborator Author

Ya, that could definitely work. Would need to move MatrixHttpApi.base_url to something like MatrixHttpApi._base_url to make sure people don't modify it at runtime, though. Reality is we have exactly one place where we call into requests with variable arguments (in MatrixHttpApi._send), so it's probably a lot easier to just catch this error there. It's not like this sdk will ever get used for very long with a MatrixHttpApi instantiated but no other methods called, so moving the validation to instantiation doesn't have a huge positive effect. I wouldn't be opposed to either solution though.

slipeer pushed a commit to slipeer/matrix-python-sdk that referenced this issue Jul 19, 2018
slipeer pushed a commit to slipeer/matrix-python-sdk that referenced this issue Jul 19, 2018
non-Jedi pushed a commit that referenced this issue Jul 29, 2018
@non-Jedi
Copy link
Collaborator Author

Closed in #256

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants