-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
ssl.SSLError explicit support #2297
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2297 +/- ##
==========================================
- Coverage 97.24% 97.19% -0.06%
==========================================
Files 39 39
Lines 8198 8227 +29
Branches 1433 1435 +2
==========================================
+ Hits 7972 7996 +24
- Misses 98 102 +4
- Partials 128 129 +1
Continue to review full report at Codecov.
|
I am not sure hot to add tests when |
aiohttp/connector.py
Outdated
@@ -390,6 +394,11 @@ def connect(self, req): | |||
if self._closed: | |||
proto.close() | |||
raise ClientConnectionError("Connector is closed.") | |||
except ClientConnectorSSLError as exc: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about just raise
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@fafhrd91 could you review too?
Four eyes is better than two.
will look later today |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs. |
What do these changes do?
Explicit
ssl.SSLError
support andssl.CertificateError
Are there changes in behavior for the user?
Right now
Connector
wraps anyOSError
toClientConnectorError
aiohttp/aiohttp/connector.py
Line 813 in 0b807a8
Later
connect
wraps it one more timeaiohttp/aiohttp/connector.py
Line 393 in 0b807a8
OSError
is lost and 2 timesraise from
Not sure is there any reason to catch
OSError
inconnect
If explicitly re raise
ClientConnectorError
onlytests.test_connector.test_connect_connection_error
fails, but it seems mocked wrong, cuz there is no way to raiseOSError
from_create_connection
it is already re raised toClientConnectorError
in_create_direct_connection
For first, can we remove catching
OSError
fromconnect
? It makes no sense as for meRelated issue number
#2294
Checklist
CONTRIBUTORS.txt
changes
folder<issue_id>.<type>
for example (588.bug)issue_id
change it to the pr id after creating the pr.feature
: Signifying a new feature..bugfix
: Signifying a bug fix..doc
: Signifying a documentation improvement..removal
: Signifying a deprecation or removal of public API..misc
: A ticket has been closed, but it is not of interest to users.