feat: Add experimental async transport (port of PR #4572) #5646
3 issues
code-review: Found 3 issues (2 medium, 1 low)
Medium
Test will fail: close() uses warnings.warn but test expects logger.warning - `sentry_sdk/client.py:1049-1052`
The close() method uses warnings.warn() with message 'close() used with AsyncHttpTransport. Use close_async() instead.' but the corresponding test test_close_with_async_transport_warns expects logger.warning() with a different message. This will cause the test to fail. Either change to logger.warning() to match the test expectation, or update the test to use pytest.warns() to capture warnings.
Also found at:
sentry_sdk/client.py:1095-1100tests/test_client.py:1669-1674tests/test_client.py:1737-1741
Test mocking does not satisfy isinstance check, causing test to not verify intended behavior - `tests/integrations/asyncio/test_asyncio.py:669-670`
The test test_loop_close_flushes_async_transport creates mock_transport = Mock(spec=AsyncHttpTransport), but the _flush() function in patch_loop_close() checks isinstance(client.transport, AsyncHttpTransport) which will return False for a Mock object, even with spec=. This means _flush() returns early without calling close_async(). The test assertions should fail, or if they pass, the test is not actually verifying the flush-on-close behavior it intends to test.
Also found at:
tests/test_transport.py:1058
Low
test_async_transport_concurrent_requests doesn't clean up global client state - `tests/test_transport.py:1067`
The test sets the global client via sentry_sdk.get_global_scope().set_client(client) but never resets it to None after the test completes. All other similar tests in this file use either request.addfinalizer() or a try/finally block to reset the client. This can cause test pollution affecting subsequent tests.
Duration: 15m 15s · Tokens: 4.9M in / 61.8k out · Cost: $7.72 (+extraction: $0.01, +merge: $0.00, +fix_gate: $0.00)
Annotations
Check warning on line 1052 in sentry_sdk/client.py
sentry-warden / warden: code-review
Test will fail: close() uses warnings.warn but test expects logger.warning
The `close()` method uses `warnings.warn()` with message 'close() used with AsyncHttpTransport. Use close_async() instead.' but the corresponding test `test_close_with_async_transport_warns` expects `logger.warning()` with a different message. This will cause the test to fail. Either change to `logger.warning()` to match the test expectation, or update the test to use `pytest.warns()` to capture warnings.
Check warning on line 1100 in sentry_sdk/client.py
sentry-warden / warden: code-review
[VM3-AA2] Test will fail: close() uses warnings.warn but test expects logger.warning (additional location)
The `close()` method uses `warnings.warn()` with message 'close() used with AsyncHttpTransport. Use close_async() instead.' but the corresponding test `test_close_with_async_transport_warns` expects `logger.warning()` with a different message. This will cause the test to fail. Either change to `logger.warning()` to match the test expectation, or update the test to use `pytest.warns()` to capture warnings.
Check warning on line 1674 in tests/test_client.py
sentry-warden / warden: code-review
[VM3-AA2] Test will fail: close() uses warnings.warn but test expects logger.warning (additional location)
The `close()` method uses `warnings.warn()` with message 'close() used with AsyncHttpTransport. Use close_async() instead.' but the corresponding test `test_close_with_async_transport_warns` expects `logger.warning()` with a different message. This will cause the test to fail. Either change to `logger.warning()` to match the test expectation, or update the test to use `pytest.warns()` to capture warnings.
Check warning on line 1741 in tests/test_client.py
sentry-warden / warden: code-review
[VM3-AA2] Test will fail: close() uses warnings.warn but test expects logger.warning (additional location)
The `close()` method uses `warnings.warn()` with message 'close() used with AsyncHttpTransport. Use close_async() instead.' but the corresponding test `test_close_with_async_transport_warns` expects `logger.warning()` with a different message. This will cause the test to fail. Either change to `logger.warning()` to match the test expectation, or update the test to use `pytest.warns()` to capture warnings.
Check warning on line 670 in tests/integrations/asyncio/test_asyncio.py
sentry-warden / warden: code-review
Test mocking does not satisfy isinstance check, causing test to not verify intended behavior
The test `test_loop_close_flushes_async_transport` creates `mock_transport = Mock(spec=AsyncHttpTransport)`, but the `_flush()` function in `patch_loop_close()` checks `isinstance(client.transport, AsyncHttpTransport)` which will return `False` for a Mock object, even with `spec=`. This means `_flush()` returns early without calling `close_async()`. The test assertions should fail, or if they pass, the test is not actually verifying the flush-on-close behavior it intends to test.
Check warning on line 1058 in tests/test_transport.py
sentry-warden / warden: code-review
[66H-SEJ] Test mocking does not satisfy isinstance check, causing test to not verify intended behavior (additional location)
The test `test_loop_close_flushes_async_transport` creates `mock_transport = Mock(spec=AsyncHttpTransport)`, but the `_flush()` function in `patch_loop_close()` checks `isinstance(client.transport, AsyncHttpTransport)` which will return `False` for a Mock object, even with `spec=`. This means `_flush()` returns early without calling `close_async()`. The test assertions should fail, or if they pass, the test is not actually verifying the flush-on-close behavior it intends to test.