Skip to content

Commit 38f97c2

Browse files
BYKclaude
andcommitted
refactor: Address reviewer feedback
- Remove all hasattr(transport, 'loop') checks in client.py — redundant since AsyncHttpTransport.__init__ always sets self.loop - Remove irrelevant lore entries from AGENTS.md (Consola, Zod, remark-lint) - Remove duplicate and implementation-detail tests per reviewer: - asyncio tests: keep only e2e tests (test_internal_tasks_not_wrapped, test_loop_close_patching, test_loop_close_flushes_async_transport) - client tests: remove _close_components/_flush_components detail tests - transport tests: remove all sync-wrapper and mock-based coverage tests that were not reviewable - Remove section separator comments from test files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 94b6c73 commit 38f97c2

5 files changed

Lines changed: 6 additions & 2255 deletions

File tree

AGENTS.md

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -69,16 +69,4 @@ Do NOT edit these directly — modify source scripts instead:
6969
| `scripts/populate_tox/config.py` | Test suite configuration |
7070

7171
<!-- This section is maintained by the coding agent via lore (https://github.com/BYK/opencode-lore) -->
72-
## Long-term Knowledge
73-
74-
### Gotcha
75-
76-
<!-- lore:019cc484-f0e1-7016-a851-177fb9ad2cc4 -->
77-
* **AGENTS.md must be excluded from markdown linters**: AGENTS.md is auto-managed by lore and uses \`\*\` list markers and long lines that violate typical remark-lint rules (unordered-list-marker-style, maximum-line-length). When a project uses remark with \`--frail\` (warnings become errors), AGENTS.md will fail CI. Fix: add \`AGENTS.md\` to \`.remarkignore\`. This applies to any lore-managed project with markdown linting.
78-
79-
<!-- lore:019cc40e-e56e-71e9-bc5d-545f97df732b -->
80-
* **Consola prompt cancel returns truthy Symbol, not false**: When a user cancels a \`consola\` / \`@clack/prompts\` confirmation prompt (Ctrl+C), the return value is \`Symbol(clack:cancel)\`, not \`false\`. Since Symbols are truthy in JavaScript, checking \`!confirmed\` will be \`false\` and the code falls through as if the user confirmed. Fix: use \`confirmed !== true\` (strict equality) instead of \`!confirmed\` to correctly handle cancel, false, and any other non-true values.
81-
82-
<!-- lore:019cc303-e397-75b9-9762-6f6ad108f50a -->
83-
* **Zod z.coerce.number() converts null to 0 silently**: Zod gotchas in this codebase: (1) \`z.coerce.number()\` passes input through \`Number()\`, so \`null\` silently becomes \`0\`. Be aware if \`null\` vs \`0\` distinction matters. (2) Zod v4 \`.default({})\` short-circuits — it returns the default value without parsing through inner schema defaults. So \`.object({ enabled: z.boolean().default(true) }).default({})\` returns \`{}\`, not \`{ enabled: true }\`. Fix: provide fully-populated default objects. This affected nested config sections in src/config.ts during the v3→v4 upgrade.
8472
<!-- End lore-managed section -->

sentry_sdk/client.py

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,9 +1037,7 @@ def close(
10371037
semantics as :py:meth:`Client.flush`.
10381038
"""
10391039
if self.transport is not None:
1040-
if isinstance(self.transport, AsyncHttpTransport) and hasattr(
1041-
self.transport, "loop"
1042-
):
1040+
if isinstance(self.transport, AsyncHttpTransport):
10431041
logger.warning(
10441042
"close() used with AsyncHttpTransport. "
10451043
"Prefer close_async() for graceful async shutdown. "
@@ -1062,10 +1060,7 @@ async def close_async(
10621060
semantics as :py:meth:`Client.flush_async`.
10631061
"""
10641062
if self.transport is not None:
1065-
if not (
1066-
isinstance(self.transport, AsyncHttpTransport)
1067-
and hasattr(self.transport, "loop")
1068-
):
1063+
if not isinstance(self.transport, AsyncHttpTransport):
10691064
logger.debug(
10701065
"close_async() used with non-async transport, aborting. Please use close() instead."
10711066
)
@@ -1090,9 +1085,7 @@ def flush(
10901085
:param callback: Is invoked with the number of pending events and the configured timeout.
10911086
"""
10921087
if self.transport is not None:
1093-
if isinstance(self.transport, AsyncHttpTransport) and hasattr(
1094-
self.transport, "loop"
1095-
):
1088+
if isinstance(self.transport, AsyncHttpTransport):
10961089
logger.warning(
10971090
"flush() used with AsyncHttpTransport. Please use flush_async() instead."
10981091
)
@@ -1116,10 +1109,7 @@ async def flush_async(
11161109
:param callback: Is invoked with the number of pending events and the configured timeout.
11171110
"""
11181111
if self.transport is not None:
1119-
if not (
1120-
isinstance(self.transport, AsyncHttpTransport)
1121-
and hasattr(self.transport, "loop")
1122-
):
1112+
if not isinstance(self.transport, AsyncHttpTransport):
11231113
logger.debug(
11241114
"flush_async() used with non-async transport, aborting. Please use flush() instead."
11251115
)

0 commit comments

Comments
 (0)