Update function open_unix_socket to check the type of filename argument #1399
Conversation
Update related test file and docstring
trio/_highlevel_open_unix_stream.py
Outdated
| @@ -36,12 +36,13 @@ def close_on_error(obj): | |||
| Raises: | |||
| OSError: If the socket file could not be connected to. | |||
| RuntimeError: If AF_UNIX sockets are not supported. | |||
| TypeError: if filename is not str or bytes. | |||
There was a problem hiding this comment.
I don't think we should mention this in the documentation. It's a general rule that pretty much any Python function can raise TypeError if you give it the wrong argument types, and mentioning it explicitly here distracts from more relevant information.
There was a problem hiding this comment.
Ok for TypeError I removed it
trio/_highlevel_open_unix_stream.py
Outdated
| if filename is None: | ||
| raise ValueError("Filename cannot be None") | ||
| if not isinstance(filename, (str, trio.Path, bytes)): | ||
| raise TypeError("Filename must be str, trio.Path or bytes") |
There was a problem hiding this comment.
This isn't quite correct... really the criterion is "is the object a valid path". And starting in Python 3.6, the proper way to check that is to use the fspath protocol. So if we want to do this check, then the right way to do it would be something like:
filename = os.fspath(filename)This automatically handles all "path-like" objects, including new ones defined in third-party libraries that we've never heard of. And if the user passes in something inappropriate, it gives a good error message:
>>> os.fspath(4)
TypeError: expected str, bytes or os.PathLike object, not intAnnoyingly, though, we still support Python 3.5 (though not for long, see #1396), so we can't just use os.fspath. Instead, use our compatibility wrapper: trio._util.fspath
There was a problem hiding this comment.
Thank you for the information, I didn't know the function os.fspath. To be honest, when I made the change, I told myself that a pathlib.Path object would also be valid but I was "comforted" by the fact that the tests only took into account trio.Path 😛
I changed the check
Remove TypeError in docstring
|
Cycling the pull request to fix the Travis checks. |
|
Thanks! |
|
You are welcome! |
Hi,
this PR fixes an edge case where a user can pass a number when calling
open_unix_socket. In this case he will get aTypeError. There are two possibilities:socketfunction raises the error orAnd in all cases, we need to update the docstring :)