Custom error message for Unix connect errors #4985

zealws · 2020-09-30T21:38:31Z

What do these changes do?

UnixConnector client errors result in an error message which could be improved.

aiohttp.client_exceptions.ClientConnectorError: Cannot connect to host host:80 ssl:default [Connection refused]

In particular, the error message notes the host/port pair, but for UnixConnector, the socket path is the primary way of identifying the connection, not host/port pair.

This is what the new error message looks like:

aiohttp.client_exceptions.UnixClientConnectorError: Cannot connect to unix socket /tmp/server.sock ssl:default [Connection refused]

Full error and sample code in this gist

Are there changes in behavior for the user?

UnixConnector now raises UnixClientConnectorError instead of ClientConnectorError.

UnixClientConnectorError is a subclass of ClientConnectorError so any catch ClientConnectorError code should still function as expected.

Related issue number

N/A

Checklist

I think the code is well written
Unit tests for the changes exist
Documentation reflects the changes
If you provide code modification, please add yourself to CONTRIBUTORS.txt
- The format is <Name> <Surname>.
- Please keep alphabetical order, the file is sorted by names.
Add a new news fragment into the CHANGES folder
- name it <issue_id>.<type> for example (588.bugfix)
- if you don't have an issue_id change it to the pr id after creating the pr
- ensure type is one of the following:
  - .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.
- Make sure to use full sentences with correct case and punctuation, for example: "Fix issue with non-ascii contents in doctest text files."

zealws · 2020-09-30T21:39:00Z

This fixes #4984

zealws · 2020-09-30T21:40:27Z

I didn't feel like adding a unit-test was essential for this, but if you'd like me to add one, I can.

codecov-commenter · 2020-09-30T21:43:37Z

Codecov Report

Merging #4985 into master will decrease coverage by 0.01%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##           master    #4985      +/-   ##
==========================================
- Coverage   97.60%   97.58%   -0.02%     
==========================================
  Files          43       43              
  Lines        8932     8941       +9     
  Branches     1406     1406              
==========================================
+ Hits         8718     8725       +7     
- Misses         95       97       +2     
  Partials      119      119

Impacted Files	Coverage Δ
aiohttp/client_exceptions.py	`98.31% <77.77%> (-1.69%)`	⬇️
aiohttp/connector.py	`96.28% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90acab1...f466311. Read the comment docs.

aiohttp/client_exceptions.py

webknjaz · 2020-10-02T22:25:21Z

aiohttp/client_exceptions.py

+        super().__init__(connection_key, os_error)
+
+    @property
+    def path(self) -> str:


I think I'd prefer this to be a pathlib.Path instance that's only stringified when used.

UnixConnector historically works with str and has .path string property.
I think we should keep the type for consistency.

webknjaz · 2020-10-02T22:59:37Z

aiohttp/client_exceptions.py

+    Raised in :class:`aiohttp.connector.UnixConnector` if
+        connection to unix socket can not be established.
+    """
+    def __init__(self, path: str, connection_key: ConnectionKey,


It'd be great to expect os.PathLike / pathlib.Path for path.

webknjaz · 2020-10-02T23:01:18Z

aiohttp/client_exceptions.py

+        return self._path
+
+    def __str__(self) -> str:
+        return ('Cannot connect to unix socket {0.path} ssl:{1} [{2}]'


I have two comments here:

it's better to use named placeholders instead of positional ones

you could do self.path in args like with other vars, otherwise, it looks inconsistent

webknjaz

It'd be great to add some integration tests + check how this works with abstract sockets.

zealws · 2020-10-15T04:01:56Z

Thanks for the comments @webknjaz . I'll update this tomorrow per your suggestions.

I can add an integration test for a simple unix socket. However, I'm not sure exactly what you mean by "abstract sockets". Do you have some documentation on the topic you can refer me to?

webknjaz · 2020-10-15T18:12:10Z

@zealws it's a Linux-only feature: https://utcc.utoronto.ca/~cks/space/blog/linux/SocketAbstractNamespace

asvetlov

Looks good in general.
@webknjaz please feel free to merge when ready.
I think the PR can be backported to 3.7 branch (label + bot works fine)

aiohttp/client_exceptions.py

Co-authored-by: Sviatoslav Sydorenko <[email protected]>

asvetlov · 2020-11-21T14:56:46Z

aiohttp/client_exceptions.py

+class UnixClientConnectorError(ClientConnectorError):
+    """Unix connector error.
+
+    Raised in :class:`aiohttp.connector.UnixConnector` if


Suggested change

Raised in :class:`aiohttp.connector.UnixConnector` if

Raised in aiohttp.UnixConnector if

asvetlov · 2020-11-21T15:01:39Z

aiohttp/client_exceptions.py

+        super().__init__(connection_key, os_error)
+
+    @property
+    def path(self) -> str:


UnixConnector historically works with str and has .path string property.
I think we should keep the type for consistency.

asvetlov · 2020-11-21T15:03:22Z

I'll fix the black formatting error by the next commit.

github-actions · 2020-11-21T15:04:09Z

💔 Backport was not successful

The PR was attempted backported to the following branches:

❌ 3.8: Commit could not be cherrypicked due to conflicts

* Custom error message for Unix connect errors * Update aiohttp/client_exceptions.py Co-authored-by: Sviatoslav Sydorenko <[email protected]> Co-authored-by: Andrew Svetlov <[email protected]> Co-authored-by: Sviatoslav Sydorenko <[email protected]>. (cherry picked from commit e19c79e) Co-authored-by: Zeal Wierslee <[email protected]>

* Custom error message for Unix connect errors * Update aiohttp/client_exceptions.py Co-authored-by: Sviatoslav Sydorenko <[email protected]> Co-authored-by: Andrew Svetlov <[email protected]> Co-authored-by: Sviatoslav Sydorenko <[email protected]>. (cherry picked from commit e19c79e) Co-authored-by: Zeal Wierslee <[email protected]> Co-authored-by: Zeal Wierslee <[email protected]>

* Custom error message for Unix connect errors * Update aiohttp/client_exceptions.py Co-authored-by: Sviatoslav Sydorenko <[email protected]> Co-authored-by: Andrew Svetlov <[email protected]> Co-authored-by: Sviatoslav Sydorenko <[email protected]>

Custom error message for Unix connect errors

f466311

zealws requested review from asvetlov and webknjaz as code owners September 30, 2020 21:38

psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Sep 30, 2020

webknjaz reviewed Oct 2, 2020

View reviewed changes

aiohttp/client_exceptions.py Outdated Show resolved Hide resolved

webknjaz reviewed Oct 2, 2020

View reviewed changes

Merge branch 'master' into master

25471f6

asvetlov reviewed Oct 16, 2020

View reviewed changes

aiohttp/client_exceptions.py Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Update aiohttp/client_exceptions.py

25e33c0

Co-authored-by: Sviatoslav Sydorenko <[email protected]>

asvetlov reviewed Nov 21, 2020

View reviewed changes

asvetlov added the backport-3.8 label Nov 21, 2020

asvetlov merged commit e19c79e into aio-libs:master Nov 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom error message for Unix connect errors #4985

Custom error message for Unix connect errors #4985

zealws commented Sep 30, 2020

zealws commented Sep 30, 2020

zealws commented Sep 30, 2020

codecov-commenter commented Sep 30, 2020 •

edited

Loading

webknjaz Oct 2, 2020

asvetlov Nov 21, 2020

webknjaz Oct 2, 2020

webknjaz Oct 2, 2020

webknjaz left a comment

zealws commented Oct 15, 2020

webknjaz commented Oct 15, 2020

asvetlov left a comment

This comment has been minimized.

asvetlov Nov 21, 2020

asvetlov Nov 21, 2020

asvetlov commented Nov 21, 2020

github-actions bot commented Nov 21, 2020

	Raised in :class:`aiohttp.connector.UnixConnector` if
	Raised in aiohttp.UnixConnector if

Custom error message for Unix connect errors #4985

Custom error message for Unix connect errors #4985

Conversation

zealws commented Sep 30, 2020

What do these changes do?

Are there changes in behavior for the user?

Related issue number

Checklist

zealws commented Sep 30, 2020

zealws commented Sep 30, 2020

codecov-commenter commented Sep 30, 2020 • edited Loading

Codecov Report

webknjaz Oct 2, 2020

Choose a reason for hiding this comment

asvetlov Nov 21, 2020

Choose a reason for hiding this comment

webknjaz Oct 2, 2020

Choose a reason for hiding this comment

webknjaz Oct 2, 2020

Choose a reason for hiding this comment

webknjaz left a comment

Choose a reason for hiding this comment

zealws commented Oct 15, 2020

webknjaz commented Oct 15, 2020

asvetlov left a comment

Choose a reason for hiding this comment

This comment has been minimized.

asvetlov Nov 21, 2020

Choose a reason for hiding this comment

asvetlov Nov 21, 2020

Choose a reason for hiding this comment

asvetlov commented Nov 21, 2020

github-actions bot commented Nov 21, 2020

💔 Backport was not successful

codecov-commenter commented Sep 30, 2020 •

edited

Loading