Skip to content

Fix EnvelopePrinterTransport isinstance, return values, and make_tran…

f050b85
Select commit
Loading
Failed to load commit list.
Merged

feat(transport): Add EnvelopePrinterTransport for debug logging #6181

Fix EnvelopePrinterTransport isinstance, return values, and make_tran…
f050b85
Select commit
Loading
Failed to load commit list.
@sentry/warden / warden: code-review completed May 1, 2026 in 58s

2 issues

code-review: Found 2 issues (1 medium, 1 low)

Medium

Overriding __class__ as a property breaks isinstance checks and type identity - `sentry_sdk/transport.py:1091-1093`

The __class__ property returns self._inner.__class__, so type(wrapper) and isinstance(wrapper, EnvelopePrinterTransport) no longer behave as expected. This is a side effect that affects components elsewhere (e.g., any code introspecting the active transport's type, debug logs, or repr), and conflicts with the existing transport architecture, which uses real subclassing rather than type-spoofing decorators. Backwards-compatibility consumers checking isinstance(client.transport, HttpTransport) would silently get the wrong answer for the wrapper.

Low

item.get_bytes() may re-read attachment file from disk on the fallback path - `sentry_sdk/transport.py:1108-1115`

Item.get_bytes() (envelope.py line 179) reads from self.path via open(...).read() when self.bytes is None. After a successful read it caches into self.bytes, so subsequent calls are cheap, but json.loads(item.get_bytes()) on a binary attachment will still load the entire file into memory before raising ValueError, then call get_bytes() again for the size. For large attachments this means the full payload is loaded and then parsed as JSON in debug mode, which can be a meaningful performance hit even though this is a debug-only path.


Duration: 54.9s · Tokens: 172.5k in / 2.4k out · Cost: $0.94 (+merge: $0.00)

Annotations

Check warning on line 1093 in sentry_sdk/transport.py

See this annotation in the file changed.

@sentry-warden sentry-warden / warden: code-review

Overriding __class__ as a property breaks isinstance checks and type identity

The `__class__` property returns `self._inner.__class__`, so `type(wrapper)` and `isinstance(wrapper, EnvelopePrinterTransport)` no longer behave as expected. This is a side effect that affects components elsewhere (e.g., any code introspecting the active transport's type, debug logs, or `repr`), and conflicts with the existing transport architecture, which uses real subclassing rather than type-spoofing decorators. Backwards-compatibility consumers checking `isinstance(client.transport, HttpTransport)` would silently get the wrong answer for the wrapper.