Skip to content

Commit 2a3155f

Browse files
committed
Raise ValueError for required or unacceptable arguments.
This improves consistency with asyncio and within websockets.
1 parent c75b1df commit 2a3155f

File tree

9 files changed

+36
-20
lines changed

9 files changed

+36
-20
lines changed

docs/project/changelog.rst

+12
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,18 @@ Backwards-incompatible changes
6767
Aliases for deprecated API were removed from ``__all__``. As a consequence,
6868
they cannot be imported e.g. with ``from websockets import *`` anymore.
6969

70+
.. admonition:: Several API raise :exc:`ValueError` instead of :exc:`TypeError`
71+
for invalid arguments.
72+
:class: note
73+
74+
:func:`~asyncio.client.connect`, :func:`~asyncio.client.unix_connect`, and
75+
:func:`~asyncio.server.basic_auth` in the :mod:`asyncio` implementation as
76+
well as :func:`~sync.client.connect`, :func:`~sync.client.unix_connect`,
77+
:func:`~sync.server.serve`, :func:`~sync.server.unix_serve`, and
78+
:func:`~sync.server.basic_auth` raise :exc:`ValueError` when a required
79+
argument isn't provided or an argument is provided that is incompatible with
80+
others.
81+
7082
.. admonition:: :attr:`Frame.data <frames.Frame.data>` is now a bytes-like object.
7183
:class: note
7284

src/websockets/asyncio/client.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -352,10 +352,10 @@ def factory() -> ClientConnection:
352352
kwargs.setdefault("ssl", True)
353353
kwargs.setdefault("server_hostname", wsuri.host)
354354
if kwargs.get("ssl") is None:
355-
raise TypeError("ssl=None is incompatible with a wss:// URI")
355+
raise ValueError("ssl=None is incompatible with a wss:// URI")
356356
else:
357357
if kwargs.get("ssl") is not None:
358-
raise TypeError("ssl argument is incompatible with a ws:// URI")
358+
raise ValueError("ssl argument is incompatible with a ws:// URI")
359359

360360
if kwargs.pop("unix", False):
361361
_, connection = await loop.create_unix_connection(factory, **kwargs)

src/websockets/asyncio/server.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -890,10 +890,12 @@ def basic_auth(
890890
whether they're valid.
891891
Raises:
892892
TypeError: If ``credentials`` or ``check_credentials`` is wrong.
893+
ValueError: If ``credentials`` and ``check_credentials`` are both
894+
provided or both not provided.
893895
894896
"""
895897
if (credentials is None) == (check_credentials is None):
896-
raise TypeError("provide either credentials or check_credentials")
898+
raise ValueError("provide either credentials or check_credentials")
897899

898900
if credentials is not None:
899901
if is_credentials(credentials):

src/websockets/sync/client.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -221,17 +221,17 @@ def connect(
221221

222222
wsuri = parse_uri(uri)
223223
if not wsuri.secure and ssl is not None:
224-
raise TypeError("ssl argument is incompatible with a ws:// URI")
224+
raise ValueError("ssl argument is incompatible with a ws:// URI")
225225

226226
# Private APIs for unix_connect()
227227
unix: bool = kwargs.pop("unix", False)
228228
path: str | None = kwargs.pop("path", None)
229229

230230
if unix:
231231
if path is None and sock is None:
232-
raise TypeError("missing path argument")
232+
raise ValueError("missing path argument")
233233
elif path is not None and sock is not None:
234-
raise TypeError("path and sock arguments are incompatible")
234+
raise ValueError("path and sock arguments are incompatible")
235235

236236
if subprotocols is not None:
237237
validate_subprotocols(subprotocols)

src/websockets/sync/server.py

+5-3
Original file line numberDiff line numberDiff line change
@@ -477,14 +477,14 @@ def handler(websocket):
477477
if sock is None:
478478
if unix:
479479
if path is None:
480-
raise TypeError("missing path argument")
480+
raise ValueError("missing path argument")
481481
kwargs.setdefault("family", socket.AF_UNIX)
482482
sock = socket.create_server(path, **kwargs)
483483
else:
484484
sock = socket.create_server((host, port), **kwargs)
485485
else:
486486
if path is not None:
487-
raise TypeError("path and sock arguments are incompatible")
487+
raise ValueError("path and sock arguments are incompatible")
488488

489489
# Initialize TLS wrapper
490490

@@ -667,10 +667,12 @@ def basic_auth(
667667
whether they're valid.
668668
Raises:
669669
TypeError: If ``credentials`` or ``check_credentials`` is wrong.
670+
ValueError: If ``credentials`` and ``check_credentials`` are both
671+
provided or both not provided.
670672
671673
"""
672674
if (credentials is None) == (check_credentials is None):
673-
raise TypeError("provide either credentials or check_credentials")
675+
raise ValueError("provide either credentials or check_credentials")
674676

675677
if credentials is not None:
676678
if is_credentials(credentials):

tests/asyncio/test_client.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -607,7 +607,7 @@ async def test_set_server_hostname(self):
607607
class ClientUsageErrorsTests(unittest.IsolatedAsyncioTestCase):
608608
async def test_ssl_without_secure_uri(self):
609609
"""Client rejects ssl when URI isn't secure."""
610-
with self.assertRaises(TypeError) as raised:
610+
with self.assertRaises(ValueError) as raised:
611611
await connect("ws://localhost/", ssl=CLIENT_CONTEXT)
612612
self.assertEqual(
613613
str(raised.exception),
@@ -616,7 +616,7 @@ async def test_ssl_without_secure_uri(self):
616616

617617
async def test_secure_uri_without_ssl(self):
618618
"""Client rejects no ssl when URI is secure."""
619-
with self.assertRaises(TypeError) as raised:
619+
with self.assertRaises(ValueError) as raised:
620620
await connect("wss://localhost/", ssl=None)
621621
self.assertEqual(
622622
str(raised.exception),

tests/asyncio/test_server.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ async def check_credentials(username, password):
717717

718718
async def test_without_credentials_or_check_credentials(self):
719719
"""basic_auth requires either credentials or check_credentials."""
720-
with self.assertRaises(TypeError) as raised:
720+
with self.assertRaises(ValueError) as raised:
721721
basic_auth()
722722
self.assertEqual(
723723
str(raised.exception),
@@ -726,7 +726,7 @@ async def test_without_credentials_or_check_credentials(self):
726726

727727
async def test_with_credentials_and_check_credentials(self):
728728
"""basic_auth requires only one of credentials and check_credentials."""
729-
with self.assertRaises(TypeError) as raised:
729+
with self.assertRaises(ValueError) as raised:
730730
basic_auth(
731731
credentials=("hello", "iloveyou"),
732732
check_credentials=lambda: False, # pragma: no cover

tests/sync/test_client.py

+3-3
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ def test_set_server_hostname(self):
311311
class ClientUsageErrorsTests(unittest.TestCase):
312312
def test_ssl_without_secure_uri(self):
313313
"""Client rejects ssl when URI isn't secure."""
314-
with self.assertRaises(TypeError) as raised:
314+
with self.assertRaises(ValueError) as raised:
315315
connect("ws://localhost/", ssl=CLIENT_CONTEXT)
316316
self.assertEqual(
317317
str(raised.exception),
@@ -320,7 +320,7 @@ def test_ssl_without_secure_uri(self):
320320

321321
def test_unix_without_path_or_sock(self):
322322
"""Unix client requires path when sock isn't provided."""
323-
with self.assertRaises(TypeError) as raised:
323+
with self.assertRaises(ValueError) as raised:
324324
unix_connect()
325325
self.assertEqual(
326326
str(raised.exception),
@@ -331,7 +331,7 @@ def test_unix_with_path_and_sock(self):
331331
"""Unix client rejects path when sock is provided."""
332332
sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
333333
self.addCleanup(sock.close)
334-
with self.assertRaises(TypeError) as raised:
334+
with self.assertRaises(ValueError) as raised:
335335
unix_connect(path="/", sock=sock)
336336
self.assertEqual(
337337
str(raised.exception),

tests/sync/test_server.py

+4-4
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ def test_connection(self):
361361
class ServerUsageErrorsTests(unittest.TestCase):
362362
def test_unix_without_path_or_sock(self):
363363
"""Unix server requires path when sock isn't provided."""
364-
with self.assertRaises(TypeError) as raised:
364+
with self.assertRaises(ValueError) as raised:
365365
unix_serve(handler)
366366
self.assertEqual(
367367
str(raised.exception),
@@ -372,7 +372,7 @@ def test_unix_with_path_and_sock(self):
372372
"""Unix server rejects path when sock is provided."""
373373
sock = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
374374
self.addCleanup(sock.close)
375-
with self.assertRaises(TypeError) as raised:
375+
with self.assertRaises(ValueError) as raised:
376376
unix_serve(handler, path="/", sock=sock)
377377
self.assertEqual(
378378
str(raised.exception),
@@ -504,7 +504,7 @@ def check_credentials(username, password):
504504

505505
def test_without_credentials_or_check_credentials(self):
506506
"""basic_auth requires either credentials or check_credentials."""
507-
with self.assertRaises(TypeError) as raised:
507+
with self.assertRaises(ValueError) as raised:
508508
basic_auth()
509509
self.assertEqual(
510510
str(raised.exception),
@@ -513,7 +513,7 @@ def test_without_credentials_or_check_credentials(self):
513513

514514
def test_with_credentials_and_check_credentials(self):
515515
"""basic_auth requires only one of credentials and check_credentials."""
516-
with self.assertRaises(TypeError) as raised:
516+
with self.assertRaises(ValueError) as raised:
517517
basic_auth(
518518
credentials=("hello", "iloveyou"),
519519
check_credentials=lambda: False, # pragma: no cover

0 commit comments

Comments
 (0)