From c7e15a7dd3a49ea520497170e0c91f97d2f1d6c0 Mon Sep 17 00:00:00 2001 From: Jason Morcos Date: Sun, 2 Aug 2026 10:54:21 -0700 Subject: [PATCH] feat(errors): add redacted typed failures --- README.md | 31 ++++ smartthings_local/errors.py | 103 +++++++++++++ smartthings_local/protocol/coap.py | 6 +- smartthings_local/protocol/dtls_probe.py | 5 +- smartthings_local/protocol/dtls_session.py | 47 +++--- tests/test_coap_wire.py | 13 ++ tests/test_dtls_probe.py | 3 +- tests/test_errors.py | 166 +++++++++++++++++++++ 8 files changed, 348 insertions(+), 26 deletions(-) create mode 100644 smartthings_local/errors.py create mode 100644 tests/test_errors.py diff --git a/README.md b/README.md index 1aae362..61a74c2 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,37 @@ If the cert/key are minted at runtime and never written to disk (e.g. inside an sess = DtlsCoapSession("192.168.1.100", 49154, cert_pem=cert_pem, key_pem=key_pem) ``` +### Classified errors + +Runtime transport failures use the public types in + +```python +from smartthings_local.errors import SessionClosedError, SmartThingsLocalError +``` + +All classified errors inherit from `SmartThingsLocalError` and expose a stable +`code`. Their messages are fixed and deliberately omit remote endpoints, local +paths, credential metadata, raw packets, and backend exception text. Existing +callers can keep catching the built-in types used by earlier releases: + +| Error | Stable code | Compatible built-in | +| --- | --- | --- | +| `EndpointError` | `endpoint` | `OSError` | +| `ProbeError` | `probe` | `ConnectionError` | +| `SessionError` | `session` | `ConnectionError` | +| `AuthenticationError` | `authentication` | `ConnectionError` | +| `AuthorizationError` | `authorization` | `PermissionError` | +| `SessionTimeoutError` | `timeout` | `TimeoutError` | +| `SessionClosedError` | `session_closed` | `ConnectionError` | +| `MalformedMessageError` | `malformed_message` | `ValueError` | +| `BlockwiseError` | `blockwise` | `ConnectionError` | +| `ObserveError` | `observe` | `ConnectionError` | + +Constructor argument validation remains a normal `ValueError`. When a backend +failure is chained for debugging, the cause is replaced with a fixed redacted +marker; raw backend text is not copied into the public error or its formatted +traceback. + For a full worked integration, the higher-level `smartthings_local.ocf` layer (`StateCache`, `PollScheduler`, `KeepaliveTask`, `ObserveRefreshTask`) coordinates tiered polling and OBSERVE on top of a session. The MQTT bridge demo below wires all of it together. ### What the demo bridge gives you diff --git a/smartthings_local/errors.py b/smartthings_local/errors.py new file mode 100644 index 0000000..2cc3180 --- /dev/null +++ b/smartthings_local/errors.py @@ -0,0 +1,103 @@ +"""Public, redacted exception types for smartthings-local.""" + +__all__ = [ + 'AuthenticationError', + 'AuthorizationError', + 'BlockwiseError', + 'EndpointError', + 'MalformedMessageError', + 'ObserveError', + 'ProbeError', + 'SessionClosedError', + 'SessionError', + 'SessionTimeoutError', + 'SmartThingsLocalError', +] + + +class SmartThingsLocalError(Exception): + """Base class for classified library failures. + + Subclasses expose a stable ``code`` and a fixed, non-sensitive message. + They intentionally do not accept arbitrary detail because backend errors + can contain remote endpoints, local paths, or credential metadata. + """ + + code = 'smartthings_local_error' + message = 'SmartThings Local operation failed' + + def __init__(self): + super().__init__(self.message) + + def __repr__(self): + return f'{type(self).__name__}(code={self.code!r})' + + +class EndpointError(SmartThingsLocalError, OSError): + """An endpoint could not be resolved, bound, or connected.""" + + code = 'endpoint' + message = 'endpoint operation failed' + + +class ProbeError(SmartThingsLocalError, ConnectionError): + """A DTLS probe failed before producing a protocol result.""" + + code = 'probe' + message = 'DTLS probe failed' + + +class SessionError(SmartThingsLocalError, ConnectionError): + """A connected-session operation failed.""" + + code = 'session' + message = 'session operation failed' + + +class AuthenticationError(SessionError): + """The peer or local credentials could not be authenticated.""" + + code = 'authentication' + message = 'authentication failed' + + +class AuthorizationError(SmartThingsLocalError, PermissionError): + """The authenticated peer is not authorized for an operation.""" + + code = 'authorization' + message = 'operation is not authorized' + + +class SessionTimeoutError(SmartThingsLocalError, TimeoutError): + """A bounded session operation exceeded its deadline.""" + + code = 'timeout' + message = 'session operation timed out' + + +class SessionClosedError(SessionError): + """An operation was attempted on a closed session.""" + + code = 'session_closed' + message = 'session is closed' + + +class MalformedMessageError(SmartThingsLocalError, ValueError): + """A protocol message could not be decoded safely.""" + + code = 'malformed_message' + message = 'malformed protocol message' + + +class BlockwiseError(SessionError): + """A Block1 or Block2 transfer violated its bounded contract.""" + + code = 'blockwise' + message = 'blockwise transfer failed' + + +class ObserveError(SessionError): + """A CoAP Observe relation could not be established or maintained.""" + + code = 'observe' + message = 'Observe relation failed' diff --git a/smartthings_local/protocol/coap.py b/smartthings_local/protocol/coap.py index f20287b..5a7b7ba 100644 --- a/smartthings_local/protocol/coap.py +++ b/smartthings_local/protocol/coap.py @@ -7,6 +7,8 @@ independently. """ import struct +from ..errors import MalformedMessageError + # CoAP option numbers (RFC 7252 + 7641 + 7959) URI_PATH = 11 URI_QUERY = 15 @@ -81,7 +83,7 @@ def parse_coap(data): elif d_nib == 14: delta = 269 + int.from_bytes(data[i:i + 2], 'big'); i += 2 elif d_nib == 15: - raise ValueError("reserved option delta nibble 15") + raise MalformedMessageError() else: delta = d_nib if l_nib == 13: @@ -89,7 +91,7 @@ def parse_coap(data): elif l_nib == 14: length = 269 + int.from_bytes(data[i:i + 2], 'big'); i += 2 elif l_nib == 15: - raise ValueError("reserved option length nibble 15") + raise MalformedMessageError() else: length = l_nib num = prev + delta diff --git a/smartthings_local/protocol/dtls_probe.py b/smartthings_local/protocol/dtls_probe.py index 8c669dc..afd897a 100644 --- a/smartthings_local/protocol/dtls_probe.py +++ b/smartthings_local/protocol/dtls_probe.py @@ -33,6 +33,7 @@ import time from OpenSSL import SSL +from ..errors import ProbeError from .coap import split_dtls from .dtls_session import _OCF_ROOT_CA, _load_pem_chain @@ -214,10 +215,10 @@ def probe(host, port, *, cert_pem=None, key_pem=None, break except SSL.WantReadError: pass - except SSL.Error as e: + except SSL.Error: # A fatal Alert lands here; the alert record was already # captured below, so classification still works. - result.error = str(e) + result.error = ProbeError() break try: diff --git a/smartthings_local/protocol/dtls_session.py b/smartthings_local/protocol/dtls_session.py index 3c0dc11..301f960 100644 --- a/smartthings_local/protocol/dtls_session.py +++ b/smartthings_local/protocol/dtls_session.py @@ -27,6 +27,12 @@ from pathlib import Path from OpenSSL import SSL +from ..errors import ( + BlockwiseError, + SessionClosedError, + SessionError, + SessionTimeoutError, +) from .coap import ( URI_PATH, URI_QUERY, OBSERVE, CONTENT_FORMAT, ACCEPT, BLOCK2, SIZE2, TYPE_CON, TYPE_NON, TYPE_ACK, TYPE_RST, @@ -215,15 +221,17 @@ class DtlsCoapSession: dest = (self.host, self.port) t0 = time.time() + backend_failed = False while time.time() - t0 < self.HANDSHAKE_TIMEOUT_S: try: conn.do_handshake() break except SSL.WantReadError: pass - except SSL.Error as e: + except SSL.Error: sock.close() - raise ConnectionError(f"DTLS handshake error: {e}") from e + backend_failed = True + break try: o = conn.bio_read(65535) if o: @@ -240,8 +248,9 @@ class DtlsCoapSession: time.sleep(0.05) else: sock.close() - raise TimeoutError( - f"DTLS handshake timeout to {self.host}:{self.port}") + raise SessionTimeoutError() + if backend_failed: + raise SessionError() from ConnectionError('DTLS backend failed') self.sock = sock self.conn = conn @@ -303,7 +312,7 @@ class DtlsCoapSession: except Exception: pass for tok, (ev, container) in list(self._pending.items()): - container.setdefault('err', 'socket closed') + container.setdefault('err', SessionClosedError()) ev.set() self._pending.clear() self._observe_tokens.clear() @@ -340,7 +349,7 @@ class DtlsCoapSession: BIO-drain so two writers can't interleave records.""" with self._send_lock: if self.conn is None: - raise ConnectionError("DTLS session closed") + raise SessionClosedError() try: self.conn.send(datagram) self._last_send_ts = time.monotonic() @@ -412,7 +421,7 @@ class DtlsCoapSession: finally: # Make sure pending waiters don't hang if the reader dies. for tok, (ev, container) in list(self._pending.items()): - container.setdefault('err', 'reader exited') + container.setdefault('err', SessionClosedError()) ev.set() def _dispatch_coap(self, datagram): @@ -482,7 +491,7 @@ class DtlsCoapSession: token, and dropping a fresh token on block 1+ silently drops the request.""" if self.conn is None: - raise ConnectionError("DTLS session closed") + raise SessionClosedError() tok = self._next_tok() blob = b'' num = 0 @@ -518,8 +527,7 @@ class DtlsCoapSession: "GET %s /%s block %d: timed out after %d attempt(s)", self.host, '/'.join(path_segs), num, attempt + 1, ) - raise TimeoutError( - f"GET /{'/'.join(path_segs)} block {num} timeout") + raise SessionTimeoutError() logger.debug( "GET %s /%s block %d: attempt %d/%d timeout, retrying", self.host, '/'.join(path_segs), num, @@ -528,7 +536,7 @@ class DtlsCoapSession: finally: self._pending.pop(tok, None) if 'err' in container: - raise ConnectionError(container['err']) + raise container['err'] code = container['code'] payload = container['payload'] @@ -552,16 +560,14 @@ class DtlsCoapSession: break num += 1 if num > self.MAX_BLOCKS: - raise ConnectionError( - f"GET /{'/'.join(path_segs)}: >{self.MAX_BLOCKS} " - f"blocks, aborting") + raise BlockwiseError() return last_code, blob def post(self, path_segs, body_cbor, timeout=8.0): """Single-frame POST with a CBOR-encoded body. Returns (code, payload_bytes). body_cbor must already be encoded.""" if self.conn is None: - raise ConnectionError("DTLS session closed") + raise SessionClosedError() tok = self._next_tok() mid = self._next_mid() opts = [(URI_PATH, s.encode()) for s in path_segs] @@ -575,10 +581,9 @@ class DtlsCoapSession: try: self._send_dgram(datagram) if not ev.wait(timeout): - raise TimeoutError( - f"POST /{'/'.join(path_segs)} timeout") + raise SessionTimeoutError() if 'err' in container: - raise ConnectionError(container['err']) + raise container['err'] return container['code'], container['payload'] finally: self._pending.pop(tok, None) @@ -596,7 +601,7 @@ class DtlsCoapSession: `last_success_ts`, surfaced through KeepaliveTask's `liveness_fn`.""" if self.conn is None: - raise ConnectionError("DTLS session closed") + raise SessionClosedError() mid = self._next_mid() self._send_dgram(build_coap(TYPE_CON, 0, mid, b'', [])) return mid @@ -614,7 +619,7 @@ class DtlsCoapSession: old token gets dropped as 'stale' — acceptable for a 6h-scale safety net.""" if self.conn is None: - raise ConnectionError("DTLS session closed") + raise SessionClosedError() for tok, href in list(self._observe_tokens.items()): segs = [s for s in href.split('/') if s] try: @@ -638,7 +643,7 @@ class DtlsCoapSession: Returns the token used (in case the caller wants to deregister later).""" if self.conn is None: - raise ConnectionError("DTLS session closed") + raise SessionClosedError() tok = self._next_observe_tok() href = '/' + '/'.join(path_segs) # Register the token BEFORE sending — otherwise the device diff --git a/tests/test_coap_wire.py b/tests/test_coap_wire.py index 6f5f0eb..b1e4195 100644 --- a/tests/test_coap_wire.py +++ b/tests/test_coap_wire.py @@ -1,3 +1,6 @@ +import pytest + +from smartthings_local.errors import MalformedMessageError from smartthings_local.protocol.coap import ( build_coap, parse_coap, encode_options, block_value, fmt_code, TYPE_CON, METHOD_GET, URI_PATH, ACCEPT, CF_CBOR, BLOCK2, @@ -43,3 +46,13 @@ def test_block_value_promotes_to_two_bytes_when_num_is_large(): def test_fmt_code_formats_class_dot_detail(): assert fmt_code(0x45) == '2.05' assert fmt_code(0x84) == '4.04' + + +@pytest.mark.parametrize('option_header', (b'\xf0', b'\x0f')) +def test_reserved_option_nibbles_raise_classified_value_error(option_header): + datagram = b'\x40\x01\x00\x01' + option_header + + with pytest.raises(MalformedMessageError) as exc: + parse_coap(datagram) + + assert isinstance(exc.value, ValueError) diff --git a/tests/test_dtls_probe.py b/tests/test_dtls_probe.py index 74aee77..d29b878 100644 --- a/tests/test_dtls_probe.py +++ b/tests/test_dtls_probe.py @@ -1,6 +1,7 @@ import socket import time +from smartthings_local.errors import ProbeError from smartthings_local.protocol import dtls_probe as p @@ -156,4 +157,4 @@ def test_diagnostic_mode_feeds_server_flight_back(monkeypatch): _patch_sock(monkeypatch, fake) r = p.probe('127.0.0.1', 5684, stateless=False, timeout=3.0) assert r.outcome == p.LIVE # HVR still proved liveness - assert r.error is not None # OpenSSL processed the fed-back flight + assert isinstance(r.error, ProbeError) # OpenSSL processed the flight diff --git a/tests/test_errors.py b/tests/test_errors.py new file mode 100644 index 0000000..fdceac2 --- /dev/null +++ b/tests/test_errors.py @@ -0,0 +1,166 @@ +import traceback + +import pytest +from OpenSSL import SSL + +from smartthings_local.errors import ( + AuthenticationError, + AuthorizationError, + BlockwiseError, + EndpointError, + MalformedMessageError, + ObserveError, + ProbeError, + SessionClosedError, + SessionError, + SessionTimeoutError, + SmartThingsLocalError, +) +from smartthings_local.protocol import dtls_session +from smartthings_local.protocol.dtls_session import DtlsCoapSession + +ERROR_TYPES = ( + EndpointError, + ProbeError, + SessionError, + AuthenticationError, + AuthorizationError, + SessionTimeoutError, + SessionClosedError, + MalformedMessageError, + BlockwiseError, + ObserveError, +) + + +@pytest.mark.parametrize( + ('error_type', 'legacy_type'), + ( + (EndpointError, OSError), + (ProbeError, ConnectionError), + (SessionError, ConnectionError), + (AuthenticationError, ConnectionError), + (AuthorizationError, PermissionError), + (SessionTimeoutError, TimeoutError), + (SessionClosedError, ConnectionError), + (MalformedMessageError, ValueError), + (BlockwiseError, ConnectionError), + (ObserveError, ConnectionError), + ), +) +def test_errors_preserve_legacy_builtin_catches(error_type, legacy_type): + error = error_type() + assert isinstance(error, SmartThingsLocalError) + assert isinstance(error, legacy_type) + + +@pytest.mark.parametrize('error_type', ERROR_TYPES) +def test_error_text_is_fixed_and_redacted(error_type): + error = error_type() + text = f'{error!s} {error!r}' + assert error.code + assert str(error) == error.message + assert repr(error) == f'{error_type.__name__}(code={error.code!r})' + assert 'device.example' not in text + assert '/synthetic/client.key' not in text + assert 'credential-value' not in text + with pytest.raises(TypeError): + error_type('credential-value') + + +def test_chained_backend_error_is_not_copied_into_public_text(): + backend = ConnectionError('DTLS backend failed') + + try: + raise SessionError() from backend + except SessionError as error: + assert error.__cause__ is backend + formatted = ''.join(traceback.format_exception(error)) + assert 'DTLS backend failed' in formatted + assert 'device.example' not in formatted + assert 'credential-value' not in formatted + + +def test_handshake_error_is_classified_without_backend_text(monkeypatch): + class FakeContext: + def load_verify_locations(self, *args): + pass + + def set_verify(self, *args): + pass + + def set_cipher_list(self, *args): + pass + + def use_certificate_chain_file(self, *args): + pass + + def use_privatekey_file(self, *args): + pass + + def check_privatekey(self): + pass + + class FakeConnection: + def set_connect_state(self): + pass + + def set_ciphertext_mtu(self, *args): + pass + + def do_handshake(self): + raise SSL.Error('credential-value at device.example') + + class FakeSocket: + closed = False + + def settimeout(self, *args): + pass + + def close(self): + self.closed = True + + fake_socket = FakeSocket() + monkeypatch.setattr(dtls_session.SSL, 'Context', lambda *args: FakeContext()) + monkeypatch.setattr( + dtls_session.SSL, 'Connection', lambda *args: FakeConnection()) + monkeypatch.setattr( + dtls_session.socket, 'socket', lambda *args: fake_socket) + + session = DtlsCoapSession( + 'device.example', 5684, + cert_path='/synthetic/client.pem', + key_path='/synthetic/client.key', + ) + with pytest.raises(SessionError) as exc: + session.connect() + + assert fake_socket.closed + assert isinstance(exc.value, ConnectionError) + assert isinstance(exc.value.__cause__, ConnectionError) + assert exc.value.__context__ is None + formatted = ''.join(traceback.format_exception(exc.value)) + assert 'DTLS backend failed' in formatted + assert 'device.example' not in formatted + assert 'credential-value' not in formatted + + +@pytest.mark.parametrize( + 'operation', + ( + lambda session: session.get(['resource']), + lambda session: session.post(['resource'], b'payload'), + lambda session: session.ping(), + lambda session: session.refresh_observes([]), + lambda session: session.subscribe(['resource']), + ), +) +def test_closed_session_operations_raise_classified_error(operation): + session = DtlsCoapSession( + 'device.example', 5684, + cert_path='/synthetic/client.pem', + key_path='/synthetic/client.key', + ) + + with pytest.raises(SessionClosedError): + operation(session)