Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: error in IPython while handling HTTP4xxClientError #486

Merged
merged 2 commits into from
Sep 11, 2022
Merged

fix: error in IPython while handling HTTP4xxClientError #486

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7bcdd92
fix: error while handling http exceptions
seeM Sep 10, 2022
8fdfd81
simplify `HTTP4xxClientError` subclasses
seeM Sep 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions fastcore/_modidx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,7 @@
'fastcore.net': { 'fastcore.net.HTTP4xxClientError': ('net.html#http4xxclienterror', 'fastcore/net.py'),
'fastcore.net.HTTP5xxServerError': ('net.html#http5xxservererror', 'fastcore/net.py'),
'fastcore.net.Request.summary': ('net.html#request.summary', 'fastcore/net.py'),
'fastcore.net._mk_error': ('net.html#_mk_error', 'fastcore/net.py'),
'fastcore.net._socket_det': ('net.html#_socket_det', 'fastcore/net.py'),
'fastcore.net.do_request': ('net.html#do_request', 'fastcore/net.py'),
'fastcore.net.start_client': ('net.html#start_client', 'fastcore/net.py'),
Expand Down
9 changes: 7 additions & 2 deletions fastcore/net.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,15 @@ def urlopener():
(431,'Header Fields Too Large'),(451,'Legal Reasons')
)

def _mk_error(nm, code, msg):
cls = get_class(nm, 'url', 'hdrs', 'fp', sup=HTTP4xxClientError, msg=msg, code=code)
def _init(self, url, hdrs, fp, msg=msg, code=code): super(cls, self).__init__(url, code, msg, hdrs, fp)
cls.__init__ = _init
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The crux is to override the __init__ set by get_class to call super().__init__.

The _mk_error function was needed so that _init is included as-is in the closure. Without _mk_error, each iteration of the for loop would overwrite the previously defined _init.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is getting pretty obscure! Is there any simpler way to do this, even if a bit more verbose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you like having separate classes for each error code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer to use the urllib.HTTPError class

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing I do want is to be able to catch exceptions based on what kind of error occurred. Is there a way to do that without separate classes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would using type(name, bases, attributes) directly be an easier way to create these?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think like so:

try: urlopen(...)
except HTTPError as e:
    if e.code != 401: raise
    # ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would using type(name, bases, attributes) directly be an easier way to create these?

I thought about it. type(nm, (HTTP4xxClientError,), {'code': code, 'msg': msg}) works and is concise, but would be a tiny breaking change in the signature, from current:

def __init__(self, url, hdrs, fp, msg=default_msg, code=default_code):

to the parent class':

def __init__(self, url, code, msg, hdrs, fp)

That might be okay though

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add __init__ to those attrs to get the init we want?

return cls

for code,msg in _httperrors:
nm = f'HTTP{code}{msg.replace(" ","")}Error'
cls = get_class(nm, 'url', 'hdrs', 'fp', sup=HTTP4xxClientError, msg=msg, code=code)
globals()[nm] = ExceptionsHTTP[code] = cls
globals()[nm] = ExceptionsHTTP[code] = _mk_error(nm, code, msg)

# %% ../nbs/03b_net.ipynb 16
_all_ = ['HTTP400BadRequestError', 'HTTP401UnauthorizedError', 'HTTP402PaymentRequiredError', 'HTTP403ForbiddenError', 'HTTP404NotFoundError', 'HTTP405MethodNotAllowedError', 'HTTP406NotAcceptableError', 'HTTP407ProxyAuthRequiredError', 'HTTP408RequestTimeoutError', 'HTTP409ConflictError', 'HTTP410GoneError', 'HTTP411LengthRequiredError', 'HTTP412PreconditionFailedError', 'HTTP413PayloadTooLargeError', 'HTTP414URITooLongError', 'HTTP415UnsupportedMediaTypeError', 'HTTP416RangeNotSatisfiableError', 'HTTP417ExpectationFailedError', 'HTTP418AmAteapotError', 'HTTP421MisdirectedRequestError', 'HTTP422UnprocessableEntityError', 'HTTP423LockedError', 'HTTP424FailedDependencyError', 'HTTP425TooEarlyError', 'HTTP426UpgradeRequiredError', 'HTTP428PreconditionRequiredError', 'HTTP429TooManyRequestsError', 'HTTP431HeaderFieldsTooLargeError', 'HTTP451LegalReasonsError']
Expand Down
9 changes: 7 additions & 2 deletions nbs/03b_net.ipynb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -284,10 +284,15 @@
" (431,'Header Fields Too Large'),(451,'Legal Reasons')\n",
")\n",
"\n",
"def _mk_error(nm, code, msg):\n",
" cls = get_class(nm, 'url', 'hdrs', 'fp', sup=HTTP4xxClientError, msg=msg, code=code)\n",
" def _init(self, url, hdrs, fp, msg=msg, code=code): super(cls, self).__init__(url, code, msg, hdrs, fp)\n",
" cls.__init__ = _init\n",
" return cls\n",
"\n",
"for code,msg in _httperrors:\n",
" nm = f'HTTP{code}{msg.replace(\" \",\"\")}Error'\n",
" cls = get_class(nm, 'url', 'hdrs', 'fp', sup=HTTP4xxClientError, msg=msg, code=code)\n",
" globals()[nm] = ExceptionsHTTP[code] = cls"
" globals()[nm] = ExceptionsHTTP[code] = _mk_error(nm, code, msg)"
]
},
{
Expand Down