Assert statement should be avoided in runtime code
aaliddell opened this issue · 4 comments
When running Python with optimisations enabled, assert
statements are skipped:
The current code generator emits no code for an assert statement when optimization is requested at compile time
-- https://docs.python.org/3/reference/simple_stmts.html#the-assert-statement
So assert
statements are fine in test code or for library debugging, but should not be used in the library code for control flow or main type checks etc. Instead, if
and raise
should be used to give a proper error message to library users.
For example, the check for the passed proto instance being the correct type would be skipped and also presently raises a rather unhelpful AssertionError
with no error context, where a TypeError
would be more useful:
grpclib/grpclib/encoding/proto.py
Line 18 in 5c1018b
# e.g. becomes
if not isinstance(message, message_type):
raise TypeError('Incorrect type {} passed for proto, expected {}'.format(type(message), message_type))
Thanks! Such TypeError is more user-friendly.
There are still a number of other assert
s in the non-testing library code besides just that example above though. Some probably fit in the 'debugging' camp, but it might be worth checking them.
A grep for assert
in the grpclib
directory gives this:
grpclib/events.py:21: assert len(kwargs) == len(self.__slots__), self.__slots__
grpclib/events.py:98: assert (isinstance(dispatches, type)
grpclib/events.py:100: assert dispatches not in dispatch_methods, dispatches
grpclib/client.py:490: assert exc_val is not None
grpclib/client.py:744: assert reply is not None
grpclib/client.py:809: assert reply is not None
grpclib/protocol.py:90: assert size >= 0, 'Size can not be negative'
grpclib/protocol.py:126: assert chunks_size == size
grpclib/protocol.py:168: assert value is None or value >= 0, value
grpclib/protocol.py:277: assert self.headers is not None
grpclib/protocol.py:286: assert self.trailers is not None
grpclib/protocol.py:296: assert self.id is None, self.id
grpclib/protocol.py:336: assert self.id is not None
grpclib/protocol.py:473: assert stream.id is not None
grpclib/protocol.py:478: assert stream.id is not None
grpclib/protocol.py:482: assert stream.id is not None
grpclib/stream.py:30: assert len(message_bin) == message_len, \
grpclib/testing.py:62: assert response.message == 'Hello, Dr. Strange!'
grpclib/testing.py:97: assert self._channel._protocol is not None
grpclib/utils.py:64: assert task
grpclib/utils.py:121: assert method_name is not None
grpclib/health/check.py:146: assert self._poll_task is not None
grpclib/health/service.py:91: assert request is not None
grpclib/health/service.py:111: assert request is not None
Most of these asserts were added specifically for mypy, to convert Optional[thing]
into thing
. Checked all current asserts, seems like all of them are properly placed.
Ok, thanks!